* [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq
@ 2026-08-22 19:55 Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum Jamal Hadi Salim
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-22 19:55 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Mohit P. Tahiliani,
Sachin D . Patil, V. Saicharan, Mohit Bhasi, Leslie Monis,
Gautam Ramakrishnan, Terry Lam, stable, vega, Victor Nogueira
Several qdiscs derive their per-flow quantum or CoDel mtu from
psched_mtu() without an overflow or zero clamp, which can drive the
dequeue/credit-refill loop into a soft lockup or silently disable the
AQM. vega@nebusec.ai provided reports and PoCs for the following qdiscs:
sch_fq, sch_fq_codel, sch_fq_pie, sch_hhf, and sch_sfq.
sch_codel was found by inspection for the same pattern. It's TheLinuxWay
(i.e cutnpaste code from somewhere for your new feature) and the AIs
are having a lot of fun finding patterns. We must overcome!
Clamp the quantum (and, for the codel family, the cparams/params mtu)
to a sane range at init/change time so the dequeue loops terminate and
the AQM stays armed. 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.
This series depends on "net/sched: bound qdisc_pkt_len to prevent qdisc
soft lockup", which caps qdisc_pkt_len() at GSO_MAX_SIZE in
__qdisc_calculate_pkt_len(). That cap closes the fq_codel TCA_STAB
backlog-wrap vector (qdisc_pkt_len inflated to ~1 GiB wrapping the u32
per-flow backlog to 0 and NULL-derefing in fq_codel_drop()); with it
upstream this series no longer needs the fq_codel_drop() hardening hunk
that the earlier respin carried. The five quantum/mtu fixes here are
psched_mtu()-driven and orthogonal to the qdisc_pkt_len() cap.
Q: Why not bound the MTU at the source instead? dummy's max_mtu == 0 is
intentional (dev_validate_mtu() treats 0 as unbounded), other drivers
can legitimately advertise large MTUs, and qdiscs must not trust
psched_mtu() regardless.
Conditions to recreate the bug: a device whose MTU (plus
hard_header_len) wraps 2 * psched_mtu() or psched_mtu() into the sign
bit (e.g. a dummy device with max_mtu == 0 accepting a huge MTU).
Requires CAP_NET_ADMIN in a user namespace.
---
v2 to v3
General: Feeback from Eric and Sashiko and one addtional qdisc from
inspection.
1. Split into one patch per file (Eric Dumazet).
2. Clamp to a range [256, FQ_CODEL_QUANTUM_MAX], not just a lower
bound, in fq_codel/fq_pie init (Sashiko).
3. Clamp psched_mtu() before multiplying in fq_init() (Sashiko).
4. Move hhf clamp before hhf_change() (Sashiko).
5. Fold fq_codel cparams.mtu clamp: same unclamped psched_mtu() six
lines below q->quantum disables codel; hoist one clamped mtu.
6. New patch 3: add sch_codel -- same params.mtu issue.
7. Drop the fq_codel_drop() hardening hunk: the qdisc_pkt_len() cap
in the posted "bound qdisc_pkt_len" dependency closes the TCA_STAB
backlog wrap at the source, making the empty-flow fallback unreachable.
8. Switch sfq to clamp_t(..., 256, 1 << 20) - all patches now
have same pattern.
9. Drop the stale TCA_FQ_INITIAL_QUANTUM narrowing: .max = INT_MAX was
set deliberately by 7041101ff6c3 and already guarantees f->credit
stays non-negative; lowering it would reject working configs.
v1 to v2
Changes based on feedback from Eric and Sashikos on V1.
1. Drop the fast-path changes in fq_dequeue() (Eric).
2. Clamp to a range, not just a lower bound (Eric); upper bound 1M
matches fq_change()'s TCA_FQ_QUANTUM cap, not Eric's 16M.
3. Dropped the TCA_FQ_INITIAL_QUANTUM policy narrowing (see v2 to v3
note 9 for why).
4. Fold fq_codel_init() quantum clamp (Sashiko).
5. Fold fq_pie_init()/fq_pie_change() quantum clamp (Sashiko).
6. Reword the hhf/sfq comments (Sashiko).
Sashiko links:
https://sashiko.dev/#/patchset/20260818101130.16203-1-jhs@mojatatu.com
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260818101130.16203-1-jhs@mojatatu.com
https://sashiko.dev/#/patchset/20260819143136.57350-1-jhs@mojatatu.com
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260819143136.57350-1-jhs@mojatatu.com
---
Jamal Hadi Salim (6):
net/sched: fq: add overflow bounds to quantum and initial quantum
net/sched: fq_codel: clamp default quantum and mtu
net/sched: sch_codel: clamp default mtu to avoid disabling CoDel
net/sched: fq_pie: clamp default quantum to avoid signed overflow
net/sched: hhf: clamp quantum before hhf_change() to avoid overflow
net/sched: sfq: clamp quantum to avoid signed overflow soft lockup
net/sched/sch_codel.c | 2 +-
net/sched/sch_fq.c | 6 ++++--
net/sched/sch_fq_codel.c | 6 ++++--
net/sched/sch_fq_pie.c | 3 ++-
net/sched/sch_hhf.c | 4 ++++
net/sched/sch_sfq.c | 3 ++-
6 files changed, 17 insertions(+), 7 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum
2026-08-22 19:55 [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq Jamal Hadi Salim
@ 2026-08-22 19:55 ` Jamal Hadi Salim
2026-08-25 10:03 ` Eric Dumazet
2026-08-22 19:55 ` [PATCH net v3 2/6] net/sched: fq_codel: clamp default quantum and mtu Jamal Hadi Salim
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-22 19:55 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, stable, vega,
Victor Nogueira
fq_init() computes quantum = 2 * psched_mtu() and initial_quantum = 10 *
psched_mtu() with no overflow check. A device with a huge MTU (e.g. dummy
with max_mtu == 0 accepting MTU 2147483634) makes psched_mtu() return
0x80000000; the 2 * and 10 * multiplications wrap to 0 in 32-bit
arithmetic, so q->quantum == 0. Then in fq_dequeue() the credit-refill
loop adds 0 to f->credit (which stays <= 0) and goto begin loops
forever under the qdisc lock, creating a soft lockup.
Clamp psched_mtu() to [1, 1 << 20] before multiplying so the product
cannot wrap, then cap the result at 1 << 20, matching the bound already
enforced on TCA_FQ_QUANTUM in fq_change().
Conditions to recreate the bug: a device whose MTU (plus
hard_header_len) is large enough that 2 * psched_mtu() wraps (e.g. a
dummy device with max_mtu == 0 accepting MTU 2147483634). Requires
CAP_NET_ADMIN in a user namespace.
Fixes: afe4fd062416 ("pkt_sched: fq: Fair Queue packet scheduler")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_fq.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 7cae082a9847..8a071c35b869 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -1222,12 +1222,14 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt,
struct netlink_ext_ack *extack)
{
struct fq_sched_data *q = qdisc_priv(sch);
+ u32 mtu;
int i, err;
sch->limit = 10000;
q->flow_plimit = 100;
- q->quantum = 2 * psched_mtu(qdisc_dev(sch));
- q->initial_quantum = 10 * psched_mtu(qdisc_dev(sch));
+ mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 1, 1 << 20);
+ q->quantum = min_t(u32, 2 * mtu, 1 << 20);
+ q->initial_quantum = min_t(u32, 10 * mtu, 1 << 20);
q->flow_refill_delay = msecs_to_jiffies(40);
q->flow_max_rate = ~0UL;
q->time_next_delayed_flow = ~0ULL;
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net v3 2/6] net/sched: fq_codel: clamp default quantum and mtu
2026-08-22 19:55 [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum Jamal Hadi Salim
@ 2026-08-22 19:55 ` Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 3/6] net/sched: sch_codel: clamp default mtu to avoid disabling CoDel Jamal Hadi Salim
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-22 19:55 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, stable, vega,
Victor Nogueira
fq_codel_init() sets q->quantum = psched_mtu(qdisc_dev(sch)) without
clamping. A device with a huge MTU (e.g. dummy with max_mtu == 0
accepting MTU 2147483634) makes psched_mtu() return 0x80000000, which
overflows the signed flow->deficit to INT_MIN in fq_codel_dequeue(),
causing an infinite loop and soft lockup. Emulate fq_codel_change()
and constrain to [256, FQ_CODEL_QUANTUM_MAX].
The same unclamped psched_mtu() is assigned to q->cparams.mtu a bit
below, and fq_codel_change() never updates it. codel_should_drop()
tests "*backlog <= params->mtu"; with mtu == 0x80000000 (~2 GiB) and
the default 32 MiB memory_limit, the test is always true, so CoDel is
silently and completely disabled (no drops, no ECN). Declare a single
clamped mtu and assign both q->quantum and q->cparams.mtu from it,
which also removes the double psched_mtu() call.
Conditions to recreate the bug: a device whose MTU (plus
hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
device with max_mtu == 0 accepting MTU 2147483634). Requires
CAP_NET_ADMIN in a user namespace.
Fixes: 4b549a2ef4be ("fq_codel: Fair Queue Codel AQM")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_fq_codel.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 6cce86ba383c..969b2510b0b8 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -509,6 +509,7 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
struct netlink_ext_ack *extack)
{
struct fq_codel_sched_data *q = qdisc_priv(sch);
+ u32 mtu;
int i;
int err;
@@ -516,13 +517,14 @@ 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));
+ mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 256, FQ_CODEL_QUANTUM_MAX);
+ q->quantum = mtu;
INIT_LIST_HEAD(&q->new_flows);
INIT_LIST_HEAD(&q->old_flows);
codel_params_init(&q->cparams);
codel_stats_init(&q->cstats);
q->cparams.ecn = true;
- q->cparams.mtu = psched_mtu(qdisc_dev(sch));
+ q->cparams.mtu = mtu;
if (opt) {
err = fq_codel_change(sch, opt, extack);
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net v3 3/6] net/sched: sch_codel: clamp default mtu to avoid disabling CoDel
2026-08-22 19:55 [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 2/6] net/sched: fq_codel: clamp default quantum and mtu Jamal Hadi Salim
@ 2026-08-22 19:55 ` Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow Jamal Hadi Salim
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-22 19:55 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, stable, vega,
Victor Nogueira
codel_init() sets q->params.mtu = psched_mtu(qdisc_dev(sch)) without
clamping. A device with a huge MTU (e.g. dummy with max_mtu == 0
accepting MTU 2147483634) makes psched_mtu() return 0x80000000. In
codel_should_drop() the test "*backlog <= params->mtu" then compares
the backlog against ~2 GiB; with the default sch->limit of
DEFAULT_CODEL_LIMIT (1000) packets the backlog can never reach it, so
the test is always true and CoDel is silently and completely disabled
i.e no drops, no ECN marking, codel degrades to a tail-drop FIFO.
codel_change() never updates params.mtu, so the init path is the only
place to clamp it. Constrain to [256, 1 << 20], matching the fq_codel
bound; 256 is a sane floor that only makes CoDel slightly more willing
to act on very small queues, which is the safe direction.
Conditions to recreate the bug: a device whose MTU (plus
hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
device with max_mtu == 0 accepting MTU 2147483634). Requires
CAP_NET_ADMIN in a user namespace.
Fixes: 76e3cc126bb2 ("codel: Controlled Delay AQM")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_codel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
index cacf5244958e..6aa5829d6961 100644
--- a/net/sched/sch_codel.c
+++ b/net/sched/sch_codel.c
@@ -205,7 +205,7 @@ static int codel_init(struct Qdisc *sch, struct nlattr *opt,
codel_params_init(&q->params);
codel_vars_init(&q->vars);
codel_stats_init(&q->stats);
- q->params.mtu = psched_mtu(qdisc_dev(sch));
+ q->params.mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 256, 1 << 20);
if (opt) {
int err = codel_change(sch, opt, extack);
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow
2026-08-22 19:55 [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq Jamal Hadi Salim
` (2 preceding siblings ...)
2026-08-22 19:55 ` [PATCH net v3 3/6] net/sched: sch_codel: clamp default mtu to avoid disabling CoDel Jamal Hadi Salim
@ 2026-08-22 19:55 ` Jamal Hadi Salim
2026-08-25 8:33 ` Paolo Abeni
2026-08-22 19:55 ` [PATCH net v3 5/6] net/sched: hhf: clamp quantum before hhf_change() to avoid overflow Jamal Hadi Salim
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-22 19:55 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Mohit P. Tahiliani,
Sachin D . Patil, V. Saicharan, Mohit Bhasi, Leslie Monis,
Gautam Ramakrishnan, stable, vega, Victor Nogueira
fq_pie_init() sets q->quantum = psched_mtu(qdisc_dev(sch)) without
clamping. A device with a huge MTU (e.g. dummy with max_mtu == 0
accepting MTU 2147483634) makes psched_mtu() return 0x80000000, which
overflows the signed flow->deficit to INT_MIN in fq_pie_qdisc_dequeue(),
causing an infinite loop and soft lockup. Emulate fq_pie_policy which
is already bounded to [1, 1 << 20]; clamp the default to [256, 1 << 20].
256 matches fq_codel's floor and is a sane minimum for a DRR quantum.
Conditions to recreate the bug: a device whose MTU (plus
hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
device with max_mtu == 0 accepting MTU 2147483634). Requires
CAP_NET_ADMIN in a user namespace.
Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_fq_pie.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
index 069e1facd413..b27d95418707 100644
--- a/net/sched/sch_fq_pie.c
+++ b/net/sched/sch_fq_pie.c
@@ -427,7 +427,8 @@ 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 = clamp_t(u32, psched_mtu(qdisc_dev(sch)),
+ 256, 1 << 20);
q->sch = sch;
q->ecn_prob = 10;
q->flows_cnt = 1024;
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net v3 5/6] net/sched: hhf: clamp quantum before hhf_change() to avoid overflow
2026-08-22 19:55 [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq Jamal Hadi Salim
` (3 preceding siblings ...)
2026-08-22 19:55 ` [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow Jamal Hadi Salim
@ 2026-08-22 19:55 ` Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 6/6] net/sched: sfq: clamp quantum to avoid signed overflow soft lockup Jamal Hadi Salim
2026-08-25 11:30 ` [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq patchwork-bot+netdevbpf
6 siblings, 0 replies; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-22 19:55 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Terry Lam, stable,
vega, Victor Nogueira
hhf_init() sets q->quantum = psched_mtu(qdisc_dev(sch)) with no overflow
check. A device with a huge MTU (e.g. dummy with max_mtu == 0 accepting
MTU 2147483634) makes weight * quantum overflow the signed deficit in
hhf_dequeue(), spinning forever.
Clamp q->quantum before hhf_change() so both the opt and !opt paths see
a sane quantum. Without this, bare "tc qdisc add ... hhf" succeeds with
a clamped quantum but "tc qdisc add ... hhf limit 1000" (any option
present) fails with -EINVAL because hhf_change() re-validates the
unclamped default (sch_hhf.c:559). 256 matches fq_codel's floor and is
a sane minimum for a DRR quantum.
Conditions to recreate the bug: a device whose MTU (plus
hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
device with max_mtu == 0 accepting MTU 2147483634). Requires
CAP_NET_ADMIN in a user namespace.
Fixes: 10239edf86f1 ("net-qdisc-hhf: Heavy-Hitter Filter (HHF) qdisc")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_hhf.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
index d85cb0263b67..96acab6a8da0 100644
--- a/net/sched/sch_hhf.c
+++ b/net/sched/sch_hhf.c
@@ -624,6 +624,10 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
q->hhf_evict_timeout = HZ; /* 1 sec */
q->hhf_non_hh_weight = 2;
+ if ((int)q->quantum <= 0 ||
+ (u64)q->quantum * q->hhf_non_hh_weight > INT_MAX)
+ q->quantum = 256;
+
if (opt) {
int err = hhf_change(sch, opt, extack);
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net v3 6/6] net/sched: sfq: clamp quantum to avoid signed overflow soft lockup
2026-08-22 19:55 [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq Jamal Hadi Salim
` (4 preceding siblings ...)
2026-08-22 19:55 ` [PATCH net v3 5/6] net/sched: hhf: clamp quantum before hhf_change() to avoid overflow Jamal Hadi Salim
@ 2026-08-22 19:55 ` Jamal Hadi Salim
2026-08-25 11:30 ` [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq patchwork-bot+netdevbpf
6 siblings, 0 replies; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-22 19:55 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, stable, vega,
Victor Nogueira
sfq_init() sets q->quantum = psched_mtu(qdisc_dev(sch)) (unsigned). A
device with a huge MTU (e.g. dummy with max_mtu == 0 accepting MTU
2147483634) makes psched_mtu() return 0x80000000, so slot->allot = INT_MIN
and INT_MIN + INT_MIN toggles between INT_MIN and 0 forever, spinning
sfq_dequeue() under the qdisc lock.
Clamp the quantum to [256, 1 << 20] so the refill loop terminates. The
lower bound also covers q->quantum == 0 (psched_mtu() returning 0),
which spins sfq_dequeue() identically. sfq_change() already rejects a
negative quantum, so only the init path was exposed.
Conditions to recreate the bug: a device whose MTU (plus
hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
device with max_mtu == 0 accepting MTU 2147483634). Requires
CAP_NET_ADMIN in a user namespace.
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>
---
net/sched/sch_sfq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 77675f9a4c46..187d3ed578f2 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -799,7 +799,8 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt,
q->tail = NULL;
q->divisor = SFQ_DEFAULT_HASH_DIVISOR;
q->maxflows = SFQ_DEFAULT_FLOWS;
- q->quantum = psched_mtu(qdisc_dev(sch));
+ q->quantum = clamp_t(u32, psched_mtu(qdisc_dev(sch)),
+ 256, 1 << 20);
q->perturb_period = 0;
get_random_bytes(&q->perturbation, sizeof(q->perturbation));
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow
2026-08-22 19:55 ` [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow Jamal Hadi Salim
@ 2026-08-25 8:33 ` Paolo Abeni
2026-08-25 9:16 ` Jamal Hadi Salim
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Abeni @ 2026-08-25 8:33 UTC (permalink / raw)
To: Jamal Hadi Salim, netdev
Cc: Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski,
Simon Horman, Mohit P. Tahiliani, Sachin D . Patil, V. Saicharan,
Mohit Bhasi, Leslie Monis, Gautam Ramakrishnan, stable, vega,
Victor Nogueira
Hi,
On 8/22/26 9:55 PM, Jamal Hadi Salim wrote:
> fq_pie_init() sets q->quantum = psched_mtu(qdisc_dev(sch)) without
> clamping. A device with a huge MTU (e.g. dummy with max_mtu == 0
> accepting MTU 2147483634) makes psched_mtu() return 0x80000000, which
> overflows the signed flow->deficit to INT_MIN in fq_pie_qdisc_dequeue(),
> causing an infinite loop and soft lockup. Emulate fq_pie_policy which
> is already bounded to [1, 1 << 20]; clamp the default to [256, 1 << 20].
> 256 matches fq_codel's floor and is a sane minimum for a DRR quantum.
>
> Conditions to recreate the bug: a device whose MTU (plus
> hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
> device with max_mtu == 0 accepting MTU 2147483634). Requires
> CAP_NET_ADMIN in a user namespace.
>
> Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
> Reported-by: vega@nebusec.ai
> Tested-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> net/sched/sch_fq_pie.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
> index 069e1facd413..b27d95418707 100644
> --- a/net/sched/sch_fq_pie.c
> +++ b/net/sched/sch_fq_pie.c
> @@ -427,7 +427,8 @@ 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 = clamp_t(u32, psched_mtu(qdisc_dev(sch)),
> + 256, 1 << 20);
Sashiko thinks that the soft lookup is still reachable via pie_change:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260822195509.112717-1-jhs%40mojatatu.com
and has similar concerns for patch 6/6, too. It marks the issues as
pre-existing, but AFAICS they overlap with the things addressed here.
WDYT?
/P
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow
2026-08-25 8:33 ` Paolo Abeni
@ 2026-08-25 9:16 ` Jamal Hadi Salim
2026-08-25 9:43 ` Paolo Abeni
0 siblings, 1 reply; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-25 9:16 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski,
Simon Horman, Mohit P. Tahiliani, Sachin D . Patil, V. Saicharan,
Mohit Bhasi, Leslie Monis, Gautam Ramakrishnan, stable, vega,
Victor Nogueira
On Tue, Aug 25, 2026 at 4:33 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> On 8/22/26 9:55 PM, Jamal Hadi Salim wrote:
> > fq_pie_init() sets q->quantum = psched_mtu(qdisc_dev(sch)) without
> > clamping. A device with a huge MTU (e.g. dummy with max_mtu == 0
> > accepting MTU 2147483634) makes psched_mtu() return 0x80000000, which
> > overflows the signed flow->deficit to INT_MIN in fq_pie_qdisc_dequeue(),
> > causing an infinite loop and soft lockup. Emulate fq_pie_policy which
> > is already bounded to [1, 1 << 20]; clamp the default to [256, 1 << 20].
> > 256 matches fq_codel's floor and is a sane minimum for a DRR quantum.
> >
> > Conditions to recreate the bug: a device whose MTU (plus
> > hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
> > device with max_mtu == 0 accepting MTU 2147483634). Requires
> > CAP_NET_ADMIN in a user namespace.
> >
> > Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
> > Reported-by: vega@nebusec.ai
> > Tested-by: Victor Nogueira <victor@mojatatu.com>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > ---
> > net/sched/sch_fq_pie.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
> > index 069e1facd413..b27d95418707 100644
> > --- a/net/sched/sch_fq_pie.c
> > +++ b/net/sched/sch_fq_pie.c
> > @@ -427,7 +427,8 @@ 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 = clamp_t(u32, psched_mtu(qdisc_dev(sch)),
> > + 256, 1 << 20);
>
> Sashiko thinks that the soft lookup is still reachable via pie_change:
>
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260822195509.112717-1-jhs%40mojatatu.com
>
> and has similar concerns for patch 6/6, too. It marks the issues as
> pre-existing, but AFAICS they overlap with the things addressed here.
>
> WDYT?
You are right, they overlap. I had them as followups (with a few
others derived from the sashiko feedback with justification that the
v3 init-path clamps are independently correct and the stab cap already
mitigates the change-path worst case to a stall; but those two a
(adding max(256U, ...) to both fq_pie_change() and sfq_change(),
matching the fq_codel_change()) are more serious.
So if you'd prefer a v4 respin of the whole series, I can do that.
Sashiko is a double edge sword - i think code quality is improving but
it feels like the work load has doubled ;->
Here's what i had as followups (some still to be vetted, just noting
what sashiko is stating to be reviewed later when cycles available and
potential followup patches sent):
- sch_dualpi2 unclamped psched_mtu
- sch_pie unclamped psched_mtu → AQM disable / div-by-zero
- hhf TCA_HHF_HH_FLOWS_LIMIT unbounded
- fq_pie_change() / sfq_change() 256 floor missing (one that you bring up here)
- DRR/ETS quantum=0 spin (have a patch, was reported already as a bug by vega@)
- Consider two separate clamps for fq_codel/sch_codel (nipa
gpt-5-6-sol-3-15): quantum in [256, FQ_CODEL_QUANTUM_MAX], mtubounded
separately (no 256 floor on mtu)
cheers,
jamal
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow
2026-08-25 9:16 ` Jamal Hadi Salim
@ 2026-08-25 9:43 ` Paolo Abeni
2026-08-25 9:48 ` Jamal Hadi Salim
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Abeni @ 2026-08-25 9:43 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski,
Simon Horman, Mohit P. Tahiliani, Sachin D . Patil, V. Saicharan,
Mohit Bhasi, Leslie Monis, Gautam Ramakrishnan, stable, vega,
Victor Nogueira
On 8/25/26 11:16 AM, Jamal Hadi Salim wrote:
> On Tue, Aug 25, 2026 at 4:33 AM Paolo Abeni <pabeni@redhat.com> wrote:
>> On 8/22/26 9:55 PM, Jamal Hadi Salim wrote:
>>> fq_pie_init() sets q->quantum = psched_mtu(qdisc_dev(sch)) without
>>> clamping. A device with a huge MTU (e.g. dummy with max_mtu == 0
>>> accepting MTU 2147483634) makes psched_mtu() return 0x80000000, which
>>> overflows the signed flow->deficit to INT_MIN in fq_pie_qdisc_dequeue(),
>>> causing an infinite loop and soft lockup. Emulate fq_pie_policy which
>>> is already bounded to [1, 1 << 20]; clamp the default to [256, 1 << 20].
>>> 256 matches fq_codel's floor and is a sane minimum for a DRR quantum.
>>>
>>> Conditions to recreate the bug: a device whose MTU (plus
>>> hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
>>> device with max_mtu == 0 accepting MTU 2147483634). Requires
>>> CAP_NET_ADMIN in a user namespace.
>>>
>>> Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
>>> Reported-by: vega@nebusec.ai
>>> Tested-by: Victor Nogueira <victor@mojatatu.com>
>>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>>> ---
>>> net/sched/sch_fq_pie.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
>>> index 069e1facd413..b27d95418707 100644
>>> --- a/net/sched/sch_fq_pie.c
>>> +++ b/net/sched/sch_fq_pie.c
>>> @@ -427,7 +427,8 @@ 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 = clamp_t(u32, psched_mtu(qdisc_dev(sch)),
>>> + 256, 1 << 20);
>>
>> Sashiko thinks that the soft lookup is still reachable via pie_change:
>>
>> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260822195509.112717-1-jhs%40mojatatu.com
>>
>> and has similar concerns for patch 6/6, too. It marks the issues as
>> pre-existing, but AFAICS they overlap with the things addressed here.
>>
>> WDYT?
>
> You are right, they overlap. I had them as followups (with a few
> others derived from the sashiko feedback with justification that the
> v3 init-path clamps are independently correct and the stab cap already
> mitigates the change-path worst case to a stall; but those two a
> (adding max(256U, ...) to both fq_pie_change() and sfq_change(),
> matching the fq_codel_change()) are more serious.
> So if you'd prefer a v4 respin of the whole series, I can do that.
I initially did not notice that the _change path would lead to
"upper-bounded" stall, I think a follow-up is fine.
> Sashiko is a double edge sword - i think code quality is improving but
> it feels like the work load has doubled ;->
FWIW, I agree with the "double edge" assessment.
A reference we must keep in mind is that there is no way back, so we
need to adapt somehow.
> Here's what i had as followups (some still to be vetted, just noting
> what sashiko is stating to be reviewed later when cycles available and
> potential followup patches sent):
> - sch_dualpi2 unclamped psched_mtu
> - sch_pie unclamped psched_mtu → AQM disable / div-by-zero
> - hhf TCA_HHF_HH_FLOWS_LIMIT unbounded
> - fq_pie_change() / sfq_change() 256 floor missing (one that you bring up here)
> - DRR/ETS quantum=0 spin (have a patch, was reported already as a bug by vega@)
> - Consider two separate clamps for fq_codel/sch_codel (nipa
> gpt-5-6-sol-3-15): quantum in [256, FQ_CODEL_QUANTUM_MAX], mtubounded
> separately (no 256 floor on mtu)
FWIW, LGTM!
/P
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow
2026-08-25 9:43 ` Paolo Abeni
@ 2026-08-25 9:48 ` Jamal Hadi Salim
0 siblings, 0 replies; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-25 9:48 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski,
Simon Horman, Mohit P. Tahiliani, Sachin D . Patil, V. Saicharan,
Mohit Bhasi, Leslie Monis, Gautam Ramakrishnan, stable, vega,
Victor Nogueira
On Tue, Aug 25, 2026 at 5:43 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 8/25/26 11:16 AM, Jamal Hadi Salim wrote:
> > On Tue, Aug 25, 2026 at 4:33 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >> On 8/22/26 9:55 PM, Jamal Hadi Salim wrote:
> >>> fq_pie_init() sets q->quantum = psched_mtu(qdisc_dev(sch)) without
> >>> clamping. A device with a huge MTU (e.g. dummy with max_mtu == 0
> >>> accepting MTU 2147483634) makes psched_mtu() return 0x80000000, which
> >>> overflows the signed flow->deficit to INT_MIN in fq_pie_qdisc_dequeue(),
> >>> causing an infinite loop and soft lockup. Emulate fq_pie_policy which
> >>> is already bounded to [1, 1 << 20]; clamp the default to [256, 1 << 20].
> >>> 256 matches fq_codel's floor and is a sane minimum for a DRR quantum.
> >>>
> >>> Conditions to recreate the bug: a device whose MTU (plus
> >>> hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
> >>> device with max_mtu == 0 accepting MTU 2147483634). Requires
> >>> CAP_NET_ADMIN in a user namespace.
> >>>
> >>> Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
> >>> Reported-by: vega@nebusec.ai
> >>> Tested-by: Victor Nogueira <victor@mojatatu.com>
> >>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> >>> ---
> >>> net/sched/sch_fq_pie.c | 3 ++-
> >>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
> >>> index 069e1facd413..b27d95418707 100644
> >>> --- a/net/sched/sch_fq_pie.c
> >>> +++ b/net/sched/sch_fq_pie.c
> >>> @@ -427,7 +427,8 @@ 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 = clamp_t(u32, psched_mtu(qdisc_dev(sch)),
> >>> + 256, 1 << 20);
> >>
> >> Sashiko thinks that the soft lookup is still reachable via pie_change:
> >>
> >> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260822195509.112717-1-jhs%40mojatatu.com
> >>
> >> and has similar concerns for patch 6/6, too. It marks the issues as
> >> pre-existing, but AFAICS they overlap with the things addressed here.
> >>
> >> WDYT?
> >
> > You are right, they overlap. I had them as followups (with a few
> > others derived from the sashiko feedback with justification that the
> > v3 init-path clamps are independently correct and the stab cap already
> > mitigates the change-path worst case to a stall; but those two a
> > (adding max(256U, ...) to both fq_pie_change() and sfq_change(),
> > matching the fq_codel_change()) are more serious.
> > So if you'd prefer a v4 respin of the whole series, I can do that.
>
> I initially did not notice that the _change path would lead to
> "upper-bounded" stall, I think a follow-up is fine.
I will prioritize moving those two i mentioned as a followup as soon
as this patchset goes in.
> > Sashiko is a double edge sword - i think code quality is improving but
> > it feels like the work load has doubled ;->
>
> FWIW, I agree with the "double edge" assessment.
> A reference we must keep in mind is that there is no way back, so we
> need to adapt somehow.
>
Agreed ;-> Will take a while to adopt...
cheers,
jamal
> > Here's what i had as followups (some still to be vetted, just noting
> > what sashiko is stating to be reviewed later when cycles available and
> > potential followup patches sent):
> > - sch_dualpi2 unclamped psched_mtu
> > - sch_pie unclamped psched_mtu → AQM disable / div-by-zero
> > - hhf TCA_HHF_HH_FLOWS_LIMIT unbounded
> > - fq_pie_change() / sfq_change() 256 floor missing (one that you bring up here)
> > - DRR/ETS quantum=0 spin (have a patch, was reported already as a bug by vega@)
> > - Consider two separate clamps for fq_codel/sch_codel (nipa
> > gpt-5-6-sol-3-15): quantum in [256, FQ_CODEL_QUANTUM_MAX], mtubounded
> > separately (no 256 floor on mtu)
> FWIW, LGTM!
>
> /P
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum
2026-08-22 19:55 ` [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum Jamal Hadi Salim
@ 2026-08-25 10:03 ` Eric Dumazet
2026-08-25 10:43 ` Jamal Hadi Salim
0 siblings, 1 reply; 15+ messages in thread
From: Eric Dumazet @ 2026-08-25 10:03 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, Jiri Pirko, David S. Miller, Jakub Kicinski, Paolo Abeni,
Simon Horman, stable, vega, Victor Nogueira
On Sat, Aug 22, 2026 at 9:55 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> fq_init() computes quantum = 2 * psched_mtu() and initial_quantum = 10 *
> psched_mtu() with no overflow check. A device with a huge MTU (e.g. dummy
> with max_mtu == 0 accepting MTU 2147483634) makes psched_mtu() return
> 0x80000000; the 2 * and 10 * multiplications wrap to 0 in 32-bit
> arithmetic, so q->quantum == 0. Then in fq_dequeue() the credit-refill
> loop adds 0 to f->credit (which stays <= 0) and goto begin loops
> forever under the qdisc lock, creating a soft lockup.
>
> Clamp psched_mtu() to [1, 1 << 20] before multiplying so the product
> cannot wrap, then cap the result at 1 << 20, matching the bound already
> enforced on TCA_FQ_QUANTUM in fq_change().
>
> Conditions to recreate the bug: a device whose MTU (plus
> hard_header_len) is large enough that 2 * psched_mtu() wraps (e.g. a
> dummy device with max_mtu == 0 accepting MTU 2147483634). Requires
> CAP_NET_ADMIN in a user namespace.
>
> Fixes: afe4fd062416 ("pkt_sched: fq: Fair Queue packet scheduler")
> Reported-by: vega@nebusec.ai
> Tested-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> net/sched/sch_fq.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
> index 7cae082a9847..8a071c35b869 100644
> --- a/net/sched/sch_fq.c
> +++ b/net/sched/sch_fq.c
> @@ -1222,12 +1222,14 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt,
> struct netlink_ext_ack *extack)
> {
> struct fq_sched_data *q = qdisc_priv(sch);
> + u32 mtu;
> int i, err;
>
> sch->limit = 10000;
> q->flow_plimit = 100;
> - q->quantum = 2 * psched_mtu(qdisc_dev(sch));
> - q->initial_quantum = 10 * psched_mtu(qdisc_dev(sch));
> + mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 1, 1 << 20);
> + q->quantum = min_t(u32, 2 * mtu, 1 << 20);
> + q->initial_quantum = min_t(u32, 10 * mtu, 1 << 20);
> q->flow_refill_delay = msecs_to_jiffies(40);
> q->flow_max_rate = ~0UL;
> q->time_next_delayed_flow = ~0ULL;
Note that after FQ qdisc has been created, it can be changed, and
fq_change() and/or iq_range
need to be fixed.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum
2026-08-25 10:03 ` Eric Dumazet
@ 2026-08-25 10:43 ` Jamal Hadi Salim
2026-08-25 11:02 ` Jamal Hadi Salim
0 siblings, 1 reply; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-25 10:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, Jiri Pirko, David S. Miller, Jakub Kicinski, Paolo Abeni,
Simon Horman, stable, vega, Victor Nogueira
On Tue, Aug 25, 2026 at 6:03 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Sat, Aug 22, 2026 at 9:55 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > fq_init() computes quantum = 2 * psched_mtu() and initial_quantum = 10 *
> > psched_mtu() with no overflow check. A device with a huge MTU (e.g. dummy
> > with max_mtu == 0 accepting MTU 2147483634) makes psched_mtu() return
> > 0x80000000; the 2 * and 10 * multiplications wrap to 0 in 32-bit
> > arithmetic, so q->quantum == 0. Then in fq_dequeue() the credit-refill
> > loop adds 0 to f->credit (which stays <= 0) and goto begin loops
> > forever under the qdisc lock, creating a soft lockup.
> >
> > Clamp psched_mtu() to [1, 1 << 20] before multiplying so the product
> > cannot wrap, then cap the result at 1 << 20, matching the bound already
> > enforced on TCA_FQ_QUANTUM in fq_change().
> >
> > Conditions to recreate the bug: a device whose MTU (plus
> > hard_header_len) is large enough that 2 * psched_mtu() wraps (e.g. a
> > dummy device with max_mtu == 0 accepting MTU 2147483634). Requires
> > CAP_NET_ADMIN in a user namespace.
> >
> > Fixes: afe4fd062416 ("pkt_sched: fq: Fair Queue packet scheduler")
> > Reported-by: vega@nebusec.ai
> > Tested-by: Victor Nogueira <victor@mojatatu.com>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > ---
> > net/sched/sch_fq.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
> > index 7cae082a9847..8a071c35b869 100644
> > --- a/net/sched/sch_fq.c
> > +++ b/net/sched/sch_fq.c
> > @@ -1222,12 +1222,14 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt,
> > struct netlink_ext_ack *extack)
> > {
> > struct fq_sched_data *q = qdisc_priv(sch);
> > + u32 mtu;
> > int i, err;
> >
> > sch->limit = 10000;
> > q->flow_plimit = 100;
> > - q->quantum = 2 * psched_mtu(qdisc_dev(sch));
> > - q->initial_quantum = 10 * psched_mtu(qdisc_dev(sch));
> > + mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 1, 1 << 20);
> > + q->quantum = min_t(u32, 2 * mtu, 1 << 20);
> > + q->initial_quantum = min_t(u32, 10 * mtu, 1 << 20);
> > q->flow_refill_delay = msecs_to_jiffies(40);
> > q->flow_max_rate = ~0UL;
> > q->time_next_delayed_flow = ~0ULL;
>
> Note that after FQ qdisc has been created, it can be changed, and
> fq_change() and/or iq_range
> need to be fixed.
Right. TCA_FQ_QUANTUM is already bounded to (0, 1<<20] in fq_change(),
but TCA_FQ_INITIAL_QUANTUM goes through iq_range which has .max =
INT_MAX — so fq_change() accepts values up to INT_MAX while init now
clamps to 1<<20.
I'll fold that into the follow-up patch I'm preparing for the
fq_pie_change()/sfq_change() 256-floor gaps Paolo flagged — same
pattern (init clamped, change path not). Narrowing iq_range.max to
1<<20 will reject at parse time, matching the init clamp.
cheers,
jamal
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum
2026-08-25 10:43 ` Jamal Hadi Salim
@ 2026-08-25 11:02 ` Jamal Hadi Salim
0 siblings, 0 replies; 15+ messages in thread
From: Jamal Hadi Salim @ 2026-08-25 11:02 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, Jiri Pirko, David S. Miller, Jakub Kicinski, Paolo Abeni,
Simon Horman, stable, vega, Victor Nogueira
[-- Attachment #1: Type: text/plain, Size: 3466 bytes --]
On Tue, Aug 25, 2026 at 6:43 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Tue, Aug 25, 2026 at 6:03 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Sat, Aug 22, 2026 at 9:55 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > >
> > > fq_init() computes quantum = 2 * psched_mtu() and initial_quantum = 10 *
> > > psched_mtu() with no overflow check. A device with a huge MTU (e.g. dummy
> > > with max_mtu == 0 accepting MTU 2147483634) makes psched_mtu() return
> > > 0x80000000; the 2 * and 10 * multiplications wrap to 0 in 32-bit
> > > arithmetic, so q->quantum == 0. Then in fq_dequeue() the credit-refill
> > > loop adds 0 to f->credit (which stays <= 0) and goto begin loops
> > > forever under the qdisc lock, creating a soft lockup.
> > >
> > > Clamp psched_mtu() to [1, 1 << 20] before multiplying so the product
> > > cannot wrap, then cap the result at 1 << 20, matching the bound already
> > > enforced on TCA_FQ_QUANTUM in fq_change().
> > >
> > > Conditions to recreate the bug: a device whose MTU (plus
> > > hard_header_len) is large enough that 2 * psched_mtu() wraps (e.g. a
> > > dummy device with max_mtu == 0 accepting MTU 2147483634). Requires
> > > CAP_NET_ADMIN in a user namespace.
> > >
> > > Fixes: afe4fd062416 ("pkt_sched: fq: Fair Queue packet scheduler")
> > > Reported-by: vega@nebusec.ai
> > > Tested-by: Victor Nogueira <victor@mojatatu.com>
> > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > > ---
> > > net/sched/sch_fq.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
> > > index 7cae082a9847..8a071c35b869 100644
> > > --- a/net/sched/sch_fq.c
> > > +++ b/net/sched/sch_fq.c
> > > @@ -1222,12 +1222,14 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt,
> > > struct netlink_ext_ack *extack)
> > > {
> > > struct fq_sched_data *q = qdisc_priv(sch);
> > > + u32 mtu;
> > > int i, err;
> > >
> > > sch->limit = 10000;
> > > q->flow_plimit = 100;
> > > - q->quantum = 2 * psched_mtu(qdisc_dev(sch));
> > > - q->initial_quantum = 10 * psched_mtu(qdisc_dev(sch));
> > > + mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 1, 1 << 20);
> > > + q->quantum = min_t(u32, 2 * mtu, 1 << 20);
> > > + q->initial_quantum = min_t(u32, 10 * mtu, 1 << 20);
> > > q->flow_refill_delay = msecs_to_jiffies(40);
> > > q->flow_max_rate = ~0UL;
> > > q->time_next_delayed_flow = ~0ULL;
> >
> > Note that after FQ qdisc has been created, it can be changed, and
> > fq_change() and/or iq_range
> > need to be fixed.
>
> Right. TCA_FQ_QUANTUM is already bounded to (0, 1<<20] in fq_change(),
> but TCA_FQ_INITIAL_QUANTUM goes through iq_range which has .max =
> INT_MAX — so fq_change() accepts values up to INT_MAX while init now
> clamps to 1<<20.
>
> I'll fold that into the follow-up patch I'm preparing for the
> fq_pie_change()/sfq_change() 256-floor gaps Paolo flagged — same
> pattern (init clamped, change path not). Narrowing iq_range.max to
> 1<<20 will reject at parse time, matching the init clamp.
>
Something like attached - untested. Sigh, i think i had what you are
asking for in v2 but lost it in translation to v3.
cheers,
jamal
> cheers,
> jamal
[-- Attachment #2: p1 --]
[-- Type: application/octet-stream, Size: 1373 bytes --]
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 6144b5686f13..ab8e7c6ae203 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -980,7 +980,7 @@ static int fq_resize(struct Qdisc *sch, u32 log)
}
static const struct netlink_range_validation iq_range = {
- .max = INT_MAX,
+ .max = 1 << 20,
};
static const struct nla_policy fq_policy[TCA_FQ_MAX + 1] = {
diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
index b27d95418707..5982847df8f8 100644
--- a/net/sched/sch_fq_pie.c
+++ b/net/sched/sch_fq_pie.c
@@ -341,7 +341,8 @@ static int fq_pie_change(struct Qdisc *sch, struct nlattr *opt,
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]));
+ WRITE_ONCE(q->quantum,
+ max(256U, nla_get_u32(tb[TCA_FQ_PIE_QUANTUM])));
if (tb[TCA_FQ_PIE_MEMORY_LIMIT])
WRITE_ONCE(q->memory_limit,
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 187d3ed578f2..932a76bcf808 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -688,7 +688,7 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
/* update and validate configuration */
if (ctl->quantum)
- quantum = ctl->quantum;
+ quantum = max(256U, ctl->quantum);
if (ctl->flows)
maxflows = min_t(u32, ctl->flows, SFQ_MAX_FLOWS);
if (ctl->divisor) {
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq
2026-08-22 19:55 [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq Jamal Hadi Salim
` (5 preceding siblings ...)
2026-08-22 19:55 ` [PATCH net v3 6/6] net/sched: sfq: clamp quantum to avoid signed overflow soft lockup Jamal Hadi Salim
@ 2026-08-25 11:30 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 15+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-25 11:30 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, jiri, davem, edumazet, kuba, pabeni, horms, tahiliani,
sdp.sachin, vsaicharan1998, mohitbhasi1998, lesliemonis,
gautamramk, vtlam, stable, vega, victor
Hello:
This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Sat, 22 Aug 2026 15:55:03 -0400 you wrote:
> Several qdiscs derive their per-flow quantum or CoDel mtu from
> psched_mtu() without an overflow or zero clamp, which can drive the
> dequeue/credit-refill loop into a soft lockup or silently disable the
> AQM. vega@nebusec.ai provided reports and PoCs for the following qdiscs:
> sch_fq, sch_fq_codel, sch_fq_pie, sch_hhf, and sch_sfq.
>
> sch_codel was found by inspection for the same pattern. It's TheLinuxWay
> (i.e cutnpaste code from somewhere for your new feature) and the AIs
> are having a lot of fun finding patterns. We must overcome!
>
> [...]
Here is the summary with links:
- [net,v3,1/6] net/sched: fq: add overflow bounds to quantum and initial quantum
https://git.kernel.org/netdev/net/c/709f34f7c28d
- [net,v3,2/6] net/sched: fq_codel: clamp default quantum and mtu
https://git.kernel.org/netdev/net/c/d9ebd8f9aa8b
- [net,v3,3/6] net/sched: sch_codel: clamp default mtu to avoid disabling CoDel
https://git.kernel.org/netdev/net/c/6439461f1618
- [net,v3,4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow
https://git.kernel.org/netdev/net/c/c86cd7ed0b0e
- [net,v3,5/6] net/sched: hhf: clamp quantum before hhf_change() to avoid overflow
https://git.kernel.org/netdev/net/c/2164b512b97b
- [net,v3,6/6] net/sched: sfq: clamp quantum to avoid signed overflow soft lockup
https://git.kernel.org/netdev/net/c/816e90057ab1
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-08-25 11:31 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-22 19:55 [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum Jamal Hadi Salim
2026-08-25 10:03 ` Eric Dumazet
2026-08-25 10:43 ` Jamal Hadi Salim
2026-08-25 11:02 ` Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 2/6] net/sched: fq_codel: clamp default quantum and mtu Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 3/6] net/sched: sch_codel: clamp default mtu to avoid disabling CoDel Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow Jamal Hadi Salim
2026-08-25 8:33 ` Paolo Abeni
2026-08-25 9:16 ` Jamal Hadi Salim
2026-08-25 9:43 ` Paolo Abeni
2026-08-25 9:48 ` Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 5/6] net/sched: hhf: clamp quantum before hhf_change() to avoid overflow Jamal Hadi Salim
2026-08-22 19:55 ` [PATCH net v3 6/6] net/sched: sfq: clamp quantum to avoid signed overflow soft lockup Jamal Hadi Salim
2026-08-25 11:30 ` [PATCH net v3 0/6] net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox