From: Jamal Hadi Salim <jhs@mojatatu.com>
To: netdev@vger.kernel.org
Cc: Jamal Hadi Salim <jhs@mojatatu.com>,
Victor Nogueira <victor@mojatatu.com>,
Jiri Pirko <jiri@resnulli.us>,
"David S . Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@kernel.org>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Simon Horman <horms@kernel.org>,
Jesper Dangaard Brouer <hawk@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Sashiko <sashiko-bot@kernel.org>,
bpf@vger.kernel.org, stable@vger.kernel.org
Subject: [PATCH net] net/sched: clamp skb->queue_mapping before indexing the tx queue
Date: Tue, 6 Oct 2026 03:36:11 -0400 [thread overview]
Message-ID: <QDISC-MD1X.v1.20261005193231@mojatatu.com> (raw)
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/
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-9R8V.v2.20260924072708%40mojatatu.com
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
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
next reply other threads:[~2026-10-06 7:36 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-06 7:36 Jamal Hadi Salim [this message]
2026-10-08 17:43 ` [PATCH net] net/sched: clamp skb->queue_mapping before indexing the tx queue netdev-bot+sashiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=QDISC-MD1X.v1.20261005193231@mojatatu.com \
--to=jhs@mojatatu.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@kernel.org \
--cc=hawk@kernel.org \
--cc=horms@kernel.org \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sashiko-bot@kernel.org \
--cc=stable@vger.kernel.org \
--cc=victor@mojatatu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox