Thread (9 messages) flat view 9 messages, 5 authors, 8d ago

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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help