Re: [PATCH v2 1/2] Introduce flexible array struct helpers

[Keith Packard]

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)

> +/* 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?

-- 
-keith

Attachment: signature.asc
Description: PGP signature