Netdev List
 help / color / mirror / Atom feed
From: Jamal Hadi Salim <jhs@mojatatu.com>
To: netdev@vger.kernel.org
Cc: Jamal Hadi Salim <jhs@mojatatu.com>,
	Jiri Pirko <jiri@resnulli.us>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	stable@vger.kernel.org, vega@nebusec.ai,
	Victor Nogueira <victor@mojatatu.com>
Subject: [PATCH net v2] net: sched: fix quantum/backlog overflow in fq, fq_codel, fq_pie, hhf, sfq
Date: Wed, 19 Aug 2026 10:31:35 -0400	[thread overview]
Message-ID: <20260819143136.57350-1-jhs@mojatatu.com> (raw)

Several qdiscs derive their per-flow quantum or backlog from psched_mtu()
or accumulate qdisc_pkt_len() into a u32/int counter without an overflow
or zero clamp, which can drive the dequeue/credit-refill loop into a soft
lockup or a NULL deref.

vega@nebusec.ai provided reports and PoCs which illustrated the following:

- sch_fq: fq_init() computes quantum = 2 * psched_mtu() with no overflow
  check; a huge MTU wraps it into the sign bit, and fq_dequeue()'s
  credit-refill loop spins forever. TCA_FQ_INITIAL_QUANTUM is also
  unbounded up to INT_MAX in fq_change().
- sch_fq_codel: fq_codel_enqueue() accumulates qdisc_pkt_len() into a u32
  per-flow backlog; a crafted TCA_STAB inflates pkt_len to 1 GiB so a few
  packets wrap the counter to 0, and fq_codel_drop() then picks an empty
  flow and derefs NULL. fq_codel_init() also sets quantum =
  psched_mtu() with no clamp, so a huge MTU makes the deficit loop spin.
- sch_fq_pie: fq_pie_init() sets quantum = psched_mtu() with no clamp and
  fq_pie_change() accepts any u32 quantum; same deficit-loop spin as
  fq_codel on a huge-MTU dummy device.
- sch_hhf: hhf_init() sets quantum = psched_mtu() with no overflow check;
  a huge MTU makes weight * quantum overflow the signed deficit, and
  hhf_dequeue() loops forever.
- sch_sfq: sfq_init() sets quantum = psched_mtu() (unsigned); a huge MTU
  makes allot = INT_MIN, and INT_MIN + INT_MIN is UB that toggles between
  INT_MIN and 0 forever.

Clamp the quantum to a sane range at init/change time so the dequeue loops
terminate and the drop path never selects an empty flow. The clamps live in
the init/change paths, not the per-packet fast path, so no hot-path cost is
added for a configuration issue.

Conditions to recreate the bug: a device whose MTU (plus hard_header_len)
is large enough that 2 * psched_mtu() or psched_mtu() wraps into the sign
bit (e.g. a dummy device with max_mtu == 0 accepting a huge MTU), or a
crafted TCA_STAB that inflates qdisc_pkt_len to ~1 GiB. Requires
CAP_NET_ADMIN in a user namespace (Level 2) to configure the qdisc and MTU.

Fixes: afe4fd062416 ("pkt_sched: fq: Fair Queue packet scheduler")
Fixes: 4b549a2ef4be ("fq_codel: Fair Queue Codel AQM")
Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
Fixes: 10239edf86f1 ("net-qdisc-hhf: Heavy-Hitter Filter (HHF) qdisc")
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v1 to v2
Changes based on feedback from Eric and Sashikos[1][2] on V1.

1. Drop the fast-path changes in fq_dequeue() (Eric)

2. Clamp to a range, not just a lower bound (Eric). Uses
   clamp_t(u32, ..., 1, 1 << 20) (upper bound 1M; lower bound 1, not 256).
   Sashiko (gemini, nipa gpt-5-6-sol-1-5) pointed out the v1 max_t() left
   bit 31 set, so a 0x80000000 quantum survived and still spun.
   Upper bound is 1M, not Eric's suggested 16M. fq_change() already caps
   TCA_FQ_QUANTUM at 1M; using 16M in fq_init() would be inconsistent.

3. Clamp TCA_FQ_INITIAL_QUANTUM in fq_change() too (Eric). It was unbounded
   up to INT_MAX; v2 clamps it to > 0 && <= 1M, matching TCA_FQ_QUANTUM.

4. Fold fq_codel_init() quantum clamp (Sashiko gemini + nipa gpt-5-6-sol-1-6)

5. Fold fq_pie_init()/fq_pie_change() quantum clamp (Sashiko gemini):

6. Reword the hhf/sfq comments (Sashiko nipa gpt-5-6-sol-2-15, gpt-5-6-sol-2-14)

[1]https://sashiko.dev/#/patchset/20260818101130.16203-1-jhs@mojatatu.com        
[2]https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260818101130.16203-1-jhs@mojatatu.com
---
 net/sched/sch_fq.c       | 17 ++++++++++++-----
 net/sched/sch_fq_codel.c | 16 ++++++++++++++--
 net/sched/sch_fq_pie.c   |  9 ++++++---
 net/sched/sch_hhf.c      | 12 ++++++++++++
 net/sched/sch_sfq.c      |  8 ++++++++
 5 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 7cae082a9847..f7c92df085e7 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -1112,9 +1112,16 @@ static int fq_change(struct Qdisc *sch, struct nlattr *opt,
 		}
 	}
 
-	if (tb[TCA_FQ_INITIAL_QUANTUM])
-		WRITE_ONCE(q->initial_quantum,
-			   nla_get_u32(tb[TCA_FQ_INITIAL_QUANTUM]));
+	if (tb[TCA_FQ_INITIAL_QUANTUM]) {
+		u32 initial_quantum = nla_get_u32(tb[TCA_FQ_INITIAL_QUANTUM]);
+
+		if (initial_quantum > 0 && initial_quantum <= (1 << 20)) {
+			WRITE_ONCE(q->initial_quantum, initial_quantum);
+		} else {
+			NL_SET_ERR_MSG_MOD(extack, "invalid initial quantum");
+			err = -EINVAL;
+		}
+	}
 
 	if (tb[TCA_FQ_FLOW_DEFAULT_RATE])
 		pr_warn_ratelimited("sch_fq: defrate %u ignored.\n",
@@ -1226,8 +1233,8 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt,
 
 	sch->limit		= 10000;
 	q->flow_plimit		= 100;
-	q->quantum		= 2 * psched_mtu(qdisc_dev(sch));
-	q->initial_quantum	= 10 * psched_mtu(qdisc_dev(sch));
+	q->quantum		= clamp_t(u32, 2 * psched_mtu(qdisc_dev(sch)), 1, 1 << 20);
+	q->initial_quantum	= clamp_t(u32, 10 * psched_mtu(qdisc_dev(sch)), 1, 1 << 20);
 	q->flow_refill_delay	= msecs_to_jiffies(40);
 	q->flow_max_rate	= ~0UL;
 	q->time_next_delayed_flow = ~0ULL;
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 6cce86ba383c..19dde45794a6 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -152,12 +152,24 @@ static unsigned int fq_codel_drop(struct Qdisc *sch, unsigned int max_packets,
 	 * amortizing this linear lookup to one cache line per drop.
 	 */
 	for (i = 0; i < q->flows_cnt; i++) {
-		if (q->backlogs[i] > maxbacklog) {
+		if (q->backlogs[i] > maxbacklog && q->flows[i].head) {
 			maxbacklog = q->backlogs[i];
 			idx = i;
 		}
 	}
 
+	/* TCA_STAB can inflate qdisc_pkt_len enough to wrap per-flow
+	 * backlogs (u32) to zero; fall back to a flow with packets.
+	 */
+	if (maxbacklog == 0) {
+		for (i = 0; i < q->flows_cnt; i++) {
+			if (q->flows[i].head) {
+				idx = i;
+				break;
+			}
+		}
+	}
+
 	/* Our goal is to drop half of this fat flow backlog */
 	threshold = maxbacklog >> 1;
 
@@ -516,7 +528,7 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
 	q->flows_cnt = 1024;
 	q->memory_limit = 32 << 20; /* 32 MBytes */
 	q->drop_batch_size = 64;
-	q->quantum = psched_mtu(qdisc_dev(sch));
+	q->quantum = max(256U, psched_mtu(qdisc_dev(sch)));
 	INIT_LIST_HEAD(&q->new_flows);
 	INIT_LIST_HEAD(&q->old_flows);
 	codel_params_init(&q->cparams);
diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
index 069e1facd413..8fe6634b9aa9 100644
--- a/net/sched/sch_fq_pie.c
+++ b/net/sched/sch_fq_pie.c
@@ -340,8 +340,11 @@ static int fq_pie_change(struct Qdisc *sch, struct nlattr *opt,
 		WRITE_ONCE(q->p_params.beta,
 			   nla_get_u32(tb[TCA_FQ_PIE_BETA]));
 
-	if (tb[TCA_FQ_PIE_QUANTUM])
-		WRITE_ONCE(q->quantum, nla_get_u32(tb[TCA_FQ_PIE_QUANTUM]));
+	if (tb[TCA_FQ_PIE_QUANTUM]) {
+		u32 quantum = max(256U, nla_get_u32(tb[TCA_FQ_PIE_QUANTUM]));
+
+		WRITE_ONCE(q->quantum, quantum);
+	}
 
 	if (tb[TCA_FQ_PIE_MEMORY_LIMIT])
 		WRITE_ONCE(q->memory_limit,
@@ -427,7 +430,7 @@ static int fq_pie_init(struct Qdisc *sch, struct nlattr *opt,
 	pie_params_init(&q->p_params);
 	sch->limit = 10 * 1024;
 	q->p_params.limit = sch->limit;
-	q->quantum = psched_mtu(qdisc_dev(sch));
+	q->quantum = max(256U, psched_mtu(qdisc_dev(sch)));
 	q->sch = sch;
 	q->ecn_prob = 10;
 	q->flows_cnt = 1024;
diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
index d85cb0263b67..483e70df0ce8 100644
--- a/net/sched/sch_hhf.c
+++ b/net/sched/sch_hhf.c
@@ -624,6 +624,18 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
 	q->hhf_evict_timeout = HZ;      /* 1  sec */
 	q->hhf_non_hh_weight = 2;
 
+	if (!opt) {
+		u64 non_hh_quantum = (u64)q->quantum * q->hhf_non_hh_weight;
+
+		/* A device with max_mtu == 0 (e.g. dummy) accepts an MTU that
+		 * makes weight * quantum overflow the signed deficit and spin
+		 * hhf_dequeue() forever. Fall back to a sane minimum quantum
+		 * when the weighted product overflows or is zero.
+		 */
+		if (non_hh_quantum == 0 || non_hh_quantum > INT_MAX)
+			q->quantum = 256;
+	}
+
 	if (opt) {
 		int err = hhf_change(sch, opt, extack);
 
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 77675f9a4c46..0e8d034ef2e3 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -800,6 +800,14 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt,
 	q->divisor = SFQ_DEFAULT_HASH_DIVISOR;
 	q->maxflows = SFQ_DEFAULT_FLOWS;
 	q->quantum = psched_mtu(qdisc_dev(sch));
+	/* A device with max_mtu == 0 (e.g. dummy) accepts an MTU that makes
+	 * psched_mtu() wrap into the sign bit; that would set slot->allot
+	 * negative and spin sfq_dequeue() forever. Fall back to a safe
+	 * positive quantum so the refill loop terminates; the qdisc_pkt_len
+	 * cap in __qdisc_calculate_pkt_len() bounds the deficit depth.
+	 */
+	if ((int)q->quantum <= 0)
+		q->quantum = 10 * 1024;
 	q->perturb_period = 0;
 	get_random_bytes(&q->perturbation, sizeof(q->perturbation));
 
-- 
2.43.0


             reply	other threads:[~2026-08-19 14:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-19 14:31 Jamal Hadi Salim [this message]
2026-08-19 14:42 ` [PATCH net v2] net: sched: fix quantum/backlog overflow in fq, fq_codel, fq_pie, hhf, sfq Eric Dumazet
2026-08-19 15:03   ` Jamal Hadi Salim
2026-08-19 15:18     ` Eric Dumazet
2026-08-19 15:31       ` Jamal Hadi Salim
2026-08-19 16:01         ` Eric Dumazet
2026-08-19 19:44           ` Jamal Hadi Salim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260819143136.57350-1-jhs@mojatatu.com \
    --to=jhs@mojatatu.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=vega@nebusec.ai \
    --cc=victor@mojatatu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox