diff mbox series

[net-next,1/2] net_sched: fix a race condition in tcindex_destroy()

Message ID 20190216185827.21535-1-xiyou.wangcong@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series [net-next,1/2] net_sched: fix a race condition in tcindex_destroy() | expand

Commit Message

Cong Wang Feb. 16, 2019, 6:58 p.m. UTC 
(cherry picked from commit 8015d93ebd27484418d4952284fd02172fa4b0b2)

tcindex_destroy() invokes tcindex_destroy_element() via
a walker to delete each filter result in its perfect hash
table, and tcindex_destroy_element() calls tcindex_delete()
which schedules tcf RCU works to do the final deletion work.
Unfortunately this races with the RCU callback
__tcindex_destroy(), which could lead to use-after-free as
reported by Adrian.

Fix this by migrating this RCU callback to tcf RCU work too,
as that workqueue is ordered, we will not have use-after-free.

Note, we don't need to hold netns refcnt because we don't call
tcf_exts_destroy() here.

Fixes: 27ce4f05e2ab ("net_sched: use tcf_queue_work() in tcindex filter")
Reported-by: Adrian <bugs@abtelecom.ro>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/cls_tcindex.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Comments

David Miller Feb. 21, 2019, 4:11 a.m. UTC | #1 
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Sat, 16 Feb 2019 10:58:26 -0800

> (cherry picked from commit 8015d93ebd27484418d4952284fd02172fa4b0b2)
> 
> tcindex_destroy() invokes tcindex_destroy_element() via
> a walker to delete each filter result in its perfect hash
> table, and tcindex_destroy_element() calls tcindex_delete()
> which schedules tcf RCU works to do the final deletion work.
> Unfortunately this races with the RCU callback
> __tcindex_destroy(), which could lead to use-after-free as
> reported by Adrian.
> 
> Fix this by migrating this RCU callback to tcf RCU work too,
> as that workqueue is ordered, we will not have use-after-free.
> 
> Note, we don't need to hold netns refcnt because we don't call
> tcf_exts_destroy() here.
> 
> Fixes: 27ce4f05e2ab ("net_sched: use tcf_queue_work() in tcindex filter")
> Reported-by: Adrian <bugs@abtelecom.ro>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied.
diff mbox series

Patch

diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index e1981628047b..81a433ae31b3 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -48,7 +48,7 @@  struct tcindex_data {
 	u32 hash;		/* hash table size; 0 if undefined */
 	u32 alloc_hash;		/* allocated size */
 	u32 fall_through;	/* 0: only classify if explicit match */
-	struct rcu_head rcu;
+	struct rcu_work rwork;
 };
 
 static inline int tcindex_filter_is_set(struct tcindex_filter_result *r)
@@ -229,9 +229,11 @@  static int tcindex_destroy_element(struct tcf_proto *tp,
 	return tcindex_delete(tp, arg, &last, false, NULL);
 }
 
-static void __tcindex_destroy(struct rcu_head *head)
+static void tcindex_destroy_work(struct work_struct *work)
 {
-	struct tcindex_data *p = container_of(head, struct tcindex_data, rcu);
+	struct tcindex_data *p = container_of(to_rcu_work(work),
+					      struct tcindex_data,
+					      rwork);
 
 	kfree(p->perfect);
 	kfree(p->h);
@@ -258,9 +260,11 @@  static int tcindex_filter_result_init(struct tcindex_filter_result *r)
 	return tcf_exts_init(&r->exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
 }
 
-static void __tcindex_partial_destroy(struct rcu_head *head)
+static void tcindex_partial_destroy_work(struct work_struct *work)
 {
-	struct tcindex_data *p = container_of(head, struct tcindex_data, rcu);
+	struct tcindex_data *p = container_of(to_rcu_work(work),
+					      struct tcindex_data,
+					      rwork);
 
 	kfree(p->perfect);
 	kfree(p);
@@ -480,7 +484,7 @@  tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 	}
 
 	if (oldp)
-		call_rcu(&oldp->rcu, __tcindex_partial_destroy);
+		tcf_queue_work(&oldp->rwork, tcindex_partial_destroy_work);
 	return 0;
 
 errout_alloc:
@@ -572,7 +576,7 @@  static void tcindex_destroy(struct tcf_proto *tp, bool rtnl_held,
 	walker.fn = tcindex_destroy_element;
 	tcindex_walk(tp, &walker, true);
 
-	call_rcu(&p->rcu, __tcindex_destroy);
+	tcf_queue_work(&p->rwork, tcindex_destroy_work);
 }