On Thu, 28 Jul 2022, Kees Cook wrote:
> On Thu, Jul 28, 2022 at 07:26:57AM +0000, Richard Biener wrote:
> > On Tue, 19 Jul 2022, Qing Zhao wrote:
> > > [...]
> > > +@cindex @code{strict_flex_array} variable attribute
> > > +@item strict_flex_array (@var{level})
> > > +The @code{strict_flex_array} attribute should be attached to the trailing
> > > +array field of a structure. It specifies the level of strictness of
> > > +treating the trailing array field of a structure as a flexible array
> > > +member. @var{level} must be an integer betwen 0 to 3.
> > > +
> > > +@var{level}=0 is the least strict level, all trailing arrays of
> > > structures
> > > +are treated as flexible array members. @var{level}=3 is the strictest
> > > level,
> > > +only when the trailing array is declared as a flexible array member per
> > > C99
> > > +standard onwards ([]), it is treated as a flexible array member.
> >
> > How is level 3 (thus -fstrict-flex-array) interpreted when you specify
> > -std=c89? How for -std=gnu89?
>
> To me, it makes sense that either c99 is required (most sane to me)
> or it would disable flexible arrays entirely (seems an unlikely combo to
> be useful).
>
> >
> > > +
> > > +There are two more levels in between 0 and 3, which are provided to
> > > support
> > > +older codes that use GCC zero-length array extension ([0]) or one-size
> > > array
> > > +as flexible array member ([1]):
> > > +When @var{level} is 1, the trailing array is treated as a flexible array
> > > member
> > > +when it is declared as either "[]", "[0]", or "[1]";
> > > +When @var{level} is 2, the trailing array is treated as a flexible array
> > > member
> > > +when it is declared as either "[]", or "[0]".
> >
> > Given the above does adding level 2 make sense given that [0] is a GNU
> > extension?
>
> Level 1 removes the general "all trailing arrays are flex arrays" logic, but
> allows the 2 common "historical" fake flex array styles ("[1]" and "[0]").
> Level 2 additionally removes the "[1]" style.
> Level 3 additionally removes the "[0]" style.
>
> I don't understand how "[0]" being a GNU extension matters here for
> level 2 -- it's dropping "[1]". And for level 3, the point is to defang
> the GNU extension of "[0]" to no longer mean "flexible array", and
> instead only mean "zero sized member" (as if it were something like
> "struct { } no_size;").
>
> Note that for the Linux kernel, we only care about level 3, but could
> make do with level 2. We need to purge all the "fake" flexible array usage
> so we can start building a sane set of behaviors around array bounds
> that are reliably introspectable.
Note we've seen "historical" fake flex arrays like
struct X { int n; char str[4]; }; used by people being extra clever
(or careful? char str[1] would not be a flex array since there's
a padding "member" behind it?!) in handling the padding.
I was just worried in confusing people too much. Given
-fstrict-flex-arrays enables level 3 should we warn with -std=c89
that it disables all flex arrays? I think it should at least be
documented somehow.
> As a related bit of feature creep, it would be great to expose something
> like __builtin_has_flex_array_p() so FORTIFY could do a better job
> filtering __builtin_object_size() information.
>
> Given:
>
> struct inside {
> int foo;
> int bar;
> unsigned long items[];
> };
>
> struct outside {
> int a;
> int b;
> struct inside inner;
> };
>
> The follow properties are seen within, for example:
>
> void stuff(struct outside *outer, struct inside *inner)
> {
> ...
> }
>
> __builtin_object_size(&outer->inner, 1) == 8
> __builtin_object_size(inner, 1) == -1
>
> (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832)
I think that would be a bug in bos worth fixing.
> So things like FORTIFY misfire on &outer->inner, as it's _not_ actually
> 8 bytes -- it has 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))
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
