[PATCH net-next v8 3/4] net/sched: cap the accounted backlog before it can wrap
flat view
WARM1d
REVIEWED: 2 (1M)
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-10-20 16:57:41
Subsystem:
cake qdisc, networking [general], tc subsystem, the rest · Maintainers:
Toke Høiland-Jørgensen, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim, Jiri Pirko, Linus Torvalds
Revision v8 of 3 in this series; 2 review trailers (1 from subsystem maintainers).
Revisions (3)
- v6 [diff vs current]
- v7 [diff vs current]
- v8 current
This is a followup of commit 709f34f7c28d ("net/sched: fq: add overflow
bounds to quantum and initial quantum") as reported by Sashiko.
The issue: fq_codel, cake, codel, pie, fq_pie, dualpi2, RED, pfifo,
plug and (non-WRED) gred account every enqueued packet's stab-adjusted
length into the 32-bit sch->qstats.backlog. A crafted tc size table
inflates qdisc_pkt_len() up to QDISC_PKT_LEN_MAX (1 MiB), and a dodgy
GSO skb can account even more through qdisc_pkt_len_segs_init(), so a
few thousand packets wrap the counter mod 2^32.
The AQM then reads a small backlog and makes the wrong drop decision,
and the dequeue-side subtractions leave it corrupt (for non-WRED gred
it is the reported stats and the dequeue-side subtractions that go
wrong instead). In fq_codel a wrapped per-flow backlog can also make
fq_codel_drop()'s scan pick an empty flow, dereferencing a NULL
flow->head.
The Fix: drop at enqueue once the stored backlog cannot represent the
incoming packet's accounted length.
Ordinary packets (qdisc_pkt_len() at most QDISC_PKT_LEN_MAX) are
covered by a standing ceiling: drop once the backlog is within
QDISC_MAX_BACKLOG (U32_MAX - QDISC_PKT_LEN_MAX) of the wrap point, the
largest backlog one more 1 MiB packet cannot wrap, so there is no
per-enqueue 64-bit add.
A larger wire estimate is checked against the space that actually
remains (len > U32_MAX - backlog): qdisc_pkt_len_segs_init() adds the
headers replicated on every output segment, so a supported small-MSS
BIG TCP aggregate can carry a wire estimate above 1 MiB while its raw
length stays within GSO_MAX_SIZE, and such traffic must pass on an
empty queue.
qdisc_pkt_len() itself stays truthful for every input: it is not
clamped and the initializer keeps succeeding, so BPF
__sk_buff.wire_len, act_police and the HTB/TBF shapers read the real
length, and sch_handle_ingress() -- which ignores the initializer's
return value and reads pkt_segs for packet statistics -- keeps
complete metadata.
Cover sch_plug and non-WRED sch_gred too: plug admits on skb->len
while qdisc_enqueue_tail() adds qdisc_pkt_len(), and gred bounds each
virtual queue but adds every VQ into the one aggregate. pfifo is
included because RED reads a grafted child's backlog into
red_calc_qavg(); it can be reached standalone or as the default child
of SFB. In RED, account the packet to the aggregate before handing it
to the child, because a duplicating netem child re-enqueues its clone
at the root while the stored backlog is still un-updated -- one guard
check must not authorize two additions.
The guard bounds each qdisc's own enqueue path: a child that segments a
GSO skb (tbf, netem) can still grow a parent's aggregate through
qdisc_tree_reduce_backlog()'s unclamped byte delta, which is left to a
separate change.
Conditions to recreate the bug:
ip tuntap add tun0 mode tun
ip link set tun0 txqueuelen 32 up
ip addr add 10.99.0.1/24 dev tun0
tc qdisc add dev tun0 root handle 1: stab overhead 2000000000 \
fq_codel flows 1 limit 20000 ecn
# hold the tun fd open without reading so the backlog persists, then
# send 6000 packets: the counter wraps and the unfixed kernel reports
# a wrapped backlog, while the fixed one drops at the ceiling.
Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/20260818101130.16203-1-jhs@mojatatu.com/ (local)
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260818101130.16203-1-jhs@mojatatu.com
Link: https://lore.kernel.org/netdev/CANn89iLfMJV7ancKH1Gjzzm7ZUG-gKcrczkJjJEWN5sCuWd-ug@mail.gmail.com/ (local)
Link: https://lore.kernel.org/netdev/CANn89i+GOFH_8g+vSV0jUj0aBqLYVWuFOrHuDggZu9kPC_m6tQ@mail.gmail.com/ (local)
Link: https://lore.kernel.org/netdev/87a4oottcj.fsf@toke.dk/ (local)
Link: https://lore.kernel.org/netdev/87pkxks68f.fsf@toke.dk/ (local)
Link: https://lore.kernel.org/netdev/179231932946.3135.7497478990186385688@kernel.org/ (local)
Reviewed-by: Victor Nogueira <redacted>
Reviewed-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v7 -> v8: fq_codel keeps its original classify shape (0001 no longer
refactors it); the guard is placed between idx-- and
codel_set_enqueue_time() so a dropped packet cannot touch flow state.
(Toke Høiland-Jørgensen, v7 review)
---
include/net/pkt_sched.h | 1 -
include/net/sch_generic.h | 20 ++++++++++++++++++++
net/sched/sch_cake.c | 7 +++++++
net/sched/sch_codel.c | 16 +++++++++-------
net/sched/sch_dualpi2.c | 1 +
net/sched/sch_fifo.c | 6 ++++++
net/sched/sch_fq_codel.c | 9 +++++++++
net/sched/sch_fq_pie.c | 3 ++-
net/sched/sch_gred.c | 9 +++++++++
net/sched/sch_pie.c | 3 ++-
net/sched/sch_plug.c | 9 ++++++++-
net/sched/sch_red.c | 23 ++++++++++++++++++-----
12 files changed, 91 insertions(+), 16 deletions(-)
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 90d3e7943b19..18a419cd9d94 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h@@ -12,7 +12,6 @@ #define DEFAULT_TX_QUEUE_LEN 1000 #define STAB_SIZE_LOG_MAX 30 -#define QDISC_PKT_LEN_MAX (1 << 20) /* 1 MiB */ struct qdisc_walker { int stop;
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 1acaadb3e2cf..a5a6bdf55e0a 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h@@ -915,6 +915,26 @@ static inline unsigned int qdisc_pkt_len(const struct sk_buff *skb) return qdisc_skb_cb(skb)->pkt_len; } +#define QDISC_PKT_LEN_MAX (1 << 20) /* 1 MiB */ + +/* Largest backlog that still has room for one more 1 MiB packet. */ +#define QDISC_MAX_BACKLOG (U32_MAX - QDISC_PKT_LEN_MAX) + +/* Would this packet's accounted length wrap sch->qstats.backlog? + * Packets up to QDISC_PKT_LEN_MAX fit below QDISC_MAX_BACKLOG; GSO + * can make the length larger, so compare against the real headroom. + */ +static inline bool qdisc_backlog_overflow(const struct Qdisc *sch, + const struct sk_buff *skb) +{ + u32 backlog = READ_ONCE(sch->qstats.backlog); + u32 len = qdisc_pkt_len(skb); + + if (likely(len <= QDISC_PKT_LEN_MAX)) + return backlog > QDISC_MAX_BACKLOG; + return len > U32_MAX - backlog; +} + static inline unsigned int qdisc_pkt_segs(const struct sk_buff *skb) { u32 pkt_segs = qdisc_skb_cb(skb)->pkt_segs;
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 0d13c4b0bfe3..91c33295b30b 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c@@ -1777,6 +1777,13 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, return ret; } + /* Drop before touching the hash or shaper state. */ + if (unlikely(qdisc_backlog_overflow(sch, skb))) { + qdisc_qstats_overlimit(sch); + return qdisc_drop_reason(skb, sch, to_free, + QDISC_DROP_OVERLIMIT); + } + /* choose flow to insert into */ b = cake_select_tin(sch, skb); idx = cake_hash(b, skb, q->config->flow_mode, flow_override,
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
index a1ba2e355f2d..78c5d72d6406 100644
--- a/net/sched/sch_codel.c
+++ b/net/sched/sch_codel.c@@ -116,15 +116,17 @@ static struct sk_buff *codel_peek(struct Qdisc *sch) static int codel_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) { - struct codel_sched_data *q; + struct codel_sched_data *q = qdisc_priv(sch); - if (likely(qdisc_qlen(sch) < sch->limit)) { - codel_set_enqueue_time(skb); - return qdisc_enqueue_tail(skb, sch); + if (unlikely(qdisc_qlen(sch) >= sch->limit) || + unlikely(qdisc_backlog_overflow(sch, skb))) { + WRITE_ONCE(q->drop_overlimit, q->drop_overlimit + 1); + return qdisc_drop_reason(skb, sch, to_free, + QDISC_DROP_OVERLIMIT); } - q = qdisc_priv(sch); - WRITE_ONCE(q->drop_overlimit, q->drop_overlimit + 1); - return qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_OVERLIMIT); + + codel_set_enqueue_time(skb); + return qdisc_enqueue_tail(skb, sch); } static const struct nla_policy codel_policy[TCA_CODEL_MAX + 1] = {
diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
index 4947def7c49e..7331f0ec1c8f 100644
--- a/net/sched/sch_dualpi2.c
+++ b/net/sched/sch_dualpi2.c@@ -392,6 +392,7 @@ static int dualpi2_enqueue_skb(struct sk_buff *skb, struct Qdisc *sch, struct dualpi2_skb_cb *cb; if (unlikely(qdisc_qlen(sch) >= sch->limit) || + unlikely(qdisc_backlog_overflow(sch, skb)) || unlikely((u64)q->memory_used + skb->truesize > q->memory_limit)) { qdisc_qstats_overlimit(sch); if (skb_in_l_queue(skb))
diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index 1b6388d50967..16c8d4f74e7e 100644
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c@@ -29,6 +29,9 @@ static int bfifo_enqueue(struct sk_buff *skb, struct Qdisc *sch, static int pfifo_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) { + if (unlikely(qdisc_backlog_overflow(sch, skb))) + return qdisc_drop(skb, sch, to_free); + if (likely(sch->q.qlen < READ_ONCE(sch->limit))) return qdisc_enqueue_tail(skb, sch);
@@ -43,6 +46,9 @@ static int pfifo_tail_enqueue(struct sk_buff *skb, struct Qdisc *sch, if (unlikely(READ_ONCE(sch->limit) == 0)) return qdisc_drop(skb, sch, to_free); + if (unlikely(qdisc_backlog_overflow(sch, skb))) + return qdisc_drop(skb, sch, to_free); + if (likely(sch->q.qlen < READ_ONCE(sch->limit))) return qdisc_enqueue_tail(skb, sch);
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index e6c87a32950f..a83f20534be1 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c@@ -201,6 +201,15 @@ static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch, } idx--; + /* Drop before the flow is touched, so a dropped packet cannot + * create or update flow state. + */ + if (unlikely(qdisc_backlog_overflow(sch, skb))) { + q->drop_overlimit++; + return qdisc_drop_reason(skb, sch, to_free, + QDISC_DROP_OVERLIMIT); + } + codel_set_enqueue_time(skb); flow = &q->flows[idx]; flow_queue_add(flow, skb);
diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
index 5982847df8f8..9f927a08b7bc 100644
--- a/net/sched/sch_fq_pie.c
+++ b/net/sched/sch_fq_pie.c@@ -155,7 +155,8 @@ static int fq_pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch, memory_limited = q->memory_usage > q->memory_limit + skb->truesize; /* Checks if the qdisc is full */ - if (unlikely(qdisc_qlen(sch) >= sch->limit)) { + if (unlikely(qdisc_qlen(sch) >= sch->limit) || + unlikely(qdisc_backlog_overflow(sch, skb))) { q->stats.overlimit++; goto out; } else if (unlikely(memory_limited)) {
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index f04f425c6c44..bedb945d3b67 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c@@ -170,6 +170,15 @@ static int gred_enqueue(struct sk_buff *skb, struct Qdisc *sch, unsigned long qavg = 0; u16 dp = tc_index_to_dp(skb); + /* Per-VQ limits don't bound the total: every VQ adds into the + * same u32 backlog, so check it before picking a VQ. + */ + if (unlikely(qdisc_backlog_overflow(sch, skb))) { + qdisc_qstats_overlimit(sch); + return qdisc_drop_reason(skb, sch, to_free, + QDISC_DROP_OVERLIMIT); + } + if (dp >= t->DPs || (q = t->tab[dp]) == NULL) { dp = t->def;
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 3b7863ffd284..2a5fc493a7e5 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c@@ -89,7 +89,8 @@ static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct pie_sched_data *q = qdisc_priv(sch); bool enqueue = false; - if (unlikely(qdisc_qlen(sch) >= sch->limit)) { + if (unlikely(qdisc_qlen(sch) >= sch->limit) || + unlikely(qdisc_backlog_overflow(sch, skb))) { WRITE_ONCE(q->stats.overlimit, q->stats.overlimit + 1); goto out; }
diff --git a/net/sched/sch_plug.c b/net/sched/sch_plug.c
index b60ddfee6a68..c90b5ca60bc0 100644
--- a/net/sched/sch_plug.c
+++ b/net/sched/sch_plug.c@@ -89,7 +89,14 @@ static int plug_enqueue(struct sk_buff *skb, struct Qdisc *sch, { struct plug_sched_data *q = qdisc_priv(sch); - if (likely((u64)sch->qstats.backlog + skb->len <= q->limit)) { + if (unlikely(qdisc_backlog_overflow(sch, skb))) { + qdisc_qstats_overlimit(sch); + return qdisc_drop_reason(skb, sch, to_free, + QDISC_DROP_OVERLIMIT); + } + + if (likely((u64)sch->qstats.backlog + qdisc_pkt_len(skb) <= + q->limit)) { if (!q->unplug_indefinite) q->pkts_current_epoch++; return qdisc_enqueue_tail(skb, sch);
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index d7598214270b..72990d0db4fd 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c@@ -76,6 +76,12 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch, unsigned int len; int ret; + if (unlikely(qdisc_backlog_overflow(sch, skb))) { + qdisc_qstats_overlimit(sch); + return qdisc_drop_reason(skb, sch, to_free, + QDISC_DROP_OVERLIMIT); + } + q->vars.qavg = red_calc_qavg(&q->parms, &q->vars, child->qstats.backlog);
@@ -136,14 +142,21 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch, } len = qdisc_pkt_len(skb); + /* Reserve the length before the child sees the packet: netem + * can re-enqueue a clone here before the backlog is updated. + * Undo it if the child drops the packet. + */ + qstats_backlog_add(sch, len); ret = qdisc_enqueue(skb, child, to_free); if (likely(ret == NET_XMIT_SUCCESS)) { - qstats_backlog_add(sch, len); qdisc_qlen_inc(sch); - } else if (net_xmit_drop_count(ret)) { - WRITE_ONCE(q->stats.pdrop, - q->stats.pdrop + 1); - qdisc_qstats_drop(sch); + } else { + qstats_backlog_sub(sch, len); + if (net_xmit_drop_count(ret)) { + WRITE_ONCE(q->stats.pdrop, + q->stats.pdrop + 1); + qdisc_qstats_drop(sch); + } } return ret;
--
2.43.0