.
gcc/testsuite/ChangeLog:
PR c/113264
* gcc.dg/pr113264.c: New test.
* gcc.dg/pr113264-asan-no-instrumentation.c: New test validating
that copied sanitizer attributes prevent ASAN instrumentation.
Signed-off-by: Kees Cook
---
v3: now with the tests actually
On Tue, Aug 26, 2025 at 03:54:13PM +0300, Claudiu Zissulescu-Ianculescu wrote:
> I think the tests are missing.
Argh. I will send a v3...
--
Kees Cook
.
gcc/testsuite/ChangeLog:
PR c/113264
* gcc.dg/pr113264.c: New test.
* gcc.dg/pr113264-asan-no-instrumentation.c: New test validating
that copied sanitizer attributes prevent ASAN instrumentation.
Signed-off-by: Kees Cook
---
v2: Rebased on to latest upstream
will rebase...
> > On Aug 25, 2025, at 11:59, Kees Cook wrote:
> >
> > The __attribute__((__copy__)) functionality was crashing when copying
> > sanitizer-related attributes because these attributes violated the standard
> > GCC attribute infrastructure by storing
/builtin_fls_const.c: New test. Verify that
const attribute enables CSE optimization for mathematical ARC
builtins by checking that duplicate calls are eliminated and
results are optimized to shift operations.
Signed-off-by: Kees Cook
---
.../gcc.target/arc/builtin_fls_const.c
.
gcc/testsuite/ChangeLog:
PR c/113264
* gcc.dg/pr113264.c: New test.
* gcc.dg/pr113264-asan-no-instrumentation.c: New test validating
that copied sanitizer attributes prevent ASAN instrumentation.
Signed-off-by: Kees Cook
---
Cc: Andrew Pinski
---
gcc/asan.h
On Fri, Aug 22, 2025 at 08:29:16PM +, Qing Zhao wrote:
> > On Aug 22, 2025, at 15:02, Kees Cook wrote:
> > Right, and sometimes we have to explicitly perform a no-op
> > address-taking to make sure a symbol gets generated:
> >
> > /*
> > * Force the co
ush by attackers toward the less "low-hanging fruit" of
"data only" attacks (e.g. write to the page tables, etc). Jann Horn has
a particularly sobering write-up about this via memory allocators:
https://googleprojectzero.blogspot.com/2021/10/how-simple-linux-kernel-memory.html
-Kees
--
Kees Cook
On Fri, Aug 22, 2025 at 03:11:16PM +, Qing Zhao wrote:
> > On Aug 21, 2025, at 17:29, Kees Cook wrote:
> > For non-static functions, we cannot know if other compilation units may
> > make indirect calls to a given function, so those functions must always
> > have t
s whole 'long' vs 'long long' trainwreck :/
>
> That is the -fsanitize-cfi-icall-experimental-normalize-integers
> argument for clang (omg so long).
Yup! I forgot to include my "TODO" list in the RFC. It is:
* -fsanitize-cfi-icall-experimental-normalize-integers (but this option
needs a better name if it's going to be supported in GCC too for Rust
compat)
* -fsanitize-kcfi-arity
https://clang.llvm.org/docs/ControlFlowIntegrity.html#fsanitize-kcfi-arity
* cfi_salt function attribute
https://clang.llvm.org/docs/AttributeReference.html#cfi-salt
https://github.com/llvm/llvm-project/commit/aa4805a09052c1b6298718eeb6d30c33dd0d695f
-Kees
--
Kees Cook
> poke around with that UDB patch see what's possible.
I think I have it mostly working to force r11 when doing kcfi and
retpoline now, though I'm seeing a few glitches still. I'll keep working
on it.
--
Kees Cook
I works is by changing the indirect call ABI. Traditionally
> > the indirect call is simply:
> >
> > load-pointer-into-reg
> > call *%reg
> >
> > kCFI changes every function to have a preamble like (with IBT and
> > retpolines and all the modern crap on):
>
> Does “every function” mean all the function in the compilation? Not only the
> function whose address is taken?
I tried to explain the specific logic on how the set of functions getting
preambles is chosen in this other reply:
https://lore.kernel.org/linux-hardening/202508211258.8DEE293@keescook/
If that didn't answer your question, let me know and I'll try again. :)
-Kees
--
Kees Cook
On Thu, Aug 21, 2025 at 01:16:56AM -0700, Andrew Pinski wrote:
> On Thu, Aug 21, 2025 at 12:47 AM Kees Cook wrote:
> > +struct kcfi_target_hooks {
> > + /* Apply architecture-specific masking to type ID. */
> > + uint32_t (*mask_type_id) (uint32_t type_id);
> > +
On Thu, Aug 21, 2025 at 07:14:31PM +, Qing Zhao wrote:
>
>
> > On Aug 21, 2025, at 12:16, Kees Cook wrote:
> >
> >
> >>> + else if (TREE_CODE (fntype_or_fndecl) == FUNCTION_DECL)
> >>> +{
> >>> + tree fndecl = fntype_
h Clang (e.g. Linux kernel CFI, first LLVM-CFI and now
KCFI, has been deployed on all Android phones and all Chrome OS devices
since 2018.)
I have been trying to find someone to work on KCFI in GCC since 2018,
(see my annual arm-waving at the Linux Plumbers conference Toolchain
track) and other than Dan Li who took a stab at it for aarch64 in 2023,
no one has stepped up to do it, so I thought I'd finally try tackling
it. :)
-Kees
--
Kees Cook
On Thu, Aug 21, 2025 at 11:46:17AM -0700, Kees Cook wrote:
> On Thu, Aug 21, 2025 at 11:29:35AM +0200, Peter Zijlstra wrote:
> > On Thu, Aug 21, 2025 at 12:26:37AM -0700, Kees Cook wrote:
> > > Build and run tested on x86_64 Linux kernel with various CPU errata
> > >
On Thu, Aug 21, 2025 at 11:29:35AM +0200, Peter Zijlstra wrote:
> On Thu, Aug 21, 2025 at 12:26:37AM -0700, Kees Cook wrote:
> > Implement x86_64-specific KCFI backend:
> >
> > - Function preamble generation with type IDs positioned at -(4+prefix_nops)
> > offse
On Thu, Aug 21, 2025 at 12:59:06AM -0700, Andrew Pinski wrote:
> On Thu, Aug 21, 2025 at 12:41 AM Kees Cook wrote:
>
> > To support the KCFI type-id which needs to convert unique function
> > prototypes into unique 32-bit values, add a subset of the Itanium C++
> > mangli
from which I could extract the "func" fndecl.
It would have been so much nicer to build the selftest directly into
mangle.cc here, but I couldn't figure it out. Instead, later patches
create a "kcfi" dump file, and the large kcfi testsuite validates
expected mangle strings
!
-Kees
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107048
https://github.com/KSPP/linux/issues/369
Kees Cook (7):
sanitizer: Expand sanitizer flag from 32-bit to 64-bit
mangle: Introduce C typeinfo mangling API
kcfi: Add core Kernel Control Flow Integrity infrastructure
x86: Add x86_64 Kern
()
seems like a hack. I couldn't find a better place to do this.
Build and run tested on x86_64 Linux kernel with various CPU errata
handling alternatives and FineIBT.
Signed-off-by: Kees Cook
---
gcc/config/i386/i386-protos.h | 4 +
gcc/config/i386/i386-options.cc | 3 +
gcc/config/i386
eck-gcc RUNTESTFLAGS='kcfi.exp'
Signed-off-by: Kees Cook
---
gcc/testsuite/gcc.dg/kcfi/kcfi-aarch64-esr.c | 36 +
gcc/testsuite/gcc.dg/kcfi/kcfi-adjacency.c| 83 ++
gcc/testsuite/gcc.dg/kcfi/kcfi-basics.c | 83 ++
gcc/testsuite/gcc.dg/kcfi/kcfi-call-sharing.c | 75 +
4.
Signed-off-by: Kees Cook
---
gcc/config/aarch64/aarch64-protos.h | 4 +
gcc/config/aarch64/aarch64.cc | 112 +++
gcc/config/aarch64/aarch64.md | 137
gcc/doc/invoke.texi | 14 +++
4 files changed, 267 insertions(+)
(I am still building a proper
risc-v emulation setup). Run tested via userspace binaries.
Signed-off-by: Kees Cook
---
gcc/config/riscv/riscv-protos.h | 1 +
gcc/config/riscv/riscv.cc | 157
gcc/config/riscv/riscv.md | 49 ++
gcc/doc
rates the 32-bit hash value, using
compute_kcfi_type_id() and kcfi_hash_string() to hook to the mangling
API. The hash is FNV-1a right now: it doesn't need secrecy. It could be
replaced with any hash, though the hash will need to be coordinated
with Rust, which implements the K
er function
handling
* dwarf2asm.cc: Update save_flag_sanitize for ASan-disabled indirect
constants
Is using "unsigned long long" correct here, or is switching to
"uint64_t" preferred?
Signed-off-by: Kees Cook
---
gcc/asan.h| 4 +--
gcc/c-family/c-common.h
alidated this works with my patched Linux tree with a
bunch of new annotations that take advantage of the new coverage.
--
Kees Cook
stics, yay!)
-Kees
[1] https://gcc.gnu.org/pipermail/gcc-patches/2025-June/687924.html
--
Kees Cook
o macros: __counted_by() for
flexible array members, and __counted_by_ptr() for pointer members.
Adding __counted_by_expr() is no problem! :)
And in a decade when all the flavors are in all supported compiler
versions we can do a quick treewide replacement.
--
Kees Cook
) in all the shared headers? C++ uses will
ignore it, and C uses will apply the attributes.
It seems weird to me that Clang needs to solve how -fbounds-safety works
with C++ if it's not for _use_ in C++. I feel like I'm missing something
about features that can't be macro-ified or s
On Wed, Jul 23, 2025 at 06:40:07PM +0200, Martin Uecker wrote:
> Am Mittwoch, dem 23.07.2025 um 00:30 -0700 schrieb Kees Cook:
> > On Wed, Jul 23, 2025 at 07:47:11AM +0200, Martin Uecker wrote:
> ...
>
> >
> > How would GCC want to define the syntax for expressions
On Wed, Jul 23, 2025 at 04:28:36PM +, Qing Zhao wrote:
>
>
> > On Jul 23, 2025, at 03:30, Kees Cook wrote:
> >
> >
> > How would GCC want to define the syntax for expressions here? I still
> > think it should be possible to wire up something that matc
ze or is a byte count instead of element count)
- making calls to helper functions
For helper functions, the most common need is doing endian conversions
(e.g. for protocol (de)serializing, where a length is stored in a
different byte order than the native CPU byte order):
struct S {
struct header hdr;
__be32 bytes;
struct info array[] __counted_by(be32_to_cpu(bytes) / sizeof(struct info));
};
--
Kees Cook
as ended)
2) using a callback for expressions (no late parsing needed)
I'm well aware that Apple's implementation will not do either of these,
but I'm confident Clang can support the additional syntax -- it should
be possible to provide both, especially since it would be a "GCC
compatibility" issue. :)
-Kees
--
Kees Cook
://gcc.gnu.org/pipermail/gcc-patches/2025-July/689754.html
[9]
https://discourse.llvm.org/t/rfc-bounds-safety-in-c-syntax-compatibility-with-gcc/85885/39
--
Kees Cook
it off (kernel/padata.o):
$ make -j$CPUS KCFLAGS="$FLAGS" fs/overlayfs/util.o
$ make -j$CPUS KCFLAGS="$FLAGS" drivers/net/ethernet/mellanox/mlx4/alloc.o
$ make -j$CPUS KCFLAGS="$FLAGS" drivers/pinctrl/mediatek/pinctrl-airoha.o
If you prefer, I can also do this once the new implementation is posted. :)
-Kees
--
Kees Cook
uirement.
Agreed -- this is a task for hardware (i.e. memory tagging, etc).
> It's not wrong, because __bdos does not validate a pointer. They key
> difference from the example I cited though, is that in this case the size
> expression does not *create* a dereference of a pointer. With this patch,
> __bdos will start doing that, which is the risky proposition.
If not __bdos, I would like something that _does_.
> If we say that, it could indeed give us the ability to dereference without
> validating even for NULL, although we'll still need it to avoid a case where
> passes later assume validity of the pointer based on the dereference __bdos
> generated.
That's reasonable, yes. Use by __bdos shouldn't change the sense of
validity.
-Kees
--
Kees Cook
On Thu, Jun 05, 2025 at 03:52:21PM -0700, Kees Cook wrote:
> On Mon, May 19, 2025 at 11:23:34AM -0700, Kees Cook wrote:
> > On Fri, May 16, 2025 at 01:34:14PM +, Qing Zhao wrote:
> > > Adding -fdiagnotics-details into GCC to provide more hints to the
> > > end use
On Mon, May 19, 2025 at 11:23:34AM -0700, Kees Cook wrote:
> On Fri, May 16, 2025 at 01:34:14PM +, Qing Zhao wrote:
> > Adding -fdiagnotics-details into GCC to provide more hints to the
> > end users on how the warnings come from, in order to help the user
> > to locate
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
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 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:
> &
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 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:
> &
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
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
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
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
};
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
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
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 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
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
ing this
available. I'll give it a spin. :)
--
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
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).
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
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
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
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
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
_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
> 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
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
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
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
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
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)
> >
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
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
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
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
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
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
l:2 xpass:0 skip:0 error:0
Thanks!
-Kees
--
Kees Cook
lds the Linux kernel where we have almost 300
instances of "counted_by" attributes.
Yay!
-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,
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
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.
> >>
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
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
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
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
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
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,
>
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 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
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 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 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
> &
dexes? I'd find that kind of awkward for the kernel... but I feel like
I've misunderstood something. :)
-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
is be a
separate warning the kernel can turn off initially?
-Kees
--
Kees Cook
tribute__(optimize ("wrapv"))
>
> To mark the specific function to be wrapped around.
>
> However, this attribute does not work for linux kernel due to the following
> reason:
>
> Attribute optimize should be only used for debugging purpose;
> The kernel has banned its usage;
>
> So, a new attribute was requested from Linux kernel security:
>
> request wrap-around behavior for specific function (PR102317)
> __attribute__((wrapv))
>
> Is this request reasonable?
After working through this discussion, I'd say it's likely more helpful
to have a way to disable the sanitizers for a given function (or
variable). i.e. The goal for the kernel would that untrapped wrap-around
would be the very rare exception. e.g. our refcount_t implementation:
https://elixir.bootlin.com/linux/v6.5/source/include/linux/refcount.h#L200
Then we can continue to build the kernel with -fno-strict-overflow (to
avoid UB), but gain sanitizer coverage for all run-time wraps, except
for the very few places where we depend on it. Getting there will also
take some non-trivial refactoring on our end, but without the sanitizers
we're unlikely to find them all.
--
Kees Cook
doing things like "if (ulong + offset < ulong) { ... }":
https://elixir.bootlin.com/linux/v6.5/source/drivers/crypto/axis/artpec6_crypto.c#L1187
This is easy for a static analyzer to find and we can replace it with a
non-wrapping test (e.g. __builtin_add_overflow()), but we'll not find
them all immediately, especially for the signed and pointer cases.
So, I need to retain the "everything wraps" behavior while still being
able to detect when it happens.
--
Kees Cook
id=80998
Ah, thanks for the link! And checking it just now it seems like Clang's
implementation doesn't work. Fun times.
-Kees
--
Kees Cook
1 - 100 of 173 matches
Mail list logo
