From: Cong Wang <xiyou.wangcong@gmail.com>
To: netdev@vger.kernel.org
Cc: Cong Wang <xiyou.wangcong@gmail.com>, Adrian <bugs@abtelecom.ro>,
Ben Hutchings <ben@decadent.org.uk>,
Jamal Hadi Salim <jhs@mojatatu.com>,
Jiri Pirko <jiri@resnulli.us>
Subject: [Patch net 1/3] net_sched: fix a race condition in tcindex_destroy()
Date: Sun, 10 Feb 2019 17:47:29 -0800 [thread overview]
Message-ID: <20190211014731.23932-2-xiyou.wangcong@gmail.com> (raw)
In-Reply-To: <20190211014731.23932-1-xiyou.wangcong@gmail.com>
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.
This change requires us to store a net pointer inside struct
tcindex_data, to avoid the known race with tc_action_net_exit().
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 | 46 ++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 9ccc93f257db..14e6d80dd58e 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -48,7 +48,8 @@ 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 net *net;
+ struct rcu_work rwork;
};
static inline int tcindex_filter_is_set(struct tcindex_filter_result *r)
@@ -229,15 +230,23 @@ static int tcindex_destroy_element(struct tcf_proto *tp,
return tcindex_delete(tp, arg, &last, NULL);
}
-static void __tcindex_destroy(struct rcu_head *head)
+static void __tcindex_destroy(struct tcindex_data *p)
{
- struct tcindex_data *p = container_of(head, struct tcindex_data, rcu);
-
kfree(p->perfect);
kfree(p->h);
kfree(p);
}
+static void tcindex_destroy_work(struct work_struct *work)
+{
+ struct tcindex_data *p = container_of(to_rcu_work(work),
+ struct tcindex_data,
+ rwork);
+
+ put_net(p->net);
+ __tcindex_destroy(p);
+}
+
static inline int
valid_perfect_hash(struct tcindex_data *p)
{
@@ -258,14 +267,22 @@ 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(struct tcindex_data *p)
{
- struct tcindex_data *p = container_of(head, struct tcindex_data, rcu);
-
kfree(p->perfect);
kfree(p);
}
+static void tcindex_partial_destroy_work(struct work_struct *work)
+{
+ struct tcindex_data *p = container_of(to_rcu_work(work),
+ struct tcindex_data,
+ rwork);
+
+ put_net(p->net);
+ __tcindex_partial_destroy(p);
+}
+
static void tcindex_free_perfect_hash(struct tcindex_data *cp)
{
int i;
@@ -333,6 +350,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
cp->alloc_hash = p->alloc_hash;
cp->fall_through = p->fall_through;
cp->tp = tp;
+ cp->net = net;
if (p->perfect) {
int i;
@@ -477,8 +495,13 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
rcu_assign_pointer(*fp, f);
}
- if (oldp)
- call_rcu(&oldp->rcu, __tcindex_partial_destroy);
+ if (oldp) {
+ if (oldp->net && maybe_get_net(oldp->net))
+ tcf_queue_work(&oldp->rwork,
+ tcindex_partial_destroy_work);
+ else
+ __tcindex_partial_destroy(oldp);
+ }
return 0;
errout_alloc:
@@ -570,7 +593,10 @@ static void tcindex_destroy(struct tcf_proto *tp,
walker.fn = tcindex_destroy_element;
tcindex_walk(tp, &walker);
- call_rcu(&p->rcu, __tcindex_destroy);
+ if (maybe_get_net(p->net))
+ tcf_queue_work(&p->rwork, tcindex_destroy_work);
+ else
+ __tcindex_destroy(p);
}
--
2.20.1
next prev parent reply other threads:[~2019-02-11 1:47 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-11 1:47 [Patch net 0/3] net_sched: some fixes for cls_tcindex Cong Wang
2019-02-11 1:47 ` Cong Wang [this message]
2019-02-11 1:47 ` [Patch net 2/3] net_sched: fix a memory leak in cls_tcindex Cong Wang
[not found] ` <201902111051.xRYMeLJl%fengguang.wu@intel.com>
2019-02-11 7:19 ` Cong Wang
2019-02-11 1:47 ` [Patch net 3/3] net_sched: fix two more memory leaks " Cong Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190211014731.23932-2-xiyou.wangcong@gmail.com \
--to=xiyou.wangcong@gmail.com \
--cc=ben@decadent.org.uk \
--cc=bugs@abtelecom.ro \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).