Re: [PATCH net v2] net: sched: fix quantum/backlog overflow in fq, fq_codel, fq_pie, hhf, sfq
flat view
From: Eric Dumazet <hidden>
Date: 2026-08-19 14:43:06
Also in:
stable
On Wed, Aug 19, 2026 at 4:31 PM Jamal Hadi Salim [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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.
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"")