Re: [PATCH] Fix initializing a static union variable

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 6/20/24 20:17, Yabin Cui wrote:
> saddr_wildcard is a static union variable initialized with {}.
> But c11 standard doesn't guarantee initializing all fields as
> zero for this case. As in https://godbolt.org/z/rWvdv6aEx,
> clang only initializes the first field as zero, but the bits
> corresponding to other (larger) members are undefined.

Nice finding!

You have to add a Fixes tag, perhaps:
Fixes: 08ec9af1c062 ("xfrm: Fix xfrm_state_find() wrt. wildcard source 
address.")

And the other thing is that I would change the order in the union, to
have largest element as the first. Would be best to also add such check
into static analysis tool/s.

> 
> Signed-off-by: Yabin Cui <yabinc@google.com>
> ---
>   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;