On Mon, Oct 24, 2022 at 11:35:03AM -0700, Keith Packard wrote:
> Kees Cook <keescook@xxxxxxxxxxxx> writes:
>
> > + * struct flex_array_struct_example {
> > + * ... // arbitrary members
> > + * bounded_flex_array(
> > + * u16, part_count, // count of elements stored in "parts" below.
> > + * u32, parts // flexible array with elements of type u32.
> > + * );
> > + * );
>
> > + * struct flex_array_struct_example {
> > + * ... // position-sensitive members
> > + * // count of elements stored in "parts" below.
> > + * DECLARE_FAS_COUNT(u16, part_count);
> > + * .. // position-sensitive members
> > + * // flexible array with elements of type u32.
> > + * DECLARE_FAS_ARRAY(u32, parts);
> > + * };
>
> I'm sure there's a good reason, but these two macros appear to be doing
> similar things and yet have very different naming conventions. Maybe:
>
> FAS_DECLARE_COUNT(type, name)
> FAS_DECLARE_ARRAY(type, name)
> FAS_DECLARE(size_type, size_name, array_type, array_name)
Well, the custom has been for individual things, call it "DECLARE_*",
and for groups, we went with lower-case macros (e.g. struct_group()).
>
> > +/* For use with flexible array structure helpers, in <linux/flex_array.h> */
> > +#define __DECLARE_FAS_COUNT(TYPE, NAME) \
> > + union { \
> > + TYPE __flex_array_elements_count; \
> > + TYPE NAME; \
> > + }
>
> How often could that second "public" member be 'const'? That would catch
> places which accidentally assign to this field.
>
> For code which does want to write to this field, is it mostly trimming
> data from the end, or does it actually smash in arbitrary values? For
> the former case, would it be helpful to have a test to make sure the
> assigned size isn't larger than the real size (yeah, that would probably
> take an extra field holding the real size), or larger than the current size?
I don't think this'll work within arbitrary struct declarations, but it
would be nice. :)
--
Kees Cook
