Re: [PATCH ipsec-next v2] xfrm: policy: Restore dir assignments in xfrm_hash_rebuild()

[Nathan Chancellor <nathan@kernel.org>]

Ping? This is still seen on next-20240909.

On Thu, Aug 29, 2024 at 11:14:54AM -0700, Nathan Chancellor wrote:
> Clang warns (or errors with CONFIG_WERROR):
> 
>   net/xfrm/xfrm_policy.c:1286:8: error: variable 'dir' is uninitialized when used here [-Werror,-Wuninitialized]
>    1286 |                 if ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) {
>         |                      ^~~
>   net/xfrm/xfrm_policy.c:1257:9: note: initialize the variable 'dir' to silence this warning
>    1257 |         int dir;
>         |                ^
>         |                 = 0
>   1 error generated.
> 
> A recent refactoring removed some assignments to dir because
> xfrm_policy_is_dead_or_sk() has a dir assignment in it. However, dir is
> used elsewhere in xfrm_hash_rebuild(), including within loops where it
> needs to be reloaded for each policy. Restore the assignments before the
> first use of dir to fix the warning and ensure dir is properly
> initialized throughout the function.
> 
> Fixes: 08c2182cf0b4 ("xfrm: policy: use recently added helper in more places")
> Acked-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Changes in v2:
> - Restore another dir assignment in
>     list_for_each_entry_reverse(policy, ...
>   loop, necessitating a value reload to avoid a stale value (thanks to
>   Florian for the review).
> - Reword commit message slightly based on above change.
> - Pick up Florian's ack.
> - Add 'ipsec-next' subject prefix.
> - Link to v1: https://lore.kernel.org/r/20240829-xfrm-restore-dir-assign-xfrm_hash_rebuild-v1-1-a200865497b1@kernel.org
> ---
>  net/xfrm/xfrm_policy.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 6336baa8a93c..63890c0628c4 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1283,6 +1283,7 @@ static void xfrm_hash_rebuild(struct work_struct *work)
>  		if (xfrm_policy_is_dead_or_sk(policy))
>  			continue;
>  
> +		dir = xfrm_policy_id2dir(policy->index);
>  		if ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) {
>  			if (policy->family == AF_INET) {
>  				dbits = rbits4;
> @@ -1337,6 +1338,7 @@ static void xfrm_hash_rebuild(struct work_struct *work)
>  		hlist_del_rcu(&policy->bydst);
>  
>  		newpos = NULL;
> +		dir = xfrm_policy_id2dir(policy->index);
>  		chain = policy_hash_bysel(net, &policy->selector,
>  					  policy->family, dir);
>  
> 
> ---
> base-commit: 17163f23678c7599e40758d7b96f68e3f3f2ea15
> change-id: 20240829-xfrm-restore-dir-assign-xfrm_hash_rebuild-749ca2264581
> 
> Best regards,
> -- 
> Nathan Chancellor <nathan@kernel.org>
>