netdev - [PATCH ipsec-next v2] xfrm: policy: Restore dir assignments in xfrm_hash

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-next>] [day] [month] [year] [list]

Message-Id: <20240829-xfrm-restore-dir-assign-xfrm_hash_rebuild-v2-1-1cf8958f6e8e@kernel.org>
Date: Thu, 29 Aug 2024 11:14:54 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Steffen Klassert <steffen.klassert@...unet.com>, 
 Herbert Xu <herbert@...dor.apana.org.au>, Florian Westphal <fw@...len.de>
Cc: netdev@...r.kernel.org, llvm@...ts.linux.dev, patches@...ts.linux.dev, 
 Nathan Chancellor <nathan@...nel.org>
Subject: [PATCH ipsec-next v2] xfrm: policy: Restore dir assignments in
 xfrm_hash_rebuild()

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@...len.de>
Signed-off-by: Nathan Chancellor <nathan@...nel.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@...nel.org>