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 <[email protected]> wrote:
>
> On Tue, Aug 25, 2026 at 6:03 AM Eric Dumazet <[email protected]> wrote:
> >
> > On Sat, Aug 22, 2026 at 9:55 PM Jamal Hadi Salim <[email protected]> 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: [email protected]
> > > Tested-by: Victor Nogueira <[email protected]>
> > > Signed-off-by: Jamal Hadi Salim <[email protected]>
> > > ---
> > >  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