* Re: [PATCH] overflow: Support sub-union members in struct_size()
2026-07-28 11:49 [PATCH] overflow: Support sub-union members in struct_size() Bence Csokas
@ 2026-07-28 18:57 ` Gustavo A. R. Silva
2026-08-06 17:24 ` Mark Rutland
1 sibling, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2026-07-28 18:57 UTC (permalink / raw)
To: Bence Csokas, linux-hardening, linux-kernel
Cc: Mark Rutland, Kees Cook, Gustavo A. R. Silva
On 7/28/26 05:49, 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);
>
> Replacing sizeof() by the offsetof() of the member fixes this.
>
> Existing users specifying non-union members should not be affected.
This actually affects users of flexible structs that have tail padding[1].
-Gustavo
[1] https://godbolt.org/z/KPq59EYej
>
> Cc: Mark Rutland <mark.rutland@arm.com>
>
> Signed-off-by: Bence Csokas <bence.csokas@arm.com>
> ---
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] overflow: Support sub-union members in struct_size()
2026-07-28 11:49 [PATCH] overflow: Support sub-union members in struct_size() Bence Csokas
2026-07-28 18:57 ` Gustavo A. R. Silva
@ 2026-08-06 17:24 ` Mark Rutland
1 sibling, 0 replies; 3+ messages in thread
From: Mark Rutland @ 2026-08-06 17:24 UTC (permalink / raw)
To: Bence Csokas
Cc: linux-hardening, linux-kernel, Kees Cook, Gustavo A. R. Silva
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 <mark.rutland@arm.com>
>
> Signed-off-by: Bence Csokas <bence.csokas@arm.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread