| Message ID | 20240829-xfrm-restore-dir-assign-xfrm_hash_rebuild-v1-1-a200865497b1@kernel.org (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | xfrm: policy: Restore dir assignment in xfrm_hash_rebuild() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
Nathan Chancellor <nathan@kernel.org> 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. Ugh, my bad. Acked-by: Florian Westphal <fw@strlen.de>
Florian Westphal <fw@strlen.de> wrote: > Nathan Chancellor <nathan@kernel.org> 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. > > Ugh, my bad. > > Acked-by: Florian Westphal <fw@strlen.de> Actually, this fix is incomplete, the assignment needs to be restored in the second loop as well: 1340 chain = policy_hash_bysel(net, &policy->selector, 1341 policy->family, dir); ~~~ Nathan, Steffen, I'll leave it up to you to either do a v2 or a revert.
On Thu, Aug 29, 2024 at 08:02:05PM +0200, Florian Westphal wrote: > Florian Westphal <fw@strlen.de> wrote: > > Nathan Chancellor <nathan@kernel.org> 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. > > > > Ugh, my bad. > > > > Acked-by: Florian Westphal <fw@strlen.de> > > Actually, this fix is incomplete, the assignment needs to be > restored in the second loop as well: > > 1340 chain = policy_hash_bysel(net, &policy->selector, > 1341 policy->family, dir); > ~~~ > > Nathan, Steffen, I'll leave it up to you to either do a v2 or a revert. Ah, good catch. I'll send a v2 shortly with this fixed and your ack picked up. Cheers, Nathan
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 6336baa8a93c..02eb4bd0fde6 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;
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 the assignment of dir because xfrm_policy_is_dead_or_sk() has a dir assignment in it. However, dir is used elsewhere in xfrm_hash_rebuild(), so restore the assignment to fix the warning and ensure dir is initialized throughout the function. Fixes: 08c2182cf0b4 ("xfrm: policy: use recently added helper in more places") Signed-off-by: Nathan Chancellor <nathan@kernel.org> --- net/xfrm/xfrm_policy.c | 1 + 1 file changed, 1 insertion(+) --- base-commit: 17163f23678c7599e40758d7b96f68e3f3f2ea15 change-id: 20240829-xfrm-restore-dir-assign-xfrm_hash_rebuild-749ca2264581 Best regards,