Re: [PATCH] tree-object-size: Fix type-1 size for pointers to FAM-containing subobjects [PR126975]
Jakub Jelinek <[email protected]>
| Newsgroups | org.kernel.vger.linux-hardening |
|---|---|
| Message-ID | <aogVH8UtICMP1Ihv@tucnak> |
On Fri, Aug 21, 2026 at 10:56:34AM +0200, Richard Biener wrote: > On Fri, Aug 21, 2026 at 8:30 AM Gustavo A. R. Silva > <[email protected]> wrote: > > > > For a pointer to a subobject whose record/union type ends in a flexible-array > > member (directly, or through its trailing nested struct), addr_object_size() > > walked up to the enclosing object (v = TREE_OPERAND (v, 0)) instead of > > measuring the referenced subobject. __builtin_object_size() and > > __builtin_dynamic_object_size() type 1 therefore returned the whole-object > > size, collapsing type 1 onto type 0 and losing the distinction between > > &p->inner and p. FORTIFY_SOURCE relies on the type-1 distinction, so this > > weakens its bounds checks. > > > > Fix this by computing the size directly from the referenced record/union > > instead of walking up, restoring the type-0/type-1 distinction that Clang > > already implements. > > > > Bootstraped and regtested on x86_64-linux-gnu. > > > > PR tree-optimization/126975 > > > > gcc/ChangeLog: > > > > * tree-object-size.cc (addr_object_size): For a reference to a > > record or union type, compute the object size from the referenced > > subobject instead of walking up to the enclosing object when the > > type recursively includes a flexible array member. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/builtin-object-size-pr101832.c (main): Update the expected > > results of __builtin_object_size (..., 1) queries on subobjects > > whose type contains a flexible array member. > > --- > > .../gcc.dg/builtin-object-size-pr101832.c | 6 +++--- > > gcc/tree-object-size.cc | 18 +++++------------- > > 2 files changed, 8 insertions(+), 16 deletions(-) > > > > diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c > > index 60078e11634..d76286ae454 100644 > > --- a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c > > +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c > > @@ -92,9 +92,9 @@ int main (int argc, char *argv[]) > > outer = (void *)magic1; > > outest = (void *)magic2; > > > > - expect (__builtin_object_size (&outer->a, 1), -1); > > - expect (__builtin_object_size (&outest->b, 1), -1); > > - expect (__builtin_object_size (&outest->b.a, 1), -1); > > + expect (__builtin_object_size (&outer->a, 1), sizeof(outer->a)); > > I think this warrants updating the documentation in extend.texi where > I think the > question is how for > > struct A { > int n; > char data[]; > }; > > struct B { > int m; > struct A a; > }; > > structr B *outer; > > and object &outer->a, how, based on 'type', the enclosing object > size is constrained by its enclosing object. Also for outer->a.data > what the enclosing object is and similar how the constraint from the > enclosing objects are handled. > > I'll note that __builtin_object_size behavior might be in conflict > with constraints set by -fstrict-flex-arrays. IIRC "nested" flex arrays > are a GNU extension, flex arrays not at the end of an (enclusing) > object as well (a particularly bad one). > > You also need to update the toplevel comment of the testcase > which explicitly says your change is wrong. I actually think we don't want to change this, the handling of nested flexible and flexible-like arrays has been completely intentional, it was based on investigation of real-world code and finding some reasonable middle-ground on what we consider already invalid and what we allow as an extension, especially because it occurs a lot in real-world code. What we certainly can change is the behavior when -fstrict-flex-arrays unless it already behaves the expected way, and perhaps also when the flex array has counted_by attribute. Changing this unconditionally will just break a lot of software in the wild when compiled with -O2 -D_FORTIFY_SOURCE=2 or -O2 -D_FORTIFY_SOURCE=3, which is very common. Jakub