is be a
separate warning the kernel can turn off initially?
-Kees
--
Kees Cook
, and the refactoring to make them unambiguous is non-trivial. I
think it may need to be some time before -Wflex-array-member-not-at-end
ends up in -Wall.
I gave some examples of this code pattern (and potential solutions)
here:
https://lore.kernel.org/lkml/202310161249.43FB42A6@keescook
--
Kees Cook
dexes? I'd find that kind of awkward for the kernel... but I feel like
I've misunderstood something. :)
-Kees
--
Kees Cook
On Fri, Oct 20, 2023 at 11:50:11AM +0200, Martin Uecker wrote:
> Am Donnerstag, dem 19.10.2023 um 16:33 -0700 schrieb Kees Cook:
> > On Wed, Oct 18, 2023 at 09:11:43PM +, Qing Zhao wrote:
> > > As I replied to Martin in another email, I plan to do the following to
> &
On Fri, Oct 20, 2023 at 09:54:05PM +0200, Martin Uecker wrote:
> Am Freitag, dem 20.10.2023 um 18:48 + schrieb Qing Zhao:
> >
> > > On Oct 20, 2023, at 2:34 PM, Kees Cook wrote:
> > >
> > > On Fri, Oct 20, 2023 at 11:50:11AM +0200, Martin Uecker wrote:
&
On Mon, Oct 23, 2023 at 09:57:45PM +0200, Martin Uecker wrote:
> Am Montag, dem 23.10.2023 um 12:52 -0700 schrieb Kees Cook:
> > On Fri, Oct 20, 2023 at 09:54:05PM +0200, Martin Uecker wrote:
> > > Am Freitag, dem 20.10.2023 um 18:48 + schrieb Qing Zhao:
> > > >
On Tue, Oct 24, 2023 at 07:51:55PM -0400, Siddhesh Poyarekar wrote:
> Yes, that's the tradeoff. However, maybe this is the point where Kees jumps
> in and say the kernel doesn't really care as much or something like that :)
"I only care about -O2" :)
--
Kees Cook
bdos() used by in C code is unchanged?
For example, the Linux kernel can still use __bdos() without knowing
the count member ahead of time (otherwise it kind of defeats the purpose).
--
Kees Cook
On Wed, Oct 25, 2023 at 10:27:41PM +, Qing Zhao wrote:
>
>
> > On Oct 25, 2023, at 6:06 PM, Kees Cook wrote:
> >
> > On Wed, Oct 25, 2023 at 01:27:29PM +, Qing Zhao wrote:
> >> A. Add an additional argument, the size parameter, to __bdos,
>
ble() && i < maximum_possible; i++) {
f->count ++;
p[i] = next_data_item();
}
Now perhaps the problem here is that "count" cannot be used for a count
of "logically valid members in the array" but must always be a count of
"allocated member space in the array", which I guess is tolerable, but
isn't ideal -- I'd like to catch logic bugs in addition to allocation
bugs, but the latter is certainly much more important to catch.
--
Kees Cook
rom things like
ARM's Memory Tagging Extension either -- it only tracks allocation size
(and is very expensive to change as the "used" part of an allocation
grows), so this isn't an unreasonable condition for __counted_by to
require as well.
--
Kees Cook
l:2 xpass:0 skip:0 error:0
Thanks!
-Kees
--
Kees Cook
ion supplied by this attribute
> shows up in the DWARF. It would be good if it did, because that would
> let gdb correctly print these arrays without user intervention.
Does DWARF have such an annotation? Regardless, I think this could be a
future patch to not hold up landing the initial feature.
--
Kees Cook
On Fri, Mar 29, 2024 at 04:06:58PM +, Qing Zhao wrote:
> This is the 8th version of the patch.
Thanks for the updated version!
I've done a full Linux kernel build and run through my behavioral
regression test suite. Everything is working as expected.
-Kees
--
Kees Cook
On Thu, Feb 15, 2024 at 12:32:17AM -0800, Fangrui Song wrote:
> On Fri, Sep 15, 2023 at 11:43 AM Kees Cook via Gcc-patches
> wrote:
> >
> > On Fri, Sep 15, 2023 at 05:47:08PM +, Qing Zhao wrote:
> > >
> > >
> > > > On Sep 15,
lds the Linux kernel where we have almost 300
instances of "counted_by" attributes.
Yay!
-Kees
--
Kees Cook
int array[] __counted_by(size);
} *b;
__must_be_array(b->array)
=> build failure (not expected)
__builtin_types_compatible_p(typeof(b->array), typeof(&(b->array)[0]))
=> 1 (not expected, both pointers?)
--
Kees Cook
treats count as above, so that:
printf("%d\n", p->array[index]); ==> trap when index > (count < 0 ? 0 : count)
Same logic for the sanitizer: any access to the array when count is
invalid means the access is invalid and must be trapped.
This means that software can run safely even in pathological conditions.
-Kees
--
Kees Cook
On Mon, Jan 29, 2024 at 07:32:06PM +, Qing Zhao wrote:
>
>
> > On Jan 29, 2024, at 12:25 PM, Kees Cook wrote:
> >
> > On Mon, Jan 29, 2024 at 04:00:20PM +, Qing Zhao wrote:
> >> An update on the kernel building with my version 4 patch.
> >>
report(x) printf("%s: %zu\n", #x, (size_t)(x))
int main(int argc, char *argv[])
{
struct s foo;
report(__builtin_dynamic_object_size(&foo.nothing, 1));
}
shows:
__builtin_dynamic_object_size(&foo.nothing, 1): 0
-Kees
--
Kees Cook
hould be dealt with under another topic. Should we focus
> on the main issus of cfi, and let it work first on linux kernel, and
> left the compatible issue to be solved later?
If you mean keeping the hashes identical between Clang/LLVM and GCC,
I think this is going to be a requirement due to adding Rust to the
build environment (which uses the LLVM mangling and hashing).
FWIW, I think the subset of type mangling needed isn't the entirely C++
language spec, so it shouldn't be hard to add this to GCC.
-Kees
--
Kees Cook
to clarify, but does any of this change the behavior of
__builtin_object_size() or __builtin_dynamic_object_size() within
functions that take array arguments?
i.e. does this work now?
void foo(int array[10])
{
global = __builtin_object_size(array, 1);
}
(Currently "global" will be set to SIZE_MAX, rather than 40.)
--
Kees Cook
t n" in your example after
"buf", it needs predeclaration:
int h(int n; int buf[n], int n)
{
...
}
(But Clang doesn't appear to support predeclarations.)
--
Kees Cook
rg/bugzilla/show_bug.cgi?id=108306
the difference being operating on an enum. I will reduce the case
and open a bug report for it.
- 1: still examining. It looks like a false positive so far.
Thanks!
-Kees
--
Kees Cook
rhaps, have sanitizer code not influence the value range
tracking? That continues to look like the root cause for these things.
-Kees
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108306
--
Kees Cook
> counter variable without knowing its name.
This tests great for me; thanks! My prototype allocator example I used
for testing is here:
https://github.com/kees/kernel-tools/blob/trunk/fortify/get_counted_by.c
--
Kees Cook
_builtin_get_counted_by(p->array) = count;
}
I don't strictly need to READ the value (but it seems nice). Currently I can
already do a READ with something like this:
size_t count = __builtin_dynamic_object_size(p->array, 1) / sizeof(*p->array);
But I don't have a way to examine the counter _type_ without
__builtin_get_counted_by, so I prefer it over __builtin_set_counted_by.
Thanks!
-Kees
--
Kees Cook
that
returning "void *" is a better signal that it is not valid. And I do
really like the _Generic example there, which makes it even easier to do
the "set if counted_by" action.
--
Kees Cook
actly what Bill has suggested: it is
converting the (void *)NULL into (size_t *)NULL when there is no
counted_by annotation...
-Kees
[1]
https://lore.kernel.org/linux-hardening/20240822231324.make.666-k...@kernel.org/
--
Kees Cook
On Mon, Aug 26, 2024 at 11:01:08PM +0200, Martin Uecker wrote:
> Am Montag, dem 26.08.2024 um 13:30 -0700 schrieb Kees Cook:
> > On Mon, Aug 26, 2024 at 07:30:15PM +, Qing Zhao wrote:
> > > Hi, Martin,
> > >
> > > Looks like that there is some issue
gt; projects).
Thank you for fixing this! :) This will make conversions much much
easier for the Linux kernel (and future userspace programs).
I've tested these patches and everything behaves like I'd expect.
-Kees
--
Kees Cook
single
> > > > member of type @code{char}.
> > > > +@node Flexible Array Members in Unions
> > > > +@section Unions with Flexible Array Members
> > > > +@cindex unions with flexible array members
> > > > +@cindex unions with FAMs
> > > > +
> > > > +GCC permits a C99 flexible array member (FAM) to be in a union:
> > > > +
> > > > +@smallexample
> > > > +union with_fam @{
> > > > + int a;
> > > > + int b[];
> > > > +@};
> > > > +@end smallexample
> > > > +
> > > > +If all the members of a union are flexible array member, the size of
> >
> > It’s for the following case:
> >
> > union with_fam_3 {
> > char a[];
> > int b[];
> > }
> >
> > And also include: (the only member of a union is a flexible array
> > member as you mentioned below)
> >
> > union with_fam_1 {
> > char a[];
> > }
> >
> > So, I think the original sentence:
> >
> > “If all the members of a union are flexible array member, the size of”
> >
> > Should be better than the below:
> > >
> > > "If the only member of a union is a flexible array member”
>
> Makes sense, but then it should be "members" both times rather than
> "members" and then "member".
"If every member of a union is a flexible array, the size ..." ?
--
Kees Cook
int foo;
int bar;
);
char data[];
};
struct mid_flex { struct flex_hdr hdr; int n; int o; };
Then struct flex member names don't have to change, but if anything is
trying to get at struct flex::data through struct mid_flex::hdr, that'll
need casting. But it _shouldn't_ since it has "n" and "o".
-Kees
[1] https://gcc.gnu.org/pipermail/gcc-patches/2023-May/620122.html
[2] https://github.com/RTEMS/rtems
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/container_of.h#n10
[4] https://git.kernel.org/linus/896880ff30866f386ebed14ab81ce1ad3710cfc4
[5]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/stddef.h?h=v6.8#n11
--
Kees Cook
ide of the
"if" statements is the range tracking [4,INT_MAX].)
However, in the case where jump threading has split the execution flow
and produced a copy of "*val = sg->vals[index];" where the value range
tracking for "index" is now [4,INT_MAX], is the warning valid. But it
is only for that instance. Reporting it for effectively both (there is
only 1 source line for the array indexing) is misleading because there
is nothing the user can do about it -- the compiler created the copy and
then noticed it had a range it could apply to that array index.
This situation makes -Warray-bounds unusable for the Linux kernel (we
cannot have false positives says BDFL), but we'd *really* like to have
it enabled since it usually finds real bugs. But these false positives
can't be fixed on our end. :( So, moving them to -Warray-bounds=2 makes
sense as that's the level documented to have potential false positives.
-Kees
--
Kees Cook
On Tue, May 14, 2024 at 01:38:49AM +0200, Andrew Pinski wrote:
> On Mon, May 13, 2024, 11:41 PM Kees Cook wrote:
> > But it makes no sense to warn about:
> >
> > void sparx5_set (int * ptr, struct nums * sg, int index)
> > {
> >if (index >= 4)
> >
Han,
Have you done full testsuite runs with and without this patch? I know
you mentioned you've done builds on both x86_64 and arm; it might be
nice to compare tests too.
Thanks,
-Kees
On Mon, Sep 10, 2012 at 5:26 PM, Matthias Klose wrote:
> On 08.09.2012 01:07, Kees Cook wrot
Hi,
I'm curious about the status of this patch:
http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00974.html
Chrome OS uses this, and the Ubuntu Security Team has expressed
interest in it as well. What's needed to land this in gcc?
Thanks!
-Kees
--
Kees Cook
Chrome OS Security
Linux uses the first 2 modes already, and has had plans to
use the third (smaller resulting image).
Most notably, Linux _must_ have a warn-only mode or the feature will
never get merged (this is a hard requirement from Linus). All serious
deployments of the feature will use either trap mode or use the
trap-on-warn setting, of course. But for the feature to even see the
light of day, Linus requires there be a warn-only mode.
So, given these requirements, continuing to use the sanitizer framework
seems much simpler to me. :)
-Kees
--
Kees Cook
cross fingers*
-Kees
--
Kees Cook
ecause say
> -Wno-unterminated-string-initialization will no longer turn off that
> warning, essentially -Wc++-compat and -Wunterminated-string-initialization
> are then independent warnings with -Wc++-compat having priority over the
> latter.
> Or we could guard it with
> warn_cxx_compat && warn_unterminated_string_initialization
> but then the question is what to pass to second warning_at option,
> because one needs both.
> So, I think it is better to keep -Wc++-compat working as before (perhaps
> with the extra descriptions of the two lengths) and then have this new
> -Wunterminated-string-initialization warning implied by -Wextra which
> does nothing in the rare case when -Wc++-compat is on, and otherwise
> does the new stuff of warning except when initializing nonstring
> objects.
Okay, I will take a stab at getting this reorganized. Thanks for the
review!
--
Kees Cook
ing it into GCC 15 would be amazing!
Thanks,
-Kees
--
Kees Cook
When initializing a nonstring char array when compiled with
-Wunterminated-string-initialization the warning trips even when
truncating the trailing NUL character from the string constant. Only
warn about this when running under -Wc++-compat since under C++ we should
not initialize nonstrings from
x, but it may not be too late. Linux is pretty
> much the main user of this feature, and we have pretty good control
> over how it's used there.
While it'll take a bit longer, I am convinced as well. It will let us do
things we keep tripping over (like fs_bulk.len example above).
On Fri, Mar 07, 2025 at 09:41:15PM -0800, Kees Cook wrote:
> On Tue, Feb 18, 2025 at 11:51:41AM -0800, Kees Cook wrote:
> > On Fri, Feb 14, 2025 at 11:21:07AM +0100, Jakub Jelinek wrote:
> > > On Thu, Feb 13, 2025 at 02:10:25PM +0100, Jakub Jelinek wrote:
> > > >
On April 22, 2025 12:08:51 AM PDT, Sam James wrote:
>Kees Cook writes:
>
>> On Thu, Apr 10, 2025 at 05:17:51PM -0700, Keith Packard wrote:
>>> A target using 16-bit ints won't have enough bits to hold the whole
>>> flag_sanitize set. Be explicit about
shall we issue warning and delete the counted_by attribute from the field?
I think it needs to stay attached for __bdos. And from the looks of it,
even array access works with 1-byte values too:
extern void *ptr;
void *foo(int num) {
return &ptr[num];
}
The assembly output of this shows it's doing byte addition. Clang
doesn't warn about this, but GCC does:
:5:16: warning: dereferencing 'void *' pointer
5 | return &ptr[num];
|^
So, I think even the bounds sanitizer should handle it, even if a
warning ultimately gets emitted.
-Kees
--
Kees Cook
ptimizations.
FWIW, I've been very happily using this to find and squash tricky bugs
in Linux for a few months now. This is finally getting us close to being
able to enable -Warray-bounds globally. :)
https://lore.kernel.org/all/?q=%22-fdiagnostic%22+%22-details%22
--
Kees Cook
On Thu, Apr 24, 2025 at 06:06:03PM +, Qing Zhao wrote:
>
>
> > On Apr 24, 2025, at 13:07, Kees Cook wrote:
> >
> > On Thu, Apr 24, 2025 at 04:36:14PM +, Qing Zhao wrote:
> >>
> >>> On Apr 24, 2025, at 11:59, Martin Uecker wrote:
> &
to use counted_by (and not sized_by) so that users of
the annotation don't need to have to change the marking just because it's "void
*". Everything else operates on "void *" as if it were u8 ...
Regardless, ignoring "void *", the rest of my initial testing (of both GCC and
Clang) is positive. The test cases are all behaving as expected! Yay! :) I will
try to construct some more goofy stuff to find more corner cases.
And at some future point we may want to think about -fsanitize=pointer-overflow
using this information too, to catch arithmetic and increments past the
bounds...
struct foo {
u8 *buf __counted_by(len);
int len;
} str;
u8 *walk;
str->buf = malloc(10);
str->len = 10;
walk = str->buf + 12; // trip!
for (walk = str->buf; ; walk++) // trip after 10 loops
;
-Kees
--
Kees Cook
On Fri, Apr 25, 2025 at 04:56:52PM +, Qing Zhao wrote:
>
>
> > On Apr 24, 2025, at 13:07, Kees Cook wrote:
> >
> > On Thu, Apr 24, 2025 at 04:36:14PM +, Qing Zhao wrote:
> >>
> >>> On Apr 24, 2025, at 11:59, Martin Uecker wrote:
> &
};
Is that correct? It feels like if we're already able to do this analysis,
then "1" should be possible also. Perhaps I'm misunderstanding something
about the parser.
Thanks!
-Kees
--
Kees Cook
On Tue, Apr 15, 2025 at 09:05:20PM +, Qing Zhao wrote:
> > On Apr 15, 2025, at 16:35, Kees Cook wrote:
> > 1) When applying counted_by to pointer members, are out-of-order member
> > declarations expected to be handled? As in, is this expected to be valid?
> >
> &g
n, is this something you (or someone
else) can get committed for GCC 15?
Thanks!
-Kees
P.S. Sorry for the double email Martin, it took me a while to find mbox;
I'm not subscribed to gcc-patches. Thankfully the patchwork has it:
https://patchwork.sourceware.org/project/gcc/patch/20250411001751.141494-1-kei...@keithp.com/
--
Kees Cook
ing this
available. I'll give it a spin. :)
--
Kees Cook
On Tue, Feb 18, 2025 at 11:51:41AM -0800, Kees Cook wrote:
> On Fri, Feb 14, 2025 at 11:21:07AM +0100, Jakub Jelinek wrote:
> > On Thu, Feb 13, 2025 at 02:10:25PM +0100, Jakub Jelinek wrote:
> > > Kees, are you submitting this under assignment to FSF (maybe the Google
> &g
> for details. If DCO, can you add your Signed-off-by: tag for it?
>>
>> So far lightly tested, ok for trunk if it passes bootstrap/regtest?
>
>Bootstrapped/regtested on x86_64-linux and i686-linux successfully.
>
>> 2025-02-13 Kees Cook
>> Jakub J
you're proposing adding "instance scope", which merges an
object's member names with the global scope variable names. And to access
global scope in the face of an aliased name, __builtin_global_ref(NAME)
would be needed. (Such a builtin would be useful in other areas too,
e.g. local scope to access an aliased name...)
It does seem like this solves all the same problems, but I still don't
like how fragile it is in the face of adding a global that might alias a
used member name. When this happens in local scope, it's limited to the
given function, and change any behaviors. For instance scope it means
that suddenly no globals can be added that alias with used member names.
And on the GCC side, all "instance scope" usage would just have parsing
delayed until the end of the struct parisng.
Thanks for the discussion!
-Kees
[1] https://clang.llvm.org/docs/BoundsSafety.html
--
Kees Cook
On Mon, Mar 10, 2025 at 01:03:44PM -0700, Kees Cook wrote:
> Hi! Thanks again for continuing to poke at this. Even with commit
> 1301e18f69ce ("gimple-ssa-warn-access: Adjust maybe_warn_nonstring_arg
> for nonstring multidimensional arrays [PR117178]"), this is still not
> w
truct Y y, y.n)));
> };
>
> i.e, the compiler will lookup “y” inside the current structure, then
> resolving the member access operator “.” as an expression.
>
> Is this correct understanding?
I had the same question, e.g. how is this supposed to be declared:
struct Y {
int n;
int other;
}
struct Z {
int *ints __attribute__ ((counted_by(y.n)));
struct Y y;
};
--
Kees Cook
T_Wrestrict, ovlsiz[1],
+ warning_n (&richloc, OPT_Wrestrict, ovlsiz[1],
"%qD accessing between %wu and %wu bytes "
"at offsets %s and %s may overlap %wu byte "
"at offset %s",
@@ -1664,7 +1666,7 @@ maybe_diag_overlap (location_t loc, gimple *call,
builtin_access &acs)
return true;
}
- warning_n (loc, OPT_Wrestrict, ovlsiz[1],
+ warning_n (&richloc, OPT_Wrestrict, ovlsiz[1],
"%qD accessing %wu or more bytes at offsets %s "
"and %s may overlap %wu byte at offset %s",
"%qD accessing %wu or more bytes at offsets %s "
--
Kees Cook
"counted_by":
https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/
Thanks again!
-Kees
--
Kees Cook
nt_count_seen_by_bdos
ok 12 global.alloc_size_with_bigger_element_count_enforced_by_sanitizer
Test suite is here:
https://github.com/kees/kernel-tools/blob/trunk/fortify/array-bounds.c
--
Kees Cook
n add a new warning option -Wcounted-by to report such user error if
> needed.
>
> What’s your opinion on this?
I think it is correct to allow mismatch between allocation and
counted_by as long as only the least of the two is used. This may be
desirable in a few situations. One example would be a large allocation
that is slowly filled up by the program. I.e. the counted_by member is
slowly increased during runtime (but not beyond the true allocation size).
Of course allocation size is only available in limited situations, so
the loss of that info is fine: we have counted_by for everything else.
--
Kees Cook
On Mon, Jul 17, 2023 at 09:17:48PM +, Qing Zhao wrote:
>
> > On Jul 13, 2023, at 4:31 PM, Kees Cook wrote:
> >
> > In the bug, the problem is that "p" isn't known to be allocated, if I'm
> > reading that correctly?
>
> I think tha
ram to query a pointer when neither A nor B have happened. But
for the first version of the __counted_by infrastructure, the above
limitations seen fine.
For example, maybe __builtin_counted_size(p) (which returns sizeof(*p) +
sizeof(*p->flex_array_member) * p->counted_by_member). Though since
there might be multiple flex array members, maybe this can't work. :)
-Kees
--
Kees Cook
both
> observered allocation and observed access,
> A. from observed allocation (alloc_size): (LENGTH - SIZE_BUMP) * sizeof
> (int)
> B. from observed access (TYPE): LENGTH * sizeof (int)
>*/
>
> /* for MAXIMUM size in the whole object: currently, GCC always used the A.
> */
> expect(__builtin_object_size(p->array, 0), (LENGTH - SIZE_BUMP) *
> sizeof(int));
ok: __builtin_object_size(p->array, 0) == 20
My brain just melted a little, as this is now an under-sized instance of
"p", so we have an incomplete allocation. (I would expect -Warray-bounds
to yell very loudly for this.) But, technically, yes, this looks like
the right calculation.
>
> /* for MAXIMUM size in the sub-object: currently, GCC chose the smaller
> one among these two: B. */
> expect(__builtin_object_size(p->array, 1), (LENGTH - SIZE_BUMP) *
> sizeof(int));
ok: __builtin_object_size(p->array, 1) == 20
Given the under-allocation, yes, this seems correct to me, though,
again, I would expect a compile-time warning. (Or at least, I've seen
-Warray-bounds yell about this kind of thing in the past.)
-Kees
--
Kees Cook
int, bool *);
> +static tree handle_strict_flex_array_attribute (tree *, tree, tree,
> + int, bool *);
Something mangled these patches -- leading blank characters got dropped?
--
Kees Cook
a potential trailing flex array.
If it could be introspected better, FORTIFY could check for the flex
array. For example, instead of using the inconsistent __bos(ptr, 1) for
finding member sizes, it could do something like:
#define __member_size(ptr) \
(__builtin_has_flex_array_p(ptr) ? -1 : \
__builtin_object_size(ptr, 1))
--
Kees Cook
,
as expected, many places in Linux that need to be fixed, and that work
is on-going, guided by this option's results.)
-Kees
--
Kees Cook
zeof(*q) + q->foo * sizeof (q->array) bytes available. The question
>> then is whether q could be pointing to an element of an array of `struct
>> annotated`. Could we ever have a valid array of such structs that have a
>> flex array at the end? Wouldn't it always
On Thu, Aug 03, 2023 at 07:55:54PM +, Qing Zhao wrote:
>
>
> > On Aug 3, 2023, at 1:51 PM, Kees Cook wrote:
> >
> > On August 3, 2023 10:34:24 AM PDT, Qing Zhao wrote:
> >> One thing I need to point out first is, currently, even for regular fixed
> >
builtin_object_size(p->array, 0) with the TYPE of
> the struct fix.
I think it is fine to leave __bos(..., 0) as-is. From the Linux kernel's
use of __bos, we are almost exclusively only interesting the mode 1, not
node 0. :)
--
Kees Cook
nel's macros for the name change and done
build tests with my first pass at "easy" cases for adding counted_by:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=devel/counted_by&id=adc5b3cb48a049563dc673f348eab7b6beba8a9b
Everything is working as expected. :)
-Kees
--
Kees Cook
struct {
int large;
int int_fam[];
};
};
};
https://godbolt.org/z/b1v74Mzhd
i.e.:
struct_size(x, char_fam, 1) < sizeof(*x)
so accessing "large" fails, etc. Yes, it's all horrible, but we have to
deal with this kind of thing in the kernel. :(
--
Kees Cook
; is used uninitialized
[-Wuninitialized]
366 | p->array[0] = 0;
| ^~~
Yay! :)
-Kees
--
Kees Cook
c/passes.cc:2142
with:
struct ffs_buffer {
size_t length;
char *data;
char storage[] __counted_by(length);
};
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
--
Kees Cook
On Wed, Aug 16, 2023 at 10:31:30PM -0700, Kees Cook wrote:
> On Fri, Aug 04, 2023 at 07:44:28PM +, Qing Zhao wrote:
> > This is the 2nd version of the patch, per our discussion based on the
> > review comments for the 1st version, the major changes in this version
>
stall/latest-d/bin/gcc -O2 -c -o /dev/null bug.c
> [opc@qinzhao-ol8u3-x86 108896]$
Great! Thanks. I look forward to v3. For now I'll leave off these 2
annotations in my kernel builds. :)
-Kees
--
Kees Cook
n,
It's been a couple months, and I didn't see any other feedback on this
proposal. I was curious what the status of this work is. Are you able to
attend GNU Cauldron[1] this year? I'd love to see this get some traction
in GCC.
Thanks!
-Kees
[1] https://gcc.gnu.org/wiki/cauldron2023
--
Kees Cook
27;s better to be put into GCC13.
Hi!
This series tests well for me. Getting this landed would be very nice
for the Linux kernel. :)
Are there any additional review notes for it, or is it ready to land?
Thanks!
-Kees
--
Kees Cook
th) {
unsigned long var;
case ..:
...
case ..:
...
...
}
I'm really looking forward to having this available. Thanks again!
-Kees
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/test_stackinit.c
--
Kees Cook
On Thu, Feb 25, 2021 at 12:15:01PM -0600, Qing Zhao wrote:
> > On Feb 24, 2021, at 10:41 PM, Kees Cook wrote:
> > [...]
> > test_stackinit: trailing_hole_none ok
> > test_stackinit: packed_none ok
> > test_stackinit: user ok
> > test_stackinit: failures: 8
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
I assume it's not handling the flex-array happily?
--
Kees Cook
t; >> member;
> >> 2, therefore, -std=c89 + -fstrict-flex-array does not need a warning too.
> >>
> >> ?
> >
> > Yes.
> >
>
> Okay, I am fine with this.
>
> Richard and Kees, what’s your opinion on this?
Agreed: I think it's fine not to warn about these "conflicting" flags in
those cases. It looks like the C standard warnings about flexible arrays
are already hidden behind -Wpedantic, so nothing else is needed, IMO.
Using -fstrict-flex-arrays just enforces that warning. ;)
--
Kees Cook
wants
it to be really strict, they'd just add -Wpedantic.
--
Kees Cook
ewers and everyone else poking at it.
I will go update my Linux Plumbers slides to say "supported" instead of
"proposed". :)
-Kees
--
Kees Cook
nter, and
"NUL" refers to the "null character", so the latter use of NULL should be
"NUL": ... pointers are NULL, strings are NUL filled, and size ...
I mix this up all the time, so apologies if that got introduced by me!
:)
-Kees
> + and indices are 0).
> +
> +
> +
> Debugging formats
>
>
> --
> 1.9.1
>
>
--
Kees Cook
On Wed, Sep 29, 2021 at 02:43:35PM +, Qing Zhao wrote:
> FYI, just committed the change:
>
> https://gcc.gnu.org/gcc-12/changes.html
Looks great to me; thanks! :)
-Kees
--
Kees Cook
Yup! Please report back any testing; that'll help show people are
interested in the feature. :)
--
Kees Cook
Yes, this is very important. This is one of the more common ways memory
content leaks happen in programs (especially the kernel). e.g.:
struct example {
short s;
int i;
};
struct example instance = { .i = foo };
While "s" gets zeroed, the padding may not, and may contain prior memory
contents. Having this be deterministically zero is important for this
feature. If the structure gets byte-copied to a buffer (e.g. syscall,
etc), the padding will go along for the ride.
--
Kees Cook
option).
>
> Personally, I am okay with splitting padding initialization from this current
> patch,
> Kees, what’s your opinion on this? i.e, the current -ftrivial-auto-var-init
> will NOT initialize padding, we will add another option to
> Explicitly initialize padding.
If two new options are needed, that's fine. But "-ftrivial-auto-var-init"
needs to do both (it is _not_ getting fully initialized if padding isn't
included). And would be a behavioral mismatch between Clang and GCC. :)
--
Kees Cook
it quite difficult to
> follow the conversation.
I think the first step is to make sure the MUA is sending "text only"
emails. Then configure the "quoting style" to do the standard "> "-style.
What email client are you using?
--
Kees Cook
initialize that? If not,
> why's other padding important to be initialized?
This isn't a situation that I'm aware of causing real-world problems.
The issues have all come from padding within an addressable object. I
haven't tested Clang's behavior on this (and I have no kernel tests for
this padding), but I do check for trailing padding, like:
struct test_trailing_hole {
char *one;
char *two;
char *three;
char four;
/* "sizeof(unsigned long) - 1" byte padding hole here. */
};
-Kees
--
Kees Cook
.
> Other emails are fine.
>
> > On Jun 8, 2021, at 11:56 AM, Kees Cook wrote:
> >
> > On Tue, Jun 08, 2021 at 09:37:30AM +0200, Richard Biener wrote:
> >> On Mon, 7 Jun 2021, Qing Zhao wrote:
> >>> On Jun 7, 2021, at 2:48 AM, Richard Biener
>
tern init isn't much more of an addition in this series.
> 2. Since “Pattern initialization” is just used for debugging purpose, the
> runtime and code size overhead might not be that
> Important at all, right?
That has been my impression, yes.
Thanks!
-Kees
--
Kees Cook
On Fri, Jun 11, 2021 at 01:04:09PM +0200, Richard Biener wrote:
> On Tue, 8 Jun 2021, Kees Cook wrote:
>
> > On Tue, Jun 08, 2021 at 09:41:38AM +0200, Richard Biener wrote:
> > > On Mon, 7 Jun 2021, Qing Zhao wrote:
> > >
> > > > Hi,
> > > >
odify the patterns so that the offset will be moved into the
> > immediate offset field of the LDR instructions, so currently, the ADD of
> > the offset is always a distinct instruction.
> >
>
> ... and this is no longer true now that I fixed the correctness
> problem. I will be sending out a v2 shortly, so please disregard this
> one for now.
Heh, I hadn't even had a chance to test it, so I'll hold off. :)
Thanks!
-Kees
--
Kees Cook
builds for me, and behaves as expected. I get a working kernel[1],
and have verified[2] that we have per-task canaries for arm32. :) Yay!
Tested-by: Kees Cook
Who's best to review and commit this? Qing, is something you're able to
review?
Thanks!
-Kees
[1]
https://lore.ker
[1] https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565840.html
[2] https://gcc.gnu.org/pipermail/gcc-patches/2021-April/567754.html
--
Kees Cook
On Mon, Jul 12, 2021 at 08:28:55PM +, Qing Zhao wrote:
> > On Jul 12, 2021, at 12:56 PM, Kees Cook wrote:
> > On Wed, Jul 07, 2021 at 05:38:02PM +, Qing Zhao wrote:
> >> This is the 4th version of the patch for the new security feature for GCC.
> >
1 - 100 of 134 matches
Mail list logo
