Re: [PATCH net] net/sched: clamp quantum and psched_mtu in change paths and missed siblings

Jamal Hadi Salim [off-list ref] writes:

This is a followup to commit 709f34f7c28d ("net/sched: fq: add
overflow bounds to quantum and initial quantum").
The quantum_backlog_overflow series and the five siblings that followed
clamped the init-path quantum to in fq, fq_codel, fq_pie, hhf, sfq.
The change() paths with the same pattern, same writer of q->quantum,
same privilege level (CAP_NET_ADMIN in a user namespace) were not clamped.
A user can override the init clamp via tc qdisc change, restoring the
small-quantum deficit spin that the init clamp was meant to prevent.

This follow-up also covers two siblings that were missed entirely by
the original series: sch_dualpi2 and sch_pie call psched_mtu() without
any clamp at all. With a crafted size table qdisc_pkt_len reaches ~2
GiB, so quantum=1 (or a zero psched_mtu on a headerless device) makes
the deficit-refill loop spin ~2^31 times under the qdisc lock (a soft
lockup / denial of service).

Fixes based on review of 709f34f7c28d:

1. fq_pie_change() accepts quantum=1 (NLA policy fq_pie_q_range.min=1).
   Add max(256U, ...) matching fq_codel_change()
   (Sashiko nipa gpt-5-6-sol-3-8 and gpt-5-6-sol-6-7)

2. sfq_change() accepts any non-negative quantum (only rejects
   (int)ctl->quantum < 0). Add max(256U, ...) matching fq_codel_change().
   Reject quantum > 1<<20 with -EINVAL, matching fq_codel_change() and
   the init clamp.
   (Internal review noticing same pattern)

3. hhf_change() accepts quantum=1 (only checks non_hh_quantum product).
   Add max(256U, ...) matching fq_codel_change()
   (Sashiko nipa v1 review + vega@nebusec.ai independent bug)

4. fq_change() accepts TCA_FQ_INITIAL_QUANTUM up to INT_MAX (iq_range.max
   = INT_MAX) while fq_init() now clamps to 1<<20.  Narrow iq_range.max
   to 1<<20, rejecting at parse time.  (Eric Dumazet)

5. sch_dualpi2: dualpi2_calculate_c_protection() and get_memory_limit()
   call psched_mtu() with no clamp. A huge MTU makes (s32)psched_mtu()
   overflow in the signed multiply for c_protection_init, and 2 *
   psched_mtu() wraps in get_memory_limit(). Clamp to [1, 1<<20] at
   all three call sites. (Sashiko nipa main-6-4)

6. sch_pie: pie_drop_early() calls psched_mtu() with no clamp.  With
   mtu=0x80000000 the bytemode divide silently zeroes the drop
   probability, disabling AQM. Clamp to [1, 1<<20] (Sashiko gemini)

7. sch_drr: drr_change_class() rejects explicit quantum==0 but falls
   back to psched_mtu() with no floor. Add max(256U, ...) after the
   zero reject and on the fallback path
  (vega@nebusec.ai independent bug)

8. sch_ets: ets_qdisc_change() falls back to psched_mtu() with no floor
   for bands without an explicit quantum.  Add max(256U, ...) on the
   fallback path (vega@nebusec.ai independent bug)

The init paths of fq_pie, sfq, and hhf delegate to their _change() when
opt is present, so the floor covers tc qdisc add ... quantum 1 as well
as change. The zero-quantum-from-psched_mtu case on a headerless device
(mtu==0) is also covered by the 256 floor in drr and ets; the
explicit-zero reject in drr_change_class() is preserved.

The sfq_change() silent clamp is user-visible: sfq_dump() reports the
clamped quantum, so a previously accepted quantum < 256 now reads back
as 256. Idempotent config managers that read back and compare will
see drift. fq_codel_change() made the same trade, so this is consistent.

Conditions to recreate the bug: create a fq_pie, sfq, hhf, dualpi2, or
pie qdisc (or a drr class / ets band), then tc qdisc change ... quantum 1
with a STAB size table inflating qdisc_pkt_len, e.g.:

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

Requires CAP_NET_ADMIN in a user namespace (unshare -Urn).

Fixes: 709f34f7c28d ("net/sched: fq: add overflow bounds to quantum and initial quantum")
Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
Fixes: e4650d7ae425 ("net_sched: sch_sfq: handle bigger packets")
Fixes: 10239edf86f1 ("net-qdisc-hhf: Heavy-Hitter Filter (HHF) qdisc")
Fixes: 320d031ad6e4 ("sched: Struct definition and parsing of dualpi2 qdisc")
Fixes: d4b36210c2e6 ("net: pkt_sched: PIE AQM scheme")
Fixes: 13d2a1d2b032 ("pkt_sched: add DRR scheduler")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <redacted>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: stable@vger.kernel.org

Reviewed-by: Toke Høiland-Jørgensen <redacted>