Thread (6 messages) flat view 6 messages, 2 authors, 3d ago
WARM3d REVIEWED: 2 (0M)

2 review trailers.

[PATCH net v2 1/2] net/sched: sch_hfsc: bound the classify inner-filter walk with a drift budget

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-09-14 08:07:02
Also in: stable
Subsystem: networking [general], tc subsystem, the rest · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim, Jiri Pirko, Linus Torvalds

hfsc_classify() applies the "filter may only point downwards" level
check only when the filter result carries no bound class. A filter
created with a flowid gets res.class set once at bind time, so the
check never runs for it during classification. hfsc_adjust_levels() can
later raise a class's level without revalidating existing bindings,
leaving two binds that were each legal at bind time pointing at each
other; the classify walk then bounces between two interior classes
forever with the qdisc lock held and BH disabled — a soft lockup from a
single packet.

Bound the traversal, mirroring HTB: count only the non-descending hops,
which a legitimate walked tree can only take by level drift after bind
time; anything beyond a small budget is a cycle.
Drop the packet with a rate-limited warning when the budget is
exhausted. Descending hops never consume budget, so legitimately deep
trees are unaffected, and a terminating lateral chain (the level-drift
false-positive family) still classifies normally.

Conditions to recreate the bug:
- CONFIG_NET_SCHED, CONFIG_NET_SCH_HFSC, CONFIG_NET_CLS_U32,
  CONFIG_LOCKUP_DETECTOR.
- Build a cycle with two legal-at-bind-time flowid binds and a level
  drift: class X 1:1 (child of root) with leaf child 1:10; class Y 1:2
  (sibling of X) with children 1:20 and 1:200; root u32 filter flowid
  1:1; filter on X flowid 1:2 (legal when Y is a leaf); after Y's level
  rises to 2, filter on Y flowid 1:1 (legal then). Send one packet (ping
  on the device). Unfixed kernel: classify spins with the qdisc lock
  held; with softlockup_panic=1 it panics.
- Reachable from unprivileged user via unshare -Urn (CAP_NET_ADMIN).

Fixes: a2f79227138c ("net_sched: sch_hfsc: fix classification loops")
Reported-by: Sashiko (gemini + nipa) <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/QDISC-CTUU.v1.20260908094501@mojatatu.com
Cc: stable@vger.kernel.org
Reviewed-by: Victor Nogueira <redacted>
Tested-by: hybris <redacted>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v2 changes:
   -  Replace the absolute hop bound (root->level) with a budget that
      counts only non-descending hops, so a legal lateral level
      chain still classifies. (Sashiko)
   -  Reword the "strict descent" changelog claim (Sashiko).

 net/sched/sch_hfsc.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index e87f5021a199..b073efaf2bf5 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -386,6 +386,12 @@ cftree_update(struct hfsc_class *cl)
 #define	SM_MASK		((1ULL << SM_SHIFT) - 1)
 #define	ISM_MASK	((1ULL << ISM_SHIFT) - 1)
 
+/*
+ * bound on consecutive non-descending (lateral / upward) hops in a
+ * classify walk, mirroring htb's TC_HTB_MAXDEPTH headroom
+ */
+#define	HFSC_CLASSIFY_MAX_DRIFT	8
+
 static inline u64
 seg_x2y(u64 x, u64 sm)
 {
@@ -1133,6 +1139,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 	struct hfsc_class *head, *cl;
 	struct tcf_result res;
 	struct tcf_proto *tcf;
+	unsigned int drift;
 	int result;
 
 	if (TC_H_MAJ(skb->priority ^ sch->handle) == 0 &&
@@ -1142,6 +1149,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
 	head = &q->root;
+	drift = HFSC_CLASSIFY_MAX_DRIFT;
 	tcf = rcu_dereference_bh(q->root.filter_list);
 	while (tcf && (result = tcf_classify_qdisc(skb, tcf, &res, false)) >= 0) {
 #ifdef CONFIG_NET_CLS_ACT
@@ -1167,6 +1175,16 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 		if (cl->level == 0)
 			return cl; /* hit leaf class */
 
+		/*
+		 * flowid binds skip the level check above (res.class is set
+		 * at bind time and levels drift after), so a walk can follow
+		 * lateral hops without descending; bound them here.
+		 */
+		if (cl->level >= head->level && drift-- == 0) {
+			pr_warn_ratelimited("hfsc: classify loop detected, dropping packet\n");
+			return NULL;
+		}
+
 		/* apply inner filter chain */
 		tcf = rcu_dereference_bh(cl->filter_list);
 		head = cl;
-- 
2.43.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help