Re: [PATCH net v2 1/2] net/sched: reject overly deep qdisc hierarchies
From: Zijie Huang <hidden>
Date: 2026-08-01 14:30:33
Also in:
linux-kselftest, lkml
On 8/1/2026 10:11 PM, Jamal Hadi Salim wrote:
On Sat, Aug 1, 2026 at 9:42 AM Ren Wei [off-list ref] wrote:quoted
From: Zijie Huang <redacted> Deep qdisc hierarchies can lead to excessive recursion in qdisc tree walkers and exhaust the kernel stack. The existing loop check does not cover the create-and-graft path, so a hierarchy can still be extended by creating a new child qdisc below an already deep parent. Store the hierarchy depth in struct Qdisc and update it when qdiscs are grafted. Reject new child qdiscs once the parent is already at the maximum allowed depth. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Reported-by: Vega <redacted> Assisted-by: Codex:gpt-5.4 Signed-off-by: Zijie Huang <redacted> Signed-off-by: Ren Wei <redacted>This is the same patch i sent you except you changed the failure path to send EBIG instead of ELOOP. Is there a reason? And your feedback message is not as informative like what i had. NL_SET_ERR_MSG(extack, "Qdisc hierarchy too deep (max 7)"); This way the user doesnt have to dig to find what the limit is. Note: Your AI did not assist in this patch rather, I spent about 30 minutes looking closely using human knowledge. I am not looking for credit but for your education going forward: if someone invests their time reviewing your AI generated patches and suggests a different path that you adopt, please add a suggested-by tag crediting them. Or don't bother sending any patches, just report the problem and provide the Poc, I could have come up with that patch in about the same time investment. I am going to ack the series unless someone complains about EBIG vs ELOOP in which case please fix the msg.. cheers, jamal
I apologize for the inconvenience. This is my first time try contributing to some big open source community like linux. I believe I should have learned about etiquette like "suggested-by" beforehand. In reality, I checked similar code paths about exceeding depth in the repository and found that E2BIG is more common so I replaced ELOOP with that. And after reviewing the issue, I think the patch you provided is enough to cover the problem, so I didn't change a lot. Again, I apologize for the inconvenience and appreciate your patience.
quoted
changes in v2: - Store the qdisc hierarchy depth in struct Qdisc and update it from qdisc_graft(), instead of walking the parent chain. - Move the depth check to qdisc_graft() so it is applied at the actual attach point. - Add tdc tests for the maximum allowed depth and rejection above it. - v1 Link: https://lore.kernel.org/all/cover.1785304107.git.milkory@outlook.com/ (local) include/net/sch_generic.h | 1 + net/sched/sch_api.c | 9 +++++++++ 2 files changed, 10 insertions(+)diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 45a1e8c78222..cbc248776511 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h@@ -99,6 +99,7 @@ struct Qdisc { struct hlist_node hash; u32 handle; u32 parent; + int depth; struct netdev_queue *dev_queue;diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index 668bcd60d183..65b35528d125 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c@@ -1114,6 +1114,9 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent, unsigned int i, num_q, ingress; struct netdev_queue *dev_queue; + if (new) + new->depth = 0; + ingress = 0; num_q = dev->num_tx_queues; if ((q && q->flags & TCQ_F_INGRESS) ||@@ -1211,9 +1214,15 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent, NL_SET_ERR_MSG(extack, "STAB not supported on a non root"); return -EINVAL; } + if (new && parent->depth >= 7) { + NL_SET_ERR_MSG(extack, "Qdisc hierarchy is too deep"); + return -E2BIG; + } err = cops->graft(parent, cl, new, &old, extack); if (err) return err; + if (new) + new->depth = parent->depth + 1; notify_and_destroy(net, skb, n, classid, old, new, extack); } return 0; --2.47.2