On Thu, Sep 11, 2025 at 03:04:01PM +, Qing Zhao wrote:
>
>
> > On Sep 10, 2025, at 23:05, Kees Cook wrote:
> >
> > On Tue, Sep 09, 2025 at 06:49:22PM +, Qing Zhao wrote:
> >>
> >>> On Sep 4, 2025, at 20:24, Kees Cook wrote:
> >&g
On Thu, Sep 11, 2025 at 09:29:35AM +0200, Peter Zijlstra wrote:
> On Wed, Sep 10, 2025 at 08:05:11PM -0700, Kees Cook wrote:
>
> > > > +/* Check if a function needs KCFI preamble generation.
> > > > + ALL functions get preambles when -fsanitize=kcfi i
On Tue, Sep 09, 2025 at 06:49:22PM +, Qing Zhao wrote:
>
> > On Sep 4, 2025, at 20:24, Kees Cook wrote:
> > +For indirect call sites:
> > +
> > +- Keeping indirect calls from being merged (see above) by adding a
> > + wrapping type so that equality was test
uld I like a better way to test this stuff. It's really
> > painfully doing it via forcing KCFI address-taking and then finding the
> > typeid in the dump file.
>
> Why not have a builtin that returns the hash for a type?
If this would be acceptable, sure. I would like to be able to examine
the _string_, though, since the hash is just a way to smash the string
into a u32.
-Kees
--
Kees Cook
On Mon, Sep 08, 2025 at 04:36:57PM -0700, Andrew Pinski wrote:
> On Mon, Sep 8, 2025 at 4:24 PM Kees Cook wrote:
> >
> > On Tue, Sep 09, 2025 at 12:13:19AM +0200, Martin Uecker wrote:
> > > Sorry, example should have been this:
> > >
> > > typed
*)[]) -> _ZTSFvPA_iE
How should I adjust this patch's description to say the mangling may
follow a stricter type system?
-Kees
--
Kees Cook
On Mon, Sep 08, 2025 at 05:32:58PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 05, 2025 at 09:19:29AM -0700, Kees Cook wrote:
> > On Fri, Sep 05, 2025 at 10:51:03AM +0200, Peter Zijlstra wrote:
> > > On Thu, Sep 04, 2025 at 05:24:10PM -0700, Kees Cook wrote:
> > > >
On Fri, Sep 05, 2025 at 09:06:41AM +0200, Jakub Jelinek wrote:
> On Thu, Sep 04, 2025 at 05:24:15PM -0700, Kees Cook wrote:
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/kcfi/kcfi-adjacency.c
> > @@ -0,0 +1,73 @@
> > +/* Test KCFI check/transfer adjacency - regr
On Fri, Sep 05, 2025 at 10:51:03AM +0200, Peter Zijlstra wrote:
> On Thu, Sep 04, 2025 at 05:24:10PM -0700, Kees Cook wrote:
> > +- The check-call instruction sequence must be treated a single unit: it
> > + cannot be rearranged or split or optimized. The pattern is that
> &
calls with return values.
(riscv_output_kcfi_insn): New function to emit KCFI assembly.
config/riscv/riscv.md: Add KCFI RTL patterns and hook expansion.
doc/invoke.texi: Document riscv nuances.
Signed-off-by: Kees Cook
---
gcc/config/riscv/riscv-protos.h | 3 +
gcc
On Thu, Sep 04, 2025 at 05:50:45PM -0700, Andrew Pinski wrote:
> On Thu, Sep 4, 2025 at 5:27 PM Kees Cook wrote:
> > +
> > + /* Unknown builtin type - this should never happen in a well-formed C
> > program. */
> > + debug_tree (type);
> > + internal_error
quot; dump file, and the large kcfi testsuite validates
expected mangle strings as part of the type-id validation.
gcc/ChangeLog:
* Makefile.in: Add mangle.o to build.
* mangle.cc: New file. Implement C typeinfo mangling for KCFI.
* mangle.h: New file. Export hash_function_type fu
propagation during
inlining.
* varasm.cc (assemble_start_function): Emit KCFI preambles.
(assemble_external_real): Emit KCFI typeid symbols.
(default_elf_asm_named_section): Handle .kcfi_traps using
SECTION_LINK_ORDER flag.
Signed-off-by: Kees Cook
---
gcc
ith_kcfi): New function.
(arm_maybe_wrap_call_value_with_kcfi): New function.
(arm_output_kcfi_insn): Emit KCFI assembly.
config/arm/arm.md: Add KCFI RTL patterns and hook expansion.
doc/invoke.texi: Document arm32 nuances.
Signed-off-by: Kees Cook
---
gcc/config/ar
terns.
doc/invoke.texi: Document x86 nuances.
Signed-off-by: Kees Cook
---
gcc/config/i386/i386-protos.h | 1 +
gcc/config/i386/i386.h | 3 +-
gcc/config/i386/i386-expand.cc | 21 +-
gcc/config/i386/i386.cc| 118 +
gcc/config/i386/i3
.com/KSPP/linux/issues/369
Kees Cook (7):
mangle: Introduce C typeinfo mangling API
kcfi: Add core Kernel Control Flow Integrity infrastructure
x86: Add x86_64 Kernel Control Flow Integrity implementation
aarch64: Add AArch64 Kernel Control Flow Integrity implementation
arm: Add ARM 32-bit Kern
KCFI assembly.
config/aarch64/aarch64.md: Add KCFI RTL patterns and replace
open-coded branch emission with aarch64_indirect_branch_asm.
doc/invoke.texi: Document aarch64 nuances.
Signed-off-by: Kees Cook
---
gcc/config/aarch64/aarch64-protos.h | 5 ++
gcc/config/aarc
.
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: Indu Bhagat
Cc: Claudiu Zissulescu
On Tue, Aug 26, 2025 at 06:22:31PM +, Qing Zhao wrote:
> Hi, Kees,
>
> > On Aug 26, 2025, at 13:25, Kees Cook wrote:
> >
> > The __attribute__((__copy__)) functionality was crashing when copying
> > sanitizer-related attributes because these attributes
t; In the new module kcfi.cc <http://kcfi.cc/>. such documentation will be
> helpful for current
> review and future maintenance of this feature.
Ah yeah, good idea. I will adapt the commit log.
>
> > On Aug 21, 2025, at 03:26, Kees Cook wrote:
> >
>
.
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
1 - 100 of 193 matches
Mail list logo
