Re: [PATCH net v3 1/9] net/sched: fq: clamp quantum and initial_quantum in change path

On Wed, Sep 2, 2026 at 1:52 AM Eric Dumazet [off-list ref] wrote:

On Tue, Sep 1, 2026 at 11:39 PM Jamal Hadi Salim [off-list ref] wrote:

quoted

The fq change path accepts TCA_FQ_QUANTUM in [1, INT_MAX] and
TCA_FQ_INITIAL_QUANTUM up to INT_MAX, while fq_init() already clamps to
[1, 1<<20]. A user can override the init clamp via tc qdisc change,
restoring the small-quantum deficit spin that the init clamp prevents.

Narrow iq_range.max to 1<<20 so TCA_FQ_INITIAL_QUANTUM is rejected at
parse time. Clamp TCA_FQ_QUANTUM to [256, 1<<20] in fq_change() and
fq_init() quantum to [256, 1<<20] for tiny-MTU devices.

Conditions to recreate the bug:
  CONFIG_NET_SCH_FQ=y. Requires CAP_NET_ADMIN (namespace-local via
  unshare -Urn suffices).

  tc qdisc add dev dummy0 root fq
  tc qdisc change dev dummy0 root fq quantum 1 stab data 32768 size_log 15 cell_log 0

Fixes: 709f34f7c28d ("net/sched: fq: add overflow bounds to quantum and initial quantum")
Reported-by: Vega <redacted>
Reviewed-by: Toke Høiland-Jørgensen <redacted>
Tested-by: Victor Nogueira <redacted>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_fq.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 6144b5686f13..35f940b2205d 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] = {

@@ -1106,14 +1106,10 @@ static int fq_change(struct Qdisc *sch, struct nlattr *opt,
                           nla_get_u32(tb[TCA_FQ_FLOW_PLIMIT]));

        if (tb[TCA_FQ_QUANTUM]) {
-               u32 quantum = nla_get_u32(tb[TCA_FQ_QUANTUM]);
+               u32 quantum = clamp_t(u32, nla_get_u32(tb[TCA_FQ_QUANTUM]),
+                                    256, 1 << 20);

-               if (quantum > 0 && quantum <= (1 << 20)) {
-                       WRITE_ONCE(q->quantum, quantum);
-               } else {
-                       NL_SET_ERR_MSG_MOD(extack, "invalid quantum");
-                       err = -EINVAL;
-               }
+               WRITE_ONCE(q->quantum, quantum);
        }

        if (tb[TCA_FQ_INITIAL_QUANTUM])

@@ -1232,7 +1228,7 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt,
        sch->limit              = 10000;
        q->flow_plimit          = 100;
        mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 1, 1 << 20);
-       q->quantum              = min_t(u32, 2 * mtu, 1 << 20);
+       q->quantum              = clamp_t(u32, 2 * mtu, 256, 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;
--

2.43.0

If we consider dev->mtu admissible values, which are in [0, INT_MAX],
we have to guard against u32 overflows
for 2*mtu and 10*mtu expressions.

Commit 709f34f7c28d ("net/sched: fq: add overflow bounds to quantum
and initial quantum") missed this?

Please use for q->quantum:  clamp_t(u64, 2ULL * mtu, 1ULL << 20)
and for q->initial_quantum: clamp_t(u64, 10ULL * mtu, 1ULL << 20)

Wait, I missed the
mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 1, 1 << 20);

Not clear why we have so many clamp_t(), this is quite confusing.

We could instead:

mtu = psched_mtu(qdisc_dev(sch));
q->quantum = clamp_t(u64, 2ULL * mtu, 256, 1ULL << 20);
q->initial_quantum = min_t(u64, 10ULL * mtu, 1ULL << 20);