From: Al Viro <viro@ZenIV•linux.org.uk>
To: Cong Wang <xiyou.wangcong@gmail•com>
Cc: Jamal Hadi Salim <jhs@mojatatu•com>,
Linux Kernel Network Developers <netdev@vger•kernel.org>,
Jiri Pirko <jiri@resnulli•us>
Subject: Re: [PATCH 2/7] mark root hnode explicitly
Date: Fri, 7 Sep 2018 04:49:58 +0100 [thread overview]
Message-ID: <20180907034958.GZ19965@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CAM_iQpV7H8v3Oebftd0_aHzwwU_Phr3-7a=Un2-TRmuuZr7VOw@mail.gmail.com>
On Thu, Sep 06, 2018 at 08:23:36PM -0700, Cong Wang wrote:
> Pretty sure there is a 'tp' in u32_set_parms() parameter list.
>
> Are you saying it is not what you want? If so, why?
>
> More importantly, why this information is again missing in your
> changelog? This patch is definitely not trivial, it deserves a detailed
> changelog.
>
>
> > our own root, sure. But there's nothing to stop doing the same via another
> > tcf_proto...
>
> To my best knowledge, the place where you set ->is_root=true
> is precisely same with where we set tp->root=root_ht, and it doesn't
> change after set. What am I missing here?
The fact that there can be two (or more) different tcf_proto instances sharing
->data, but not ->root. And since ->data is shared, u32_get() on one tp
will be able to return you ->root of *ANOTHER* one. So comparison with
tp->root doesn't protect you. Try this on mainline:
tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff: protocol ip prio 100 handle 1: u32 divisor 1
tc filter add dev eth0 parent ffff: protocol ip prio 200 handle 2: u32 divisor 1
tc filter delete dev eth0 parent ffff: protocol ip prio 100 handle 801: u32
and watch the fun as soon as you get an incoming packet on eth0. That panic
is fixed by 1/7, but you get "Not allowed to delete root node" for removing
_your_ root, with "Can not delete in-use filter" for other's root (as in the
last line of the reproducer).
next prev parent reply other threads:[~2018-09-07 8:28 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-05 19:01 [PATCHES] cls_u32 cleanups and fixes Al Viro
2018-09-05 19:04 ` [PATCH 1/7] fix hnode refcounting Al Viro
2018-09-05 19:04 ` [PATCH 2/7] mark root hnode explicitly Al Viro
2018-09-06 10:28 ` Jamal Hadi Salim
2018-09-06 10:34 ` Jamal Hadi Salim
2018-09-06 10:42 ` Jamal Hadi Salim
2018-09-06 10:59 ` Al Viro
2018-09-06 11:04 ` Jamal Hadi Salim
2018-09-07 2:57 ` Cong Wang
2018-09-07 3:04 ` Al Viro
2018-09-07 3:23 ` Cong Wang
2018-09-07 3:49 ` Al Viro [this message]
2018-09-07 4:14 ` Cong Wang
2018-09-05 19:04 ` [PATCH 3/7] make sure that divisor is a power of 2 Al Viro
2018-09-06 10:28 ` Jamal Hadi Salim
2018-09-05 19:04 ` [PATCH 4/7] get rid of unused argument of u32_destroy_key() Al Viro
2018-09-06 10:34 ` Jamal Hadi Salim
2018-09-05 19:04 ` [PATCH 5/7] get rid of tc_u_knode ->tp Al Viro
2018-09-06 10:35 ` Jamal Hadi Salim
2018-09-05 19:04 ` [PATCH 6/7] get rid of tc_u_common ->rcu Al Viro
2018-09-06 10:36 ` Jamal Hadi Salim
2018-09-07 4:18 ` Cong Wang
2018-09-07 4:28 ` Al Viro
2018-09-05 19:04 ` [PATCH 7/7] clean tc_u_common hashtable Al Viro
2018-09-06 10:36 ` Jamal Hadi Salim
2018-09-06 10:21 ` [PATCH 1/7] fix hnode refcounting Jamal Hadi Salim
2018-09-07 2:35 ` Al Viro
2018-09-07 12:13 ` Jamal Hadi Salim
2018-09-07 12:33 ` Jamal Hadi Salim
2018-09-08 15:03 ` Al Viro
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=20180907034958.GZ19965@ZenIV.linux.org.uk \
--to=viro@zeniv$(echo .)linux.org.uk \
--cc=jhs@mojatatu$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=netdev@vger$(echo .)kernel.org \
--cc=xiyou.wangcong@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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