linux-kernel - Re: [PATCH v2] Fix initializing a static union variable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 21 Jun 2024 14:27:25 -0700
From: Nick Desaulniers <ndesaulniers@...gle.com>
To: Yabin Cui <yabinc@...gle.com>
Cc: Steffen Klassert <steffen.klassert@...unet.com>, Herbert Xu <herbert@...dor.apana.org.au>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Nathan Chancellor <nathan@...nel.org>, Bill Wendling <morbo@...gle.com>, 
	Justin Stitt <justinstitt@...gle.com>, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v2] Fix initializing a static union variable

On Fri, Jun 21, 2024 at 2:18 PM Yabin Cui <yabinc@...gle.com> wrote:
>
> saddr_wildcard is a static union variable initialized with {}.
>
> Empty brace initialization of union types is unspecified prior to C23,
> and even in C23, it doesn't guarantee zero initialization of all fields
> (see sections 4.5 and 6.2 in
> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2900.htm).
>
> Clang currently only initializes the first field to zero, leaving other
> fields undefined. This can lead to unexpected behavior and optimizations
> that produce random values (with some optimization flags).
> See https://godbolt.org/z/hxnT1PTWo.
>
> The issue has been reported to Clang upstream (
> https://github.com/llvm/llvm-project/issues/78034#issuecomment-2183233517).
> This commit mitigates the problem by avoiding empty brace initialization
> in saddr_wildcard.

Thanks for the patch. The links add a lot more context.

Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>

>
> Fixes: 08ec9af1c062 ("xfrm: Fix xfrm_state_find() wrt. wildcard source address.")
> Signed-off-by: Yabin Cui <yabinc@...gle.com>
>
> ---
>
> Changes in v2:
> - Update commit message to add/update links.
>
> ---
>  net/xfrm/xfrm_state.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 649bb739df0d..9bc69d703e5c 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -1139,7 +1139,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
>                 struct xfrm_policy *pol, int *err,
>                 unsigned short family, u32 if_id)
>  {
> -       static xfrm_address_t saddr_wildcard = { };
> +       static const xfrm_address_t saddr_wildcard;
>         struct net *net = xp_net(pol);
>         unsigned int h, h_wildcard;
>         struct xfrm_state *x, *x0, *to_put;
> --
> 2.45.2.741.gdbec12cfda-goog
>


-- 
Thanks,
~Nick Desaulniers