On Mon, Jun 08, 2020 at 08:22:33PM -0500, Gustavo A. R. Silva wrote:
> Add flex_array_size() helper for the calculation of the size, in bytes,
> of a flexible array member contained within an enclosing structure.
>
> Example of usage:
>
> struct something {
> size_t count;
> struct foo items[];
> };
>
> struct something *instance;
>
> instance = kmalloc(struct_size(instance, items, count), GFP_KERNEL);
> instance->count = count;
>
> memcpy(instance->items, source, flex_array_size(instance, items,
> instance->count));
>
> The helper returns SIZE_MAX on overflow instead of wrapping around.
>
> (Additionally replace parameter n with count in struct_size() for
> unification).
>
> Signed-off-by: Gustavo A. R. Silva <gustavo...@kernel.org>
> ---
> Changes in v2:
> - Add further information to the helper documentation.
> - Use same code style as struct_size() for consistency.
>
> include/linux/overflow.h | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index 659045046468f..d2329a914304c 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -304,16 +304,33 @@ static inline __must_check size_t __ab_c_size(size_t a,
> size_t b, size_t c)
> * struct_size() - Calculate size of structure with trailing array.
> * @p: Pointer to the structure.
> * @member: Name of the array member.
> - * @n: Number of elements in the array.
> + * @count: Number of elements in the array.
> *
> * Calculates size of memory needed for structure @p followed by an
> - * array of @n @member elements.
> + * array of @count @member elements.
> *
> * Return: number of bytes needed or SIZE_MAX on overflow.
> */
> -#define struct_size(p, member, n) \
> - __ab_c_size(n, \
> +#define struct_size(p, member, count)
> \
> + __ab_c_size(count,
> \
> sizeof(*(p)->member) + __must_be_array((p)->member),\
> sizeof(*(p)))
I like the "count" change -- "n" can be seen as either count or bytes,
so I like this being distinctly "count".
>
> +/**
> + * flex_array_size() - Calculate size, in bytes, of a flexible array member
> + * within an enclosing structure. Read on for more details.
> + *
> + * @p: Pointer to the structure.
> + * @member: Name of the flexible array member.
> + * @count: Number of elements in the array.
> + *
> + * Calculates size, in bytes, of a flexible array @member of @count elements
> + * within structure @p.
> + *
> + * Return: number of bytes needed or SIZE_MAX on overflow.
> + */
> +#define flex_array_size(p, member, count)
> \
> + array_size(count,
> \
> + sizeof(*(p)->member) + __must_be_array((p)->member))
> +
> #endif /* __LINUX_OVERFLOW_H */
I like it! You mentioned off-list that maybe this could be named
sizeof_flex_array() (like sizeof_field(), etc), and that does seem
attractive. As you also mentioned, it begs the question of renaming
struct_size() to sizeof_struct().
Looking back through the thread[1], it seems the name came from Linus[2],
and was more related to the existing array_size() helper.
So, how about this, as a convention we can use to make a choice:
For things that are strictly constant in size, we can use sizeof_*. For
things that have a dynamic component, we'll use *_size(). So, this patch
is correct as-is.
Acked-by: Kees Cook <keesc...@chromium.org>
(I wonder who's tree this should go via?)
-Kees
[1] https://lore.kernel.org/lkml/20180507113902.gc18...@bombadil.infradead.org/
[2]
https://lore.kernel.org/lkml/ca+55afy8dsrouvtiuu5w+xgok6tyvtjgbh-i8i-y7aiud2e...@mail.gmail.com/
--
Kees Cook
