[PATCH net v2] net/sched: sch_htb: limit htb_classify inner-class filter hops

[PATCH net v2] net/sched: sch_htb: limit htb_classify inner-class filter hops

[Victor Nogueira]

From: Jamal Hadi Salim <jhs@mojatatu.com>

htb_classify() follows each filter-selected inner class by switching
to cl->filter_list, but never bounds the number of hops. A filter on
an inner class can point back to itself or to another inner class that
points back, creating an infinite loop in the packet classification
path with the qdisc lock held and BH disabled — a soft lockup / panic
from a single packet.

Bound the traversal with a hop counter and drop the packet with a
rate-limited warning once the bound is exceeded. The counter is
incremented at the point the inner filter chain is picked up, after the
TC_ACT_* switch has consumed the classifier verdict, so a terminal
TC_ACT_QUEUED/STOLEN/TRAP on the last permitted chain still sets *qerr
to __NET_XMIT_STOLEN and the packet is not charged as a drop by this
qdisc or its parent.

The bound is TC_HTB_MAXDEPTH, taken from HTB's own parameters rather than
from the qdisc hierarchy depth limit. Class levels run from 0 to
TC_HTB_MAXDEPTH - 1, so a traversal that strictly descends in level can
take at most TC_HTB_MAXDEPTH hops. That descent is what a sane
configuration does, but it is assumed here rather than enforced:
htb_find() resolves a classid against every class in the qdisc, so a
filter may equally select a sibling or an ancestor. The normal
root -> inner -> leaf path takes a single hop, so the bound does not
affect legitimate classification.

htb_classify() can now return NULL irrespective of CONFIG_NET_CLS_ACT,
whereas previously every NULL return sat inside that ifdef. The NULL
handler in htb_enqueue() therefore cannot stay conditional either, so
drop the ifdef around it. This matches hfsc_enqueue(), which has always
handled a NULL class unconditionally. Without it, a kernel built
without actions would dereference a NULL class instead of dropping.

Conditions to recreate the bug:
- CONFIG_NET_SCHED, CONFIG_NET_SCH_HTB, CONFIG_NET_CLS_U32,
  CONFIG_LOCKUP_DETECTOR.
- Create an HTB qdisc on a device (e.g. lo), add an inner class
  1:1 with a leaf child 1:10, install a root u32 filter selecting
  1:1, and an inner-class u32 filter on 1:1 also selecting 1:1.
- Send one packet (ping). On the unfixed kernel the classify loop
  spins with the qdisc lock held; with softlockup_panic=1 it panics.
- Reachable from unprivileged user via unshare -Urn (CAP_NET_ADMIN).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Vega <vega@nebusec.ai>
Co-developed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v1 -> v2:
- Count the hop where the inner filter chain is picked up instead of at
  the top of the loop body. That way we don't ignore the run's verdict
  for the last iteration (Sashiko)
- Fix reverse xmas tree declaration ordering for the new hops variable
- Add a name to the Reported-by tag
v1: https://lore.kernel.org/netdev/20260824161809.4147223-1-victor@mojatatu.com
---
 net/sched/sch_htb.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index fdac0dc8f35a..1ba67b121de4 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -224,6 +224,7 @@ static struct htb_class *htb_classify(struct sk_buff *skb, struct Qdisc *sch,
 	struct htb_class *cl;
 	struct tcf_result res;
 	struct tcf_proto *tcf;
+	unsigned int hops = 0;
 	int result;
 
 	/* allow to select class by setting skb->priority to valid classid;
@@ -266,6 +267,10 @@ static struct htb_class *htb_classify(struct sk_buff *skb, struct Qdisc *sch,
 		if (!cl->level)
 			return cl;	/* we hit leaf; return it */
 
+		if (++hops > TC_HTB_MAXDEPTH) {
+			pr_warn_ratelimited("htb: classify loop detected, dropping packet\n");
+			return NULL;
+		}
 		/* we have got inner class; apply inner filter chain */
 		tcf = rcu_dereference_bh(cl->filter_list);
 	}
@@ -633,13 +638,11 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		} else {
 			return qdisc_drop(skb, sch, to_free);
 		}
-#ifdef CONFIG_NET_CLS_ACT
 	} else if (!cl) {
 		if (ret & __NET_XMIT_BYPASS)
 			qdisc_qstats_drop(sch);
 		__qdisc_drop(skb, to_free);
 		return ret;
-#endif
 	} else if ((ret = qdisc_enqueue(skb, cl->leaf.q,
 					to_free)) != NET_XMIT_SUCCESS) {
 		if (net_xmit_drop_count(ret)) {
-- 
2.55.0

Re: [PATCH net v2] net/sched: sch_htb: limit htb_classify inner-class filter hops

[Eric Dumazet]

On Wed, Aug 26, 2026 at 4:33 PM Victor Nogueira <victor@mojatatu.com> wrote:
>
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> htb_classify() follows each filter-selected inner class by switching
> to cl->filter_list, but never bounds the number of hops. A filter on
> an inner class can point back to itself or to another inner class that
> points back, creating an infinite loop in the packet classification
> path with the qdisc lock held and BH disabled — a soft lockup / panic
> from a single packet.

Reviewed-by: Eric Dumazet <edumazet@google.com>

Note that a one-time warning (in the spirit of commit e82181de5ef4648
"pkt_sched: sch_htb: Warn on too many events.") could have been chosen.

Thanks.

Re: [PATCH net v2] net/sched: sch_htb: limit htb_classify inner-class filter hops

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 26 Aug 2026 11:33:39 -0300 you wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
> 
> htb_classify() follows each filter-selected inner class by switching
> to cl->filter_list, but never bounds the number of hops. A filter on
> an inner class can point back to itself or to another inner class that
> points back, creating an infinite loop in the packet classification
> path with the qdisc lock held and BH disabled — a soft lockup / panic
> from a single packet.
> 
> [...]

Here is the summary with links:
  - [net,v2] net/sched: sch_htb: limit htb_classify inner-class filter hops
    https://git.kernel.org/netdev/net/c/729c4896ab82

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html