Re: [PATCH net v2] net: sched: fix quantum/backlog overflow in fq, fq_codel, fq_pie, hhf, sfq

On Wed, Aug 19, 2026 at 5:03 PM Jamal Hadi Salim [off-list ref] wrote:

On Wed, Aug 19, 2026 at 10:43 AM Eric Dumazet [off-list ref] wrote:

quoted

On Wed, Aug 19, 2026 at 4:31 PM Jamal Hadi Salim [off-list ref] wrote:

quoted

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 <redacted>
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,

You should probably split these into separate patches to ease code review.

I didnt follow - are you asking to split the sch_fq into two patches?

No, you send a big patch:

 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(-)

And today I had time to review the FQ part that we care about a lot ;)