Re: [PATCH] overflow: Support sub-union members in struct_size()

Hi Bence,

As discussed elsewhere, I don't think this is right, but it really
depends on how struct_size_t() is expected to be used. We probably need
better documentation for that...

On Tue, Jul 28, 2026 at 01:49:13PM +0200, Bence Csokas wrote:
> Original definition of struct_size() assumed that the total size of an
> object with a flex-array member can be calculated simply by adding the
> flex-array size to what sizeof() returns. This is however not the case
> if the struct has a union inside it, and `member` refers to a field
> inside that sub-union:
> 
>   struct my_inner {
>     int a;
>     char b[];
>   };
> 
>   struct my_outer {
>     char c;
>     union {
>       struct my_inner inner;
>       long l;
>     };
>   };
> 
>   /* This will give the wrong result */
>   struct_size_t(struct my_outer, inner.b, n_elems);

Can you please explain what's wrong specifically? How are you expecting
to use the result?

My understanding is that struct_size_t() is meant to be used to
determine the size to allocate. Over-estimating by a small margin isn't
a functional issue.

> Replacing sizeof() by the offsetof() of the member fixes this.

That will under-estimate the size of the structure, by ignoring the
static footprint. For your example above, imagine you called:

  struct_size_t(struct my_outer, inner.b, 1) 

Where sizeof(long) == 8, that will provide a result that *smaller* than
sizeof(struct my_outer), missing the last 3 bytes of 'l'.

It's possible that 'l' is the active member at some point in time.  At
allocation we need to allocate *the largest possible size* that will be
used, but we're not guaranteeing that the flexible member is alway used.

Consider a case with a tagged union, where another field is used to
determine the active union member.

For example, something like:

| struct foo_elem {
|	/* some small set of fields */
| 	...
| };
| 
| struct source {
|	/* some large set of fields */
| 	...
| };
| 
| struct foo {
| 	int	nr_elems;
| 	bool	completed;
| 
| 	/* 
| 	 * When !completed, use @source.
| 	 * When completed, results have been placed in @elems.
| 	 */
| 	union {
| 		struct source source;
| 		struct foo_elem elems[];
| 	};
| };

The current behaviour for struct_size[_t]() will ensure sufficient space
is always allocated, whereas what you proposed might not. That could
lead to memory corruption, etc, which would be a worse than
over-allocating.

Mark.

> Existing users specifying non-union members should not be affected.
> 
> Cc: Mark Rutland <[email protected]>
> 
> Signed-off-by: Bence Csokas <[email protected]>
> ---
>  include/linux/overflow.h | 4 ++--
>  include/linux/stddef.h   | 8 ++++++++
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index a8cb6319b4fb..a11c1135f617 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -433,8 +433,8 @@ static __always_inline size_t __must_check size_sub(size_t minuend, size_t subtr
>   */
>  #define struct_size(p, member, count)					\
>  	__builtin_choose_expr(__is_constexpr(count),			\
> -		sizeof(*(p)) + flex_array_size(p, member, count),	\
> -		size_add(sizeof(*(p)), flex_array_size(p, member, count)))
> +		offsetofp((p), member) + flex_array_size(p, member, count),	\
> +		size_add(offsetofp((p), member), flex_array_size(p, member, count)))
>  
>  /**
>   * struct_size_t() - Calculate size of structure with trailing flexible array
> diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> index e1851c50c89b..b8f0dbd2de55 100644
> --- a/include/linux/stddef.h
> +++ b/include/linux/stddef.h
> @@ -32,6 +32,14 @@ enum {
>  #define offsetofend(TYPE, MEMBER) \
>  	(offsetof(TYPE, MEMBER)	+ sizeof_field(TYPE, MEMBER))
>  
> +/**
> + * offsetofp() - Get the offset of a struct field from a pointer to the struct
> + *
> + * @P: Pointer to the structure
> + * @MEMBER: The member within the structure to get the end offset of
> + */
> +#define offsetofp(P, MEMBER) offsetof(typeof(*(P)), MEMBER)
> +
>  /**
>   * struct_group() - Wrap a set of declarations in a mirrored struct
>   *
> 
> base-commit: 62cc90241548d5570ee68e01aaba6506964e9811
> -- 
> 2.54.0
>