diff options
| author | Paolo Abeni <pabeni@redhat.com> | 2026-08-25 13:09:34 +0200 |
|---|---|---|
| committer | Paolo Abeni <pabeni@redhat.com> | 2026-08-25 13:09:35 +0200 |
| commit | a687f2ae995fc366a6f2ef180451e6db4c6b4530 (patch) | |
| tree | 3a70ded8205fefedaa84d46087cb2c9da3c697f4 | |
| parent | 728836ebca239810f164262b10211ef59182f811 (diff) | |
| parent | 816e90057ab1879562a5b7cc688e35bb9027ae97 (diff) | |
| download | linux-a687f2ae995fc366a6f2ef180451e6db4c6b4530.tar.gz linux-a687f2ae995fc366a6f2ef180451e6db4c6b4530.zip | |
Merge branch 'net-sched-fix-quantum-mtu-overflow-in-fq-fq_codel-sch_codel-fq_pie-hhf-sfq'
Jamal Hadi Salim says:
====================
net: sched: fix quantum/mtu overflow in fq, fq_codel, sch_codel, fq_pie, hhf, sfq
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.
====================
Link: https://patch.msgid.link/20260822195509.112717-1-jhs@mojatatu.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
| -rw-r--r-- | net/sched/sch_codel.c | 2 | ||||
| -rw-r--r-- | net/sched/sch_fq.c | 6 | ||||
| -rw-r--r-- | net/sched/sch_fq_codel.c | 6 | ||||
| -rw-r--r-- | net/sched/sch_fq_pie.c | 3 | ||||
| -rw-r--r-- | net/sched/sch_hhf.c | 4 | ||||
| -rw-r--r-- | net/sched/sch_sfq.c | 3 |
6 files changed, 17 insertions, 7 deletions
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); diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c index 4b5f6d896c6d..6144b5686f13 100644 --- a/net/sched/sch_fq.c +++ b/net/sched/sch_fq.c @@ -1226,12 +1226,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; 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); 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; 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); 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)); |
