Messages in this thread | | | Date | Wed, 26 Oct 2022 13:33:47 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v2 1/2] Introduce flexible array struct helpers |
| |
On Mon, Oct 24, 2022 at 11:35:03AM -0700, Keith Packard wrote: > Kees Cook <keescook@chromium.org> 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
| |