[PATCH net] net/sched: clamp skb->queue_mapping before indexing the tx queue
flat view
COOLING4d
REVIEWED: 1 (0M)
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-10-06 07:36:17
Also in:
bpf, stable
Subsystem:
networking [general], tc subsystem, the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim, Jiri Pirko, Linus Torvalds
1 review trailer.
This issue was caught by Sashiko (nipa).
A producer-side path cap is already merged (ea4d4b5dddb5); this
patch adds the matching consumer-side cap.
The tx queue is picked and capped in netdev_core_pick_tx(), before
__dev_xmit_skb() qdisc egress path. A tc BPF program running inside
egress qdisc (not clsact) can then change __sk_buff->queue_mapping;
that update is never re-checked. bpf_convert_ctx_access() bounds it
only by NO_QUEUE_MAPPING, not by the device's queue count.
On dequeue, qdisc_restart() we end up in netdev_get_tx_queue() which
indexes dev->_tx[] with that value. netdev_get_tx_queue() guards it with
just a DEBUG_NET_WARN_ON_ONCE, so sch_direct_xmit() takes the out-of-bounds
txq->_xmit_lock and writes txq->xmit_lock_owner past the end of the
allocation on non-lltx devices such as ifb. Drivers that index their rings
from skb_get_queue_mapping() (ex: ifb) go out of range the same way.
Cap the mapping at the consumer and write the capped value back, so both
the qdisc's _tx[] lookup and any later driver read stay in range. The cap
folds into the dequeue path instead of a separate walk over the returned
list: dequeue_skb() caps the head, and each bulk helper caps the element it
appends. netdev_cap_txqueue() already logs a ratelimited notice and selects
queue 0. The single-packet case pays one extra compare against
real_num_tx_queues.
To recreate the bug: load a SCHED_CLS or SCHED_ACT program that stores
__sk_buff->queue_mapping (a tc BPF action is enough), attach it to a
transmit qdisc (for example a matchall filter on a prio root), and send a
packet. With a 1-queue device a store of 1 indexes one past dev->_tx[].
Under KASAN with panic_on_warn=1 the unfixed kernel faults with
"BUG: KASAN: slab-out-of-bounds in sch_direct_xmit". Loading the program
needs CAP_BPF/CAP_SYS_ADMIN, so the trigger is root.
Fixes: 74e31ca850c1 ("bpf: add skb->queue_mapping write access from tc clsact")
Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/179033713973.2160803.4914570693994398206@kernel.org/ (local)
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-9R8V.v2.20260924072708%40mojatatu.com
Reviewed-by: Victor Nogueira <redacted>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_generic.c | 45 ++++++++++++++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 5 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 6f6a6f0d5eb0..e3e76e794aae 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c@@ -96,9 +96,33 @@ static void qdisc_maybe_clear_missed(struct Qdisc *q, #define SKB_XOFF_MAGIC ((struct sk_buff *)1UL) +/* A tc BPF program attached to a transmit qdisc runs inside q->enqueue(), + * after the tx queue has been picked and capped, so its store to + * __sk_buff->queue_mapping is not re-capped. Bound it against the device at + * the consumer and write the capped value back, so a driver that re-reads + * skb_get_queue_mapping() in ndo_start_xmit() also indexes in range. + */ +static u16 qdisc_cap_skb_tx_queue(struct net_device *dev, struct sk_buff *skb) +{ + u16 queue = skb_get_queue_mapping(skb); + u16 capped; + + capped = netdev_cap_txqueue(dev, queue); + if (unlikely(capped != queue)) + skb_set_queue_mapping(skb, capped); + + return capped; +} + +static struct netdev_queue *skb_cap_tx_queue(struct net_device *dev, + struct sk_buff *skb) +{ + return netdev_get_tx_queue(dev, qdisc_cap_skb_tx_queue(dev, skb)); +} + static inline struct sk_buff *__skb_dequeue_bad_txq(struct Qdisc *q) { - const struct netdev_queue *txq = q->dev_queue; + const struct netdev_queue *txq; spinlock_t *lock = NULL; struct sk_buff *skb;
@@ -110,7 +134,7 @@ static inline struct sk_buff *__skb_dequeue_bad_txq(struct Qdisc *q) skb = skb_peek(&q->skb_bad_txq); if (skb) { /* check the reason of requeuing without tx lock first */ - txq = skb_get_tx_queue(txq->dev, skb); + txq = skb_cap_tx_queue(qdisc_dev(q), skb); if (!netif_xmit_frozen_or_stopped(txq)) { skb = __skb_dequeue(&q->skb_bad_txq); if (qdisc_is_percpu_stats(q)) {
@@ -208,6 +232,7 @@ static void try_bulk_dequeue_skb(struct Qdisc *q, int *packets, int budget) { int bytelimit = qdisc_avail_bulklimit(txq) - skb->len; + struct net_device *dev = qdisc_dev(q); int cnt = 0; while (bytelimit > 0) {
@@ -216,6 +241,10 @@ static void try_bulk_dequeue_skb(struct Qdisc *q, if (!nskb) break; + /* A tc BPF store may have poisoned the mapping after pick; + * cap every element before it reaches the driver. + */ + qdisc_cap_skb_tx_queue(dev, nskb); bytelimit -= nskb->len; /* covers GSO len */ skb->next = nskb; skb = nskb;
@@ -233,7 +262,7 @@ static void try_bulk_dequeue_skb_slow(struct Qdisc *q, struct sk_buff *skb, int *packets) { - int mapping = skb_get_queue_mapping(skb); + struct net_device *dev = qdisc_dev(q); struct sk_buff *nskb; int cnt = 0;
@@ -241,7 +270,11 @@ static void try_bulk_dequeue_skb_slow(struct Qdisc *q, nskb = q->dequeue(q); if (!nskb) break; - if (unlikely(skb_get_queue_mapping(nskb) != mapping)) { + /* cap every element; the head was already capped, so a + * poisoned follower now compares equal to a poisoned head + */ + if (unlikely(qdisc_cap_skb_tx_queue(dev, nskb) != + skb_get_queue_mapping(skb))) { qdisc_enqueue_skb_bad_txq(q, nskb); break; }
@@ -286,7 +319,7 @@ static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate, if (xfrm_offload(skb)) *validate = true; /* check the reason of requeuing without tx lock first */ - txq = skb_get_tx_queue(txq->dev, skb); + txq = skb_cap_tx_queue(qdisc_dev(q), skb); if (!netif_xmit_frozen_or_stopped(txq)) { skb = __skb_dequeue(&q->gso_skb); if (qdisc_is_percpu_stats(q)) {
@@ -322,6 +355,8 @@ static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate, skb = q->dequeue(q); if (skb) { bulk: + /* cap the head; the bulk helpers cap every appended element */ + qdisc_cap_skb_tx_queue(qdisc_dev(q), skb); if (qdisc_may_bulk(q)) try_bulk_dequeue_skb(q, skb, txq, packets, budget); else --
2.43.0