Re: [PATCH net v3 1/6] net/sched: fq: add overflow bounds to quantum and initial quantum

On Tue, Aug 25, 2026 at 6:43 AM Jamal Hadi Salim [off-list ref] wrote:

On Tue, Aug 25, 2026 at 6:03 AM Eric Dumazet [off-list ref] wrote:

quoted

On Sat, Aug 22, 2026 at 9:55 PM Jamal Hadi Salim [off-list ref] wrote:

quoted

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