netdev - [Patch net] xfrm: fix reinsert 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]

Date:   Thu,  3 Jan 2019 20:44:13 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     netdev@...r.kernel.org
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        Florian Westphal <fw@...len.de>
Subject: [Patch net] xfrm: fix reinsert in xfrm_hash_rebuild()

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+9d971dd21eb26567036b@...kaller.appspotmail.com
Fixes: 24969facd704 ("xfrm: policy: store inexact policies in an rhashtable")
Cc: Florian Westphal <fw@...len.de>
Signed-off-by: Cong Wang <xiyou.wangcong@...il.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