* [Patch net] net_sched: hold rtnl lock in tcindex_partial_destroy_work()
@ 2020-03-12 5:42 Cong Wang
2020-03-12 5:42 ` [Patch net] net_sched: keep alloc_hash updated after hash allocation Cong Wang
2020-03-15 3:42 ` [Patch net] net_sched: hold rtnl lock in tcindex_partial_destroy_work() David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Cong Wang @ 2020-03-12 5:42 UTC (permalink / raw)
To: netdev
Cc: Cong Wang, syzbot+653090db2562495901dc, Jamal Hadi Salim, Jiri Pirko
syzbot reported a use-after-free in tcindex_dump(). This is due to
the lack of RTNL in the deferred rcu work. We queue this work with
RTNL in tcindex_change(), later, tcindex_dump() is called:
fh = tp->ops->get(tp, t->tcm_handle);
...
err = tp->ops->change(..., &fh, ...);
tfilter_notify(..., fh, ...);
but there is nothing to serialize the pending
tcindex_partial_destroy_work() with tcindex_dump().
Fix this by simply holding RTNL in tcindex_partial_destroy_work(),
so that it won't be called until RTNL is released after
tc_new_tfilter() is completed.
Reported-and-tested-by: syzbot+653090db2562495901dc@syzkaller.appspotmail.com
Fixes: 3d210534cc93 ("net_sched: fix a race condition in tcindex_destroy()")
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 | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 09b7dc5fe7e0..f2cb24b6f0cf 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -261,8 +261,10 @@ static void tcindex_partial_destroy_work(struct work_struct *work)
struct tcindex_data,
rwork);
+ rtnl_lock();
kfree(p->perfect);
kfree(p);
+ rtnl_unlock();
}
static void tcindex_free_perfect_hash(struct tcindex_data *cp)
--
2.21.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Patch net] net_sched: keep alloc_hash updated after hash allocation
2020-03-12 5:42 [Patch net] net_sched: hold rtnl lock in tcindex_partial_destroy_work() Cong Wang
@ 2020-03-12 5:42 ` Cong Wang
2020-03-15 3:43 ` David Miller
2020-03-15 3:42 ` [Patch net] net_sched: hold rtnl lock in tcindex_partial_destroy_work() David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Cong Wang @ 2020-03-12 5:42 UTC (permalink / raw)
To: netdev
Cc: Cong Wang, syzbot+dcc34d54d68ef7d2d53d,
syzbot+c72da7b9ed57cde6fca2, Jamal Hadi Salim, Jiri Pirko
In commit 599be01ee567 ("net_sched: fix an OOB access in cls_tcindex")
I moved cp->hash calculation before the first
tcindex_alloc_perfect_hash(), but cp->alloc_hash is left untouched.
This difference could lead to another out of bound access.
cp->alloc_hash should always be the size allocated, we should
update it after this tcindex_alloc_perfect_hash().
Reported-and-tested-by: syzbot+dcc34d54d68ef7d2d53d@syzkaller.appspotmail.com
Reported-and-tested-by: syzbot+c72da7b9ed57cde6fca2@syzkaller.appspotmail.com
Fixes: 599be01ee567 ("net_sched: fix an OOB access in cls_tcindex")
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 | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index f2cb24b6f0cf..9904299424a1 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -359,6 +359,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
if (tcindex_alloc_perfect_hash(net, cp) < 0)
goto errout;
+ cp->alloc_hash = cp->hash;
for (i = 0; i < min(cp->hash, p->hash); i++)
cp->perfect[i].res = p->perfect[i].res;
balloc = 1;
--
2.21.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch net] net_sched: hold rtnl lock in tcindex_partial_destroy_work()
2020-03-12 5:42 [Patch net] net_sched: hold rtnl lock in tcindex_partial_destroy_work() Cong Wang
2020-03-12 5:42 ` [Patch net] net_sched: keep alloc_hash updated after hash allocation Cong Wang
@ 2020-03-15 3:42 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2020-03-15 3:42 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, syzbot+653090db2562495901dc, jhs, jiri
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Wed, 11 Mar 2020 22:42:27 -0700
> syzbot reported a use-after-free in tcindex_dump(). This is due to
> the lack of RTNL in the deferred rcu work. We queue this work with
> RTNL in tcindex_change(), later, tcindex_dump() is called:
>
> fh = tp->ops->get(tp, t->tcm_handle);
> ...
> err = tp->ops->change(..., &fh, ...);
> tfilter_notify(..., fh, ...);
>
> but there is nothing to serialize the pending
> tcindex_partial_destroy_work() with tcindex_dump().
>
> Fix this by simply holding RTNL in tcindex_partial_destroy_work(),
> so that it won't be called until RTNL is released after
> tc_new_tfilter() is completed.
>
> Reported-and-tested-by: syzbot+653090db2562495901dc@syzkaller.appspotmail.com
> Fixes: 3d210534cc93 ("net_sched: fix a race condition in tcindex_destroy()")
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied and queued up for -stable, thanks Cong.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch net] net_sched: keep alloc_hash updated after hash allocation
2020-03-12 5:42 ` [Patch net] net_sched: keep alloc_hash updated after hash allocation Cong Wang
@ 2020-03-15 3:43 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2020-03-15 3:43 UTC (permalink / raw)
To: xiyou.wangcong
Cc: netdev, syzbot+dcc34d54d68ef7d2d53d, syzbot+c72da7b9ed57cde6fca2,
jhs, jiri
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Wed, 11 Mar 2020 22:42:28 -0700
> In commit 599be01ee567 ("net_sched: fix an OOB access in cls_tcindex")
> I moved cp->hash calculation before the first
> tcindex_alloc_perfect_hash(), but cp->alloc_hash is left untouched.
> This difference could lead to another out of bound access.
>
> cp->alloc_hash should always be the size allocated, we should
> update it after this tcindex_alloc_perfect_hash().
>
> Reported-and-tested-by: syzbot+dcc34d54d68ef7d2d53d@syzkaller.appspotmail.com
> Reported-and-tested-by: syzbot+c72da7b9ed57cde6fca2@syzkaller.appspotmail.com
> Fixes: 599be01ee567 ("net_sched: fix an OOB access in cls_tcindex")
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Also applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-15 3:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-12 5:42 [Patch net] net_sched: hold rtnl lock in tcindex_partial_destroy_work() Cong Wang
2020-03-12 5:42 ` [Patch net] net_sched: keep alloc_hash updated after hash allocation Cong Wang
2020-03-15 3:43 ` David Miller
2020-03-15 3:42 ` [Patch net] net_sched: hold rtnl lock in tcindex_partial_destroy_work() 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).