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

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

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?
My philosophy now with AI bug reports is to group them into a single
patch if they address the same pattern or bug.
Almost 100% of these were caused by people causing cutting and pasting
the same pattern from the original code someone wrote whenever they
add new features.
If you understand one fix in one file, reviewing the rest should be easy.

WDYT?

About TCA_FQ_INITIAL_QUANTUM in fq_change(), you could have changed iq_range
that was added in  commit 7041101ff6c3 ("net/sched: sch_fq: fix
integer overflow of "credit"")

I will look at it if i need to do a v3.

cheers,
jamal