netdev - Fw:[Bug 70471] xfrm policy node will double unlink.

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: <tencent_0D7FA5355C7998793C100B23@qq.com>
Date:	Tue, 18 Feb 2014 10:55:57 +0800
From:	"Xianpeng Zhao" <673321875@...com>
To:	"netdev" <netdev@...r.kernel.org>
Cc:	"alan" <alan@...rguk.ukuu.org.uk>
Subject: Fw:[Bug 70471] xfrm policy node will double unlink.

Hi Group,
     I found a problem about xfrm policy.

     In corner case, xfrm policy node will be double unlinked from the list.

    The scenario like this:
    In thread context, After removed the node from list, before remove the xfrm policy expire timer. At this point, a timer interrupt come, and call the run_timer_softirq to execute the xfrm_policy_timer to remove the expired policy node; because this policy node had already removed from list. this remove will cause the node double unlinked.

     I have done such patch to protect this case. I am not sure here the policy->walk.dead means. From the name, I think it mark the policy node was dead. Maybe I use this flag is not correct, please the expert correct me if I am wrong. More detail information can reference bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70471

        my patch:
--- net/xfrm/xfrm_policy.c_old	2014-02-10 10:18:28.421504317 +0800
+++ net/xfrm/xfrm_policy.c	2014-02-10 10:19:01.661503334 +0800
@@ -330,7 +330,6 @@ static void xfrm_queue_purge(struct sk_b
 
 static void xfrm_policy_kill(struct xfrm_policy *policy)
 {
-	policy->walk.dead = 1;
 
 	atomic_inc(&policy->genid);
 
@@ -1156,6 +1155,7 @@ static struct xfrm_policy *__xfrm_policy
 	if (hlist_unhashed(&pol->bydst))
 		return NULL;
 
+	pol->walk.dead = 1;
 	hlist_del(&pol->bydst);
 	hlist_del(&pol->byidx);
 	list_del(&pol->walk.all);

------------------
Best Regards
Xianpeng