* [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact
@ 2023-05-29 19:52 Peilin Ye
2023-05-29 19:52 ` [PATCH v6 net 1/4] net/sched: sch_ingress: Only create under TC_H_INGRESS Peilin Ye
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Peilin Ye @ 2023-05-29 19:52 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Peilin Ye, John Fastabend, Daniel Borkmann, Vlad Buslov,
Pedro Tammela, Hillf Danton, Zhengchao Shao, netdev,
linux-kernel, Cong Wang, Peilin Ye
Link to v5: https://lore.kernel.org/r/cover.1684887977.git.peilin.ye@bytedance.com/
Link to v4: https://lore.kernel.org/r/cover.1684825171.git.peilin.ye@bytedance.com/
Link to v3 (incomplete): https://lore.kernel.org/r/cover.1684821877.git.peilin.ye@bytedance.com/
Link to v2: https://lore.kernel.org/r/cover.1684796705.git.peilin.ye@bytedance.com/
Link to v1: https://lore.kernel.org/r/cover.1683326865.git.peilin.ye@bytedance.com/
Hi all,
These are v6 fixes for ingress and clsact Qdiscs, including only first 4
patches (already tested and reviewed) from v5. Patch 5 and 6 from
previous versions are still under discussion and will be sent separately.
Per-patch changelog omitted.
Change in v6:
- only include first 4 patches from previous versions
Changes in v5:
- for [6/6], reinitialize @q, @p (suggested by Vlad) and @tcm after the
"replay:" tag
- for [1,2/6], do nothing in ->destroy() if ->parent isn't ffff:fff1, as
reported by Pedro
Change in v3, v4:
- add in-body From: tags
Changes in v2:
- for [1-5/6], include tags from Jamal and Pedro
- for [6/6], as suggested by Vlad, replay the request if the current
Qdisc has any ongoing (RTNL-unlocked) filter requests, instead of
returning -EBUSY to the user
- use Closes: tag as warned by checkpatch
[1,2/6]: ingress and clsact Qdiscs should only be created under ffff:fff1
[3/6]: Under ffff:fff1, only create ingress and clsact Qdiscs (for now,
at least)
[4/6]: After creating ingress and clsact Qdiscs under ffff:fff1, do not
graft them again to anywhere else (e.g. as the inner Qdisc of a
TBF Qdisc)
[5/6]: Prepare for [6/6], do not reuse that for-loop in qdisc_graft()
for ingress and clsact Qdiscs
[6/6]: Fix use-after-free [a] in mini_qdisc_pair_swap()
[a] https://syzkaller.appspot.com/bug?extid=b53a9c0d1ea4ad62da8b
Thanks,
Peilin Ye (4):
net/sched: sch_ingress: Only create under TC_H_INGRESS
net/sched: sch_clsact: Only create under TC_H_CLSACT
net/sched: Reserve TC_H_INGRESS (TC_H_CLSACT) for ingress (clsact)
Qdiscs
net/sched: Prohibit regrafting ingress or clsact Qdiscs
net/sched/sch_api.c | 12 +++++++++++-
net/sched/sch_ingress.c | 16 ++++++++++++++--
2 files changed, 25 insertions(+), 3 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v6 net 1/4] net/sched: sch_ingress: Only create under TC_H_INGRESS
2023-05-29 19:52 [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact Peilin Ye
@ 2023-05-29 19:52 ` Peilin Ye
2023-05-29 19:53 ` [PATCH v6 net 2/4] net/sched: sch_clsact: Only create under TC_H_CLSACT Peilin Ye
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Peilin Ye @ 2023-05-29 19:52 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Peilin Ye, John Fastabend, Daniel Borkmann, Vlad Buslov,
Pedro Tammela, Hillf Danton, Zhengchao Shao, netdev,
linux-kernel, Cong Wang, Peilin Ye
From: Peilin Ye <peilin.ye@bytedance.com>
ingress Qdiscs are only supposed to be created under TC_H_INGRESS.
Return -EOPNOTSUPP if 'parent' is not TC_H_INGRESS, similar to
mq_init().
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+b53a9c0d1ea4ad62da8b@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/r/0000000000006cf87705f79acf1a@google.com/
Tested-by: Pedro Tammela <pctammela@mojatatu.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Vlad Buslov <vladbu@nvidia.com>
Signed-off-by: Peilin Ye <peilin.ye@bytedance.com>
---
net/sched/sch_ingress.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
index 84838128b9c5..f9ef6deb2770 100644
--- a/net/sched/sch_ingress.c
+++ b/net/sched/sch_ingress.c
@@ -80,6 +80,9 @@ static int ingress_init(struct Qdisc *sch, struct nlattr *opt,
struct net_device *dev = qdisc_dev(sch);
int err;
+ if (sch->parent != TC_H_INGRESS)
+ return -EOPNOTSUPP;
+
net_inc_ingress_queue();
mini_qdisc_pair_init(&q->miniqp, sch, &dev->miniq_ingress);
@@ -101,6 +104,9 @@ static void ingress_destroy(struct Qdisc *sch)
{
struct ingress_sched_data *q = qdisc_priv(sch);
+ if (sch->parent != TC_H_INGRESS)
+ return;
+
tcf_block_put_ext(q->block, sch, &q->block_info);
net_dec_ingress_queue();
}
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v6 net 2/4] net/sched: sch_clsact: Only create under TC_H_CLSACT
2023-05-29 19:52 [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact Peilin Ye
2023-05-29 19:52 ` [PATCH v6 net 1/4] net/sched: sch_ingress: Only create under TC_H_INGRESS Peilin Ye
@ 2023-05-29 19:53 ` Peilin Ye
2023-05-29 19:54 ` [PATCH v6 net 3/4] net/sched: Reserve TC_H_INGRESS (TC_H_CLSACT) for ingress (clsact) Qdiscs Peilin Ye
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Peilin Ye @ 2023-05-29 19:53 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Peilin Ye, John Fastabend, Daniel Borkmann, Vlad Buslov,
Pedro Tammela, Hillf Danton, Zhengchao Shao, netdev,
linux-kernel, Cong Wang, Peilin Ye
From: Peilin Ye <peilin.ye@bytedance.com>
clsact Qdiscs are only supposed to be created under TC_H_CLSACT (which
equals TC_H_INGRESS). Return -EOPNOTSUPP if 'parent' is not
TC_H_CLSACT.
Fixes: 1f211a1b929c ("net, sched: add clsact qdisc")
Tested-by: Pedro Tammela <pctammela@mojatatu.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Vlad Buslov <vladbu@nvidia.com>
Signed-off-by: Peilin Ye <peilin.ye@bytedance.com>
---
net/sched/sch_ingress.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
index f9ef6deb2770..35963929e117 100644
--- a/net/sched/sch_ingress.c
+++ b/net/sched/sch_ingress.c
@@ -225,6 +225,9 @@ static int clsact_init(struct Qdisc *sch, struct nlattr *opt,
struct net_device *dev = qdisc_dev(sch);
int err;
+ if (sch->parent != TC_H_CLSACT)
+ return -EOPNOTSUPP;
+
net_inc_ingress_queue();
net_inc_egress_queue();
@@ -254,6 +257,9 @@ static void clsact_destroy(struct Qdisc *sch)
{
struct clsact_sched_data *q = qdisc_priv(sch);
+ if (sch->parent != TC_H_CLSACT)
+ return;
+
tcf_block_put_ext(q->egress_block, sch, &q->egress_block_info);
tcf_block_put_ext(q->ingress_block, sch, &q->ingress_block_info);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v6 net 3/4] net/sched: Reserve TC_H_INGRESS (TC_H_CLSACT) for ingress (clsact) Qdiscs
2023-05-29 19:52 [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact Peilin Ye
2023-05-29 19:52 ` [PATCH v6 net 1/4] net/sched: sch_ingress: Only create under TC_H_INGRESS Peilin Ye
2023-05-29 19:53 ` [PATCH v6 net 2/4] net/sched: sch_clsact: Only create under TC_H_CLSACT Peilin Ye
@ 2023-05-29 19:54 ` Peilin Ye
2023-05-29 19:54 ` [PATCH v6 net 4/4] net/sched: Prohibit regrafting ingress or clsact Qdiscs Peilin Ye
2023-05-31 6:40 ` [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Peilin Ye @ 2023-05-29 19:54 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Peilin Ye, John Fastabend, Daniel Borkmann, Vlad Buslov,
Pedro Tammela, Hillf Danton, Zhengchao Shao, netdev,
linux-kernel, Cong Wang, Peilin Ye
From: Peilin Ye <peilin.ye@bytedance.com>
Currently it is possible to add e.g. an HTB Qdisc under ffff:fff1
(TC_H_INGRESS, TC_H_CLSACT):
$ ip link add name ifb0 type ifb
$ tc qdisc add dev ifb0 parent ffff:fff1 htb
$ tc qdisc add dev ifb0 clsact
Error: Exclusivity flag on, cannot modify.
$ drgn
...
>>> ifb0 = netdev_get_by_name(prog, "ifb0")
>>> qdisc = ifb0.ingress_queue.qdisc_sleeping
>>> print(qdisc.ops.id.string_().decode())
htb
>>> qdisc.flags.value_() # TCQ_F_INGRESS
2
Only allow ingress and clsact Qdiscs under ffff:fff1. Return -EINVAL
for everything else. Make TCQ_F_INGRESS a static flag of ingress and
clsact Qdiscs.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 1f211a1b929c ("net, sched: add clsact qdisc")
Tested-by: Pedro Tammela <pctammela@mojatatu.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Vlad Buslov <vladbu@nvidia.com>
Signed-off-by: Peilin Ye <peilin.ye@bytedance.com>
---
net/sched/sch_api.c | 7 ++++++-
net/sched/sch_ingress.c | 4 ++--
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index fdb8f429333d..383195955b7d 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1252,7 +1252,12 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
sch->parent = parent;
if (handle == TC_H_INGRESS) {
- sch->flags |= TCQ_F_INGRESS;
+ if (!(sch->flags & TCQ_F_INGRESS)) {
+ NL_SET_ERR_MSG(extack,
+ "Specified parent ID is reserved for ingress and clsact Qdiscs");
+ err = -EINVAL;
+ goto err_out3;
+ }
handle = TC_H_MAKE(TC_H_INGRESS, 0);
} else {
if (handle == 0) {
diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
index 35963929e117..e43a45499372 100644
--- a/net/sched/sch_ingress.c
+++ b/net/sched/sch_ingress.c
@@ -140,7 +140,7 @@ static struct Qdisc_ops ingress_qdisc_ops __read_mostly = {
.cl_ops = &ingress_class_ops,
.id = "ingress",
.priv_size = sizeof(struct ingress_sched_data),
- .static_flags = TCQ_F_CPUSTATS,
+ .static_flags = TCQ_F_INGRESS | TCQ_F_CPUSTATS,
.init = ingress_init,
.destroy = ingress_destroy,
.dump = ingress_dump,
@@ -281,7 +281,7 @@ static struct Qdisc_ops clsact_qdisc_ops __read_mostly = {
.cl_ops = &clsact_class_ops,
.id = "clsact",
.priv_size = sizeof(struct clsact_sched_data),
- .static_flags = TCQ_F_CPUSTATS,
+ .static_flags = TCQ_F_INGRESS | TCQ_F_CPUSTATS,
.init = clsact_init,
.destroy = clsact_destroy,
.dump = ingress_dump,
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v6 net 4/4] net/sched: Prohibit regrafting ingress or clsact Qdiscs
2023-05-29 19:52 [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact Peilin Ye
` (2 preceding siblings ...)
2023-05-29 19:54 ` [PATCH v6 net 3/4] net/sched: Reserve TC_H_INGRESS (TC_H_CLSACT) for ingress (clsact) Qdiscs Peilin Ye
@ 2023-05-29 19:54 ` Peilin Ye
2023-05-31 6:40 ` [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Peilin Ye @ 2023-05-29 19:54 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Peilin Ye, John Fastabend, Daniel Borkmann, Vlad Buslov,
Pedro Tammela, Hillf Danton, Zhengchao Shao, netdev,
linux-kernel, Cong Wang, Peilin Ye
From: Peilin Ye <peilin.ye@bytedance.com>
Currently, after creating an ingress (or clsact) Qdisc and grafting it
under TC_H_INGRESS (TC_H_CLSACT), it is possible to graft it again under
e.g. a TBF Qdisc:
$ ip link add ifb0 type ifb
$ tc qdisc add dev ifb0 handle 1: root tbf rate 20kbit buffer 1600 limit 3000
$ tc qdisc add dev ifb0 clsact
$ tc qdisc link dev ifb0 handle ffff: parent 1:1
$ tc qdisc show dev ifb0
qdisc tbf 1: root refcnt 2 rate 20Kbit burst 1600b lat 560.0ms
qdisc clsact ffff: parent ffff:fff1 refcnt 2
^^^^^^^^
clsact's refcount has increased: it is now grafted under both
TC_H_CLSACT and 1:1.
ingress and clsact Qdiscs should only be used under TC_H_INGRESS
(TC_H_CLSACT). Prohibit regrafting them.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 1f211a1b929c ("net, sched: add clsact qdisc")
Tested-by: Pedro Tammela <pctammela@mojatatu.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Vlad Buslov <vladbu@nvidia.com>
Signed-off-by: Peilin Ye <peilin.ye@bytedance.com>
---
net/sched/sch_api.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 383195955b7d..49b9c1bbfdd9 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1596,6 +1596,11 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
NL_SET_ERR_MSG(extack, "Invalid qdisc name");
return -EINVAL;
}
+ if (q->flags & TCQ_F_INGRESS) {
+ NL_SET_ERR_MSG(extack,
+ "Cannot regraft ingress or clsact Qdiscs");
+ return -EINVAL;
+ }
if (q == p ||
(p && check_loop(q, p, 0))) {
NL_SET_ERR_MSG(extack, "Qdisc parent/child loop detected");
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact
2023-05-29 19:52 [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact Peilin Ye
` (3 preceding siblings ...)
2023-05-29 19:54 ` [PATCH v6 net 4/4] net/sched: Prohibit regrafting ingress or clsact Qdiscs Peilin Ye
@ 2023-05-31 6:40 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-05-31 6:40 UTC (permalink / raw)
To: Peilin Ye
Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
peilin.ye, john.fastabend, daniel, vladbu, pctammela, hdanton,
shaozhengchao, netdev, linux-kernel, cong.wang
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 29 May 2023 12:52:31 -0700 you wrote:
> Link to v5: https://lore.kernel.org/r/cover.1684887977.git.peilin.ye@bytedance.com/
> Link to v4: https://lore.kernel.org/r/cover.1684825171.git.peilin.ye@bytedance.com/
> Link to v3 (incomplete): https://lore.kernel.org/r/cover.1684821877.git.peilin.ye@bytedance.com/
> Link to v2: https://lore.kernel.org/r/cover.1684796705.git.peilin.ye@bytedance.com/
> Link to v1: https://lore.kernel.org/r/cover.1683326865.git.peilin.ye@bytedance.com/
>
> Hi all,
>
> [...]
Here is the summary with links:
- [v6,net,1/4] net/sched: sch_ingress: Only create under TC_H_INGRESS
https://git.kernel.org/netdev/net/c/c7cfbd115001
- [v6,net,2/4] net/sched: sch_clsact: Only create under TC_H_CLSACT
https://git.kernel.org/netdev/net/c/5eeebfe6c493
- [v6,net,3/4] net/sched: Reserve TC_H_INGRESS (TC_H_CLSACT) for ingress (clsact) Qdiscs
https://git.kernel.org/netdev/net/c/f85fa45d4a94
- [v6,net,4/4] net/sched: Prohibit regrafting ingress or clsact Qdiscs
https://git.kernel.org/netdev/net/c/9de95df5d15b
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-31 6:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-29 19:52 [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact Peilin Ye
2023-05-29 19:52 ` [PATCH v6 net 1/4] net/sched: sch_ingress: Only create under TC_H_INGRESS Peilin Ye
2023-05-29 19:53 ` [PATCH v6 net 2/4] net/sched: sch_clsact: Only create under TC_H_CLSACT Peilin Ye
2023-05-29 19:54 ` [PATCH v6 net 3/4] net/sched: Reserve TC_H_INGRESS (TC_H_CLSACT) for ingress (clsact) Qdiscs Peilin Ye
2023-05-29 19:54 ` [PATCH v6 net 4/4] net/sched: Prohibit regrafting ingress or clsact Qdiscs Peilin Ye
2023-05-31 6:40 ` [PATCH v6 net 0/4] net/sched: Fixes for sch_ingress and sch_clsact patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.