[PATCH] overflow: Match arithmetic in both parts of __builtin_choose_expr

[PATCH] overflow: Match arithmetic in both parts of __builtin_choose_expr

[Bill Wendling]

The two parts of the '__builtin_choose_expr()' had different arithmetic
expressions. This patch correct it so that both parts calculate:

  COUNT * sizeof(*MEMBER) + __must_be_array(MEMBER)

In practice, the call to '__must_be_array' is meant to assert if
'MEMBER' isn't an array and return '0' otherwise. However, symmetry is
less prone to subtle errors (and far more satisfying).

Fixes: 230f6fa2c1db ("overflow: Provide constant expression struct_size")
Signed-off-by: Bill Wendling <morbo@google.com>
---
 include/linux/overflow.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index a8cb6319b4fb..265ec265aa3e 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -418,7 +418,8 @@ static __always_inline size_t __must_check size_sub(size_t minuend, size_t subtr
 #define flex_array_size(p, member, count)				\
 	__builtin_choose_expr(__is_constexpr(count),			\
 		(count) * sizeof(*(p)->member) + __must_be_array((p)->member),	\
-		size_mul(count, sizeof(*(p)->member) + __must_be_array((p)->member)))
+		size_add(size_mul(count, sizeof(*(p)->member)),		\
+			 __must_be_array((p)->member)))
 
 /**
  * struct_size() - Calculate size of structure with trailing flexible array.
-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH] overflow: Match arithmetic in both parts of __builtin_choose_expr

[Kees Cook]

On Mon, Oct 12, 2026 at 08:59:19PM +0000, Bill Wendling wrote:
> The two parts of the '__builtin_choose_expr()' had different arithmetic
> expressions. This patch correct it so that both parts calculate:
> 
>   COUNT * sizeof(*MEMBER) + __must_be_array(MEMBER)
> 
> In practice, the call to '__must_be_array' is meant to assert if
> 'MEMBER' isn't an array and return '0' otherwise. However, symmetry is
> less prone to subtle errors (and far more satisfying).
> 
> Fixes: 230f6fa2c1db ("overflow: Provide constant expression struct_size")

A Fixes trailer implies a bug being solved. What problem is there here?
I only see downsides to this change: this adds even more layers of
macros and inlinings to gain the size_add() for a constant integer 0.

?

> -		size_mul(count, sizeof(*(p)->member) + __must_be_array((p)->member)))
> +		size_add(size_mul(count, sizeof(*(p)->member)),		\
> +			 __must_be_array((p)->member)))

-- 
Kees Cook

Re: [PATCH] overflow: Match arithmetic in both parts of __builtin_choose_expr

[Bill Wendling]

On Mon, Oct 12, 2026 at 8:30 PM Kees Cook <kees@kernel.org> wrote:
>
> On Mon, Oct 12, 2026 at 08:59:19PM +0000, Bill Wendling wrote:
> > The two parts of the '__builtin_choose_expr()' had different arithmetic
> > expressions. This patch correct it so that both parts calculate:
> >
> >   COUNT * sizeof(*MEMBER) + __must_be_array(MEMBER)
> >
> > In practice, the call to '__must_be_array' is meant to assert if
> > 'MEMBER' isn't an array and return '0' otherwise. However, symmetry is
> > less prone to subtle errors (and far more satisfying).
> >
> > Fixes: 230f6fa2c1db ("overflow: Provide constant expression struct_size")
>
> A Fixes trailer implies a bug being solved. What problem is there here?
> I only see downsides to this change: this adds even more layers of
> macros and inlinings to gain the size_add() for a constant integer 0.
>
> ?
>
I wasn't sure about the Fixes, sorry about that. Why not put the "+
__must_be_array" stuff outside of the `size_mul` instead, since it's
only there to catch an error? So something like

  (__builtin_choose_expr(<pred>, <true>, <false>) + __must_be_array(...))

It simplifies the macro itself and makes it explicit that the
__must_be_array stuff isn't useful for the actual value.
?

> > -             size_mul(count, sizeof(*(p)->member) + __must_be_array((p)->member)))
> > +             size_add(size_mul(count, sizeof(*(p)->member)),         \
> > +                      __must_be_array((p)->member)))
>
> --
> Kees Cook

[PATCH v2] overflow: Match arithmetic in both parts of __builtin_choose_expr

[Bill Wendling]

The two parts of '__builtin_choose_expr()' had different arithmetic
expressions. This patch corrects it so that both parts calculate:

  COUNT * sizeof(*MEMBER) + __must_be_array(MEMBER)

In practice, '__must_be_array' is meant to assert if 'MEMBER' isn't an
array and return '0' otherwise. However, symmetry is less prone to
subtle errors (and far more satisfying).

Signed-off-by: Bill Wendling <morbo@google.com>
---
v2: Don't use 'size_add', which just adds more complicated macro cruft to the
    expression. Instead, just adjust the parentheses.
---
 include/linux/overflow.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index a8cb6319b4fb..d0f04436343e 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -418,7 +418,7 @@ static __always_inline size_t __must_check size_sub(size_t minuend, size_t subtr
 #define flex_array_size(p, member, count)				\
 	__builtin_choose_expr(__is_constexpr(count),			\
 		(count) * sizeof(*(p)->member) + __must_be_array((p)->member),	\
-		size_mul(count, sizeof(*(p)->member) + __must_be_array((p)->member)))
+		size_mul(count, sizeof(*(p)->member)) + __must_be_array((p)->member))
 
 /**
  * struct_size() - Calculate size of structure with trailing flexible array.
-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH v2] overflow: Match arithmetic in both parts of __builtin_choose_expr

[David Laight]

On Tue, 13 Oct 2026 20:31:46 +0000
Bill Wendling <morbo@google.com> wrote:

> The two parts of '__builtin_choose_expr()' had different arithmetic
> expressions. This patch corrects it so that both parts calculate:
> 
>   COUNT * sizeof(*MEMBER) + __must_be_array(MEMBER)
> 
> In practice, '__must_be_array' is meant to assert if 'MEMBER' isn't an
> array and return '0' otherwise. However, symmetry is less prone to
> subtle errors (and far more satisfying).
> 
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> v2: Don't use 'size_add', which just adds more complicated macro cruft to the
>     expression. Instead, just adjust the parentheses.
> ---
>  include/linux/overflow.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index a8cb6319b4fb..d0f04436343e 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -418,7 +418,7 @@ static __always_inline size_t __must_check size_sub(size_t minuend, size_t subtr
>  #define flex_array_size(p, member, count)				\
>  	__builtin_choose_expr(__is_constexpr(count),			\
>  		(count) * sizeof(*(p)->member) + __must_be_array((p)->member),	\
> -		size_mul(count, sizeof(*(p)->member) + __must_be_array((p)->member)))
> +		size_mul(count, sizeof(*(p)->member)) + __must_be_array((p)->member))

Why does this use __builtin_choose_expr().
The types of the two expressions are the same so it might as well use ?:

Also doesn't this optimisation apply to all uses of size_mul() (aka saturating_mul()).
Presumably relying on the compiler to report the compilation time numeric overflow.

David

>  
>  /**
>   * struct_size() - Calculate size of structure with trailing flexible array.