xfrm_hash_rebuild() re-inserts existing policies into the hashtables,
so it should not insert a same policy in the same place twice. This
means we have to pass excl==1 to xfrm_policy_inexact_insert() and ignore
the -EEXIST error. Otherwise we end up having an entry in the hashtable
points to itself, which leads to a use-after-free as reported by syzbot.
Inside xfrm_policy_inexact_insert(), xfrm_policy_insert_list() could
only return either a NULL pointer, a valid non-NULL pointer, or an error
pointer (-EEXIST) when excl==1. Testing delpol && excl for -EEXIST
is incorrect as it could return a valid pointer for excl case too,
testing IS_ERR(delpol) is correct.
Reported-and-tested-by: syzbot+9d971dd21eb265670...@syzkaller.appspotmail.com
Fixes: 24969facd704 ("xfrm: policy: store inexact policies in an rhashtable")
Cc: Florian Westphal <f...@strlen.de>
Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
---
net/xfrm/xfrm_policy.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 934492bad8e0..c77c44b975ca 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1193,9 +1193,9 @@ xfrm_policy_inexact_insert(struct xfrm_policy *policy, u8
dir, int excl)
}
delpol = xfrm_policy_insert_list(chain, policy, excl);
- if (delpol && excl) {
+ if (IS_ERR(delpol)) {
__xfrm_policy_inexact_prune_bin(bin, false);
- return ERR_PTR(-EEXIST);
+ return delpol;
}
chain = &net->xfrm.policy_inexact[dir];
@@ -1314,9 +1314,10 @@ static void xfrm_hash_rebuild(struct work_struct *work)
chain = policy_hash_bysel(net, &policy->selector,
policy->family, dir);
if (!chain) {
- void *p = xfrm_policy_inexact_insert(policy, dir, 0);
+ void *p = xfrm_policy_inexact_insert(policy, dir, 1);
- WARN_ONCE(IS_ERR(p), "reinsert: %ld\n", PTR_ERR(p));
+ if (IS_ERR(p) && PTR_ERR(p) != -EEXIST)
+ WARN_ONCE(1, "reinsert fails: %ld\n",
PTR_ERR(p));
continue;
}
--
2.19.2
