Re: [PATCH net v2 1/2] net/sched: reject overly deep qdisc hierarchies

[Zijie Huang]

On 8/1/2026 10:11 PM, Jamal Hadi Salim wrote:
> On Sat, Aug 1, 2026 at 9:42 AM Ren Wei <enjou1224z@xxxxxxxxx> wrote:
>>
>> From: Zijie Huang <milkory@xxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxx
>> Reported-by: Vega <vega@xxxxxxxxxx>
>> Assisted-by: Codex:gpt-5.4
>> Signed-off-by: Zijie Huang <milkory@xxxxxxxxxxx>
>> Signed-off-by: Ren Wei <enjou1224z@xxxxxxxxx>
>
>
> 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.

>> 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@xxxxxxxxxxx/
>>
>>
>>  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
>>