* [Patch net-next 1/2] net_sched: fix a race condition in tcindex_destroy()
@ 2019-02-16 18:58 Cong Wang
2019-02-16 18:58 ` [Patch net-next 2/2] net_sched: fix a memory leak in cls_tcindex Cong Wang
2019-02-21 4:11 ` [Patch net-next 1/2] net_sched: fix a race condition in tcindex_destroy() David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Cong Wang @ 2019-02-16 18:58 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Adrian, Ben Hutchings, Jamal Hadi Salim, Jiri Pirko
(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(-)
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);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Patch net-next 2/2] net_sched: fix a memory leak in cls_tcindex
2019-02-16 18:58 [Patch net-next 1/2] net_sched: fix a race condition in tcindex_destroy() Cong Wang
@ 2019-02-16 18:58 ` Cong Wang
2019-02-21 4:11 ` David Miller
2019-02-21 4:11 ` [Patch net-next 1/2] net_sched: fix a race condition in tcindex_destroy() David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Cong Wang @ 2019-02-16 18:58 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, Jiri Pirko
(cherry picked from commit 033b228e7f26b29ae37f8bfa1bc6b209a5365e9f)
When tcindex_destroy() destroys all the filter results in
the perfect hash table, it invokes the walker to delete
each of them. However, results with class==0 are skipped
in either tcindex_walk() or tcindex_delete(), which causes
a memory leak reported by kmemleak.
This patch fixes it by skipping the walker and directly
deleting these filter results so we don't miss any filter
result.
As a result of this change, we have to initialize exts->net
properly in tcindex_alloc_perfect_hash(). For net-next, we
need to consider whether we should initialize ->net in
tcf_exts_init() instead, before that just directly test
CONFIG_NET_CLS_ACT=y.
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 | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 81a433ae31b3..fbf3519a12d8 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -221,14 +221,6 @@ static int tcindex_delete(struct tcf_proto *tp, void *arg, bool *last,
return 0;
}
-static int tcindex_destroy_element(struct tcf_proto *tp,
- void *arg, struct tcf_walker *walker)
-{
- bool last;
-
- return tcindex_delete(tp, arg, &last, false, NULL);
-}
-
static void tcindex_destroy_work(struct work_struct *work)
{
struct tcindex_data *p = container_of(to_rcu_work(work),
@@ -568,13 +560,32 @@ static void tcindex_destroy(struct tcf_proto *tp, bool rtnl_held,
struct netlink_ext_ack *extack)
{
struct tcindex_data *p = rtnl_dereference(tp->root);
- struct tcf_walker walker;
+ int i;
pr_debug("tcindex_destroy(tp %p),p %p\n", tp, p);
- walker.count = 0;
- walker.skip = 0;
- walker.fn = tcindex_destroy_element;
- tcindex_walk(tp, &walker, true);
+
+ if (p->perfect) {
+ for (i = 0; i < p->hash; i++) {
+ struct tcindex_filter_result *r = p->perfect + i;
+
+ tcf_unbind_filter(tp, &r->res);
+ if (tcf_exts_get_net(&r->exts))
+ tcf_queue_work(&r->rwork,
+ tcindex_destroy_rexts_work);
+ else
+ __tcindex_destroy_rexts(r);
+ }
+ }
+
+ for (i = 0; p->h && i < p->hash; i++) {
+ struct tcindex_filter *f, *next;
+ bool last;
+
+ for (f = rtnl_dereference(p->h[i]); f; f = next) {
+ next = rtnl_dereference(f->next);
+ tcindex_delete(tp, &f->result, &last, rtnl_held, NULL);
+ }
+ }
tcf_queue_work(&p->rwork, tcindex_destroy_work);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch net-next 1/2] net_sched: fix a race condition in tcindex_destroy()
2019-02-16 18:58 [Patch net-next 1/2] net_sched: fix a race condition in tcindex_destroy() Cong Wang
2019-02-16 18:58 ` [Patch net-next 2/2] net_sched: fix a memory leak in cls_tcindex Cong Wang
@ 2019-02-21 4:11 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2019-02-21 4:11 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, bugs, ben, jhs, jiri
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch net-next 2/2] net_sched: fix a memory leak in cls_tcindex
2019-02-16 18:58 ` [Patch net-next 2/2] net_sched: fix a memory leak in cls_tcindex Cong Wang
@ 2019-02-21 4:11 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2019-02-21 4:11 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, jhs, jiri
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Sat, 16 Feb 2019 10:58:27 -0800
> (cherry picked from commit 033b228e7f26b29ae37f8bfa1bc6b209a5365e9f)
>
> When tcindex_destroy() destroys all the filter results in
> the perfect hash table, it invokes the walker to delete
> each of them. However, results with class==0 are skipped
> in either tcindex_walk() or tcindex_delete(), which causes
> a memory leak reported by kmemleak.
>
> This patch fixes it by skipping the walker and directly
> deleting these filter results so we don't miss any filter
> result.
>
> As a result of this change, we have to initialize exts->net
> properly in tcindex_alloc_perfect_hash(). For net-next, we
> need to consider whether we should initialize ->net in
> tcf_exts_init() instead, before that just directly test
> CONFIG_NET_CLS_ACT=y.
>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-21 4:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-16 18:58 [Patch net-next 1/2] net_sched: fix a race condition in tcindex_destroy() Cong Wang
2019-02-16 18:58 ` [Patch net-next 2/2] net_sched: fix a memory leak in cls_tcindex Cong Wang
2019-02-21 4:11 ` David Miller
2019-02-21 4:11 ` [Patch net-next 1/2] net_sched: fix a race condition in tcindex_destroy() David Miller
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).