[PATCH net v3] net: cap skb->queue_mapping when the tx queue is picked

[PATCH net v3] net: cap skb->queue_mapping when the tx queue is picked

[Jamal Hadi Salim]

skbedit can set skb->queue_mapping and raise the per-CPU skip_txqueue
flag so __dev_queue_xmit() honours the mapping. __dev_queue_xmit()
cleared the flag before sch_handle_egress() and only read it afterwards,
so the flag was not confined to the xmit that set it: a nested xmit
(mirred redirect or mirror, or a drop after skbedit) could set the flag
and the outer xmit would consume it for an skb that never went through
skbedit.

A forwarded packet still carries the ingress NIC's rx_queue + 1 in
skb->queue_mapping, so the outer device then indexes its tx queue state
with that stale value. Taprio's child array q->qdiscs[] is sized to the
device's queue count, so taprio_enqueue() indexes past its allocation
and dereferences the result as a struct Qdisc *.

Own the flag for the whole xmit frame: save the incoming value and clear
it before any of the frame's egress work can recurse, and restore it only
when the frame exits. A transmit-qdisc classifier is a documented flag
producer too (it runs in q->enqueue(), after sch_handle_egress()), so the
flag must be owned for the whole frame, not just around the clsact hook.
The flag then never crosses an xmit boundary in either direction. Also
store the value netdev_cap_txqueue() selected back into skb->queue_mapping
in netdev_tx_queue_mapping(), as netdev_core_pick_tx() already does, so
the skip_txqueue path never hands a later reader on the xmit path a
mapping the device cannot serve. A store made still later in the same
frame, by a tc BPF program attached to a transmit qdisc, is outside this
path and is not re-capped; a separate followup will resolve that path.

netdev_xmit_skip_txqueue() now returns the previous flag value so the
save-and-clear is one call, and a no-op stub is provided when
CONFIG_NET_EGRESS is disabled. This removes the inline #ifdef around the
save and the two restores.

A local user in a network namespace can redirect a packet from a device
with more TX queues to one with fewer after setting a mapping valid only
on the larger device. That reaches these reads and, under KASAN, faults
with "slab-out-of-bounds in taprio_enqueue".

Conditions to recreate the bug: with CONFIG_NET_SCH_TAPRIO=y,
CONFIG_NET_ACT_SKBEDIT=y, CONFIG_NET_ACT_MIRRED=y,
CONFIG_NET_CLS_MATCHALL=y, CONFIG_NET_SCH_PRIO=y and KASAN enabled,
create qa (3 queues), qb (2 queues) and qc (1 queue) as dummy devices;
put a taprio root on qb and clsact on all three; then add an egress
matchall filter on every device. On qa: "action skbedit queue_mapping 2
pipe action mirred egress redirect dev qb". On qb: "action mirred egress
mirror dev qc". On qc: "action skbedit queue_mapping 0 pipe". Send one
packet out qa. qc's skbedit sets the flag while qb's outer xmit is in
flight; without the fix qb consumes it and reads its two-entry taprio
child array with the forwarded packet's stale mapping. A qc whose skbedit
is instead installed in a transmit-qdisc classifier (a matchall filter on
the qc root qdisc) reaches the same code path the same way.

Testing: on a KASAN build with panic_on_warn=1 the unfixed kernel panics
with "BUG: KASAN: slab-out-of-bounds in taprio_enqueue", a read 0 bytes
past a 16-byte taprio_init() allocation; the fixed kernel runs the
clsact-setter and the transmit-qdisc-classifier reproducers with no report
and no clamp notice, and a clsact skbedit-then-tc-BPF store with no report
but the expected "selects TX queue" clamp notice from the write-back;

Fixes: 2f1e85b1aee4 ("net: sched: use queue_mapping to pick tx queue")
Reported-by: Zero Day Initiative <zdi-disclosures@trendmicro.com>
Link: https://lore.kernel.org/netdev/CANn89iLwYx8nCVf0pCEk_MmEiyC6kQaMwCQT9WkQVeeNzNQHqQ@mail.gmail.com/
Link: https://lore.kernel.org/netdev/179008581937.2160803.7117814290574262942@kernel.org/
Link: https://lore.kernel.org/netdev/179033713973.2160803.4914570693994398206@kernel.org/
Link: https://lore.kernel.org/netdev/20260925180407.63647514@kernel.org/
Suggested-by: Eric Dumazet <edumazet@google.com>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Tested-by: hybris <hybris@mojatatu.ai>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v2 -> v3:
- netdev_xmit_skip_txqueue() returns the previous flag value, so the entry
  save-and-clear is one call; a no-op stub is provided when
  !CONFIG_NET_EGRESS, removing the three inline #ifdefs (Jakub Kicinski).
- Narrow the capability claim: the capped write-back covers the
  skip_txqueue path only; a store made later in the frame by a tc BPF
  program on a transmit qdisc is not re-capped and is a separate followup
  (nipa Sashiko).
- Comment: "per-CPU (per-task on PREEMPT_RT)" (nipa Sashiko).
- Drop the now-redundant #ifdef in act_skbedit.c.

v1 -> v2:
- Rework the root cause to the per-CPU skip_txqueue flag lifetime: it was
  cleared before sch_handle_egress() and read after, so a nested xmit could
  set it and the outer xmit consume it for an skb that never went through
  skbedit (Eric Dumazet, nipa Sashiko).
- Own the flag for the whole xmit frame: save/clear it before any of the
  frame's egress work can recurse, and restore it only when the frame exits.
- Retain the v1 producer-side cap (netdev_tx_queue_mapping() writes the
  clamped value back).
- Correct the description of the reproducer to a three-device chain
  (qa 3q -> qb 2q taprio -> qc 1q).

 include/linux/rtnetlink.h |  7 ++++++-
 net/core/dev.c            | 33 +++++++++++++++++++++++++++------
 net/sched/act_skbedit.c   |  2 --
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 95729339e7a5..a54ec40d095c 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -186,7 +186,12 @@ void net_dec_ingress_queue(void);
 #ifdef CONFIG_NET_EGRESS
 void net_inc_egress_queue(void);
 void net_dec_egress_queue(void);
-void netdev_xmit_skip_txqueue(bool skip);
+bool netdev_xmit_skip_txqueue(bool skip);
+#else
+static inline bool netdev_xmit_skip_txqueue(bool skip)
+{
+	return false;
+}
 #endif
 
 void rtnetlink_init(void);
diff --git a/net/core/dev.c b/net/core/dev.c
index f660fccfc0db..3b114fbf7015 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4404,9 +4404,14 @@ EXPORT_SYMBOL(dev_loopback_xmit);
 static struct netdev_queue *
 netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
 {
-	int qm = skb_get_queue_mapping(skb);
+	int queue = skb_get_queue_mapping(skb);
+	int capped;
 
-	return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
+	capped = netdev_cap_txqueue(dev, queue);
+	if (unlikely(capped != queue))
+		skb_set_queue_mapping(skb, capped);
+
+	return netdev_get_tx_queue(dev, capped);
 }
 
 #ifndef CONFIG_PREEMPT_RT
@@ -4415,9 +4420,13 @@ static bool netdev_xmit_txqueue_skipped(void)
 	return __this_cpu_read(softnet_data.xmit.skip_txqueue);
 }
 
-void netdev_xmit_skip_txqueue(bool skip)
+bool netdev_xmit_skip_txqueue(bool skip)
 {
+	bool prev = netdev_xmit_txqueue_skipped();
+
 	__this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
+
+	return prev;
 }
 EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
 
@@ -4427,9 +4436,13 @@ static bool netdev_xmit_txqueue_skipped(void)
 	return current->net_xmit.skip_txqueue;
 }
 
-void netdev_xmit_skip_txqueue(bool skip)
+bool netdev_xmit_skip_txqueue(bool skip)
 {
+	bool prev = netdev_xmit_txqueue_skipped();
+
 	current->net_xmit.skip_txqueue = skip;
+
+	return prev;
 }
 EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
 #endif
@@ -4824,6 +4837,7 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
 	int cpu, rc = -ENOMEM;
 	bool again = false;
 	struct Qdisc *q;
+	bool skip_txq;
 
 	skb_reset_mac_header(skb);
 	skb_assert_len(skb);
@@ -4846,6 +4860,13 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
 	skb_update_prio(skb);
 
 	tcx_set_ingress(skb, false);
+	/* The flag is per-CPU (per-task on PREEMPT_RT) and a nested xmit can
+	 * set it from its own clsact hook or transmit qdisc. Own it for the
+	 * whole frame: this frame cannot consume a nested xmit's flag and a
+	 * nested xmit cannot inherit this frame's.
+	 */
+	skip_txq = netdev_xmit_skip_txqueue(false);
+
 #ifdef CONFIG_NET_EGRESS
 	if (static_branch_unlikely(&egress_needed_key)) {
 		if (nf_hook_egress_active()) {
@@ -4854,8 +4875,6 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
 				goto out;
 		}
 
-		netdev_xmit_skip_txqueue(false);
-
 		nf_skip_egress(skb, true);
 		skb = sch_handle_egress(skb, &rc, dev);
 		if (!skb)
@@ -4952,12 +4971,14 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
 
 	reason = SKB_DROP_REASON_RECURSION_LIMIT;
 drop:
+	netdev_xmit_skip_txqueue(skip_txq);
 	rcu_read_unlock_bh();
 
 	dev_core_stats_tx_dropped_inc(dev);
 	kfree_skb_list_reason(skb, reason);
 	return rc;
 out:
+	netdev_xmit_skip_txqueue(skip_txq);
 	rcu_read_unlock_bh();
 	return rc;
 }
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index bfec6b668410..9c1ade20c004 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -72,9 +72,7 @@ TC_INDIRECT_SCOPE int tcf_skbedit_act(struct sk_buff *skb,
 	}
 	if (params->flags & SKBEDIT_F_QUEUE_MAPPING &&
 	    skb->dev->real_num_tx_queues > params->queue_mapping) {
-#ifdef CONFIG_NET_EGRESS
 		netdev_xmit_skip_txqueue(true);
-#endif
 		skb_set_queue_mapping(skb, tcf_skbedit_hash(params, skb));
 	}
 	if (params->flags & SKBEDIT_F_MARK) {
-- 
2.43.0

Re: [PATCH net v3] net: cap skb->queue_mapping when the tx queue is picked

[Eric Dumazet]

On Sat, Sep 26, 2026 at 11:48 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> skbedit can set skb->queue_mapping and raise the per-CPU skip_txqueue
> flag so __dev_queue_xmit() honours the mapping. __dev_queue_xmit()
> cleared the flag before sch_handle_egress() and only read it afterwards,
> so the flag was not confined to the xmit that set it: a nested xmit
> (mirred redirect or mirror, or a drop after skbedit) could set the flag
> and the outer xmit would consume it for an skb that never went through
> skbedit.
>
> A forwarded packet still carries the ingress NIC's rx_queue + 1 in
> skb->queue_mapping, so the outer device then indexes its tx queue state
> with that stale value. Taprio's child array q->qdiscs[] is sized to the
> device's queue count, so taprio_enqueue() indexes past its allocation
> and dereferences the result as a struct Qdisc *.
>
> Own the flag for the whole xmit frame: save the incoming value and clear
> it before any of the frame's egress work can recurse, and restore it only
> when the frame exits. A transmit-qdisc classifier is a documented flag
> producer too (it runs in q->enqueue(), after sch_handle_egress()), so the
> flag must be owned for the whole frame, not just around the clsact hook.
> The flag then never crosses an xmit boundary in either direction. Also
> store the value netdev_cap_txqueue() selected back into skb->queue_mapping
> in netdev_tx_queue_mapping(), as netdev_core_pick_tx() already does, so
> the skip_txqueue path never hands a later reader on the xmit path a
> mapping the device cannot serve. A store made still later in the same
> frame, by a tc BPF program attached to a transmit qdisc, is outside this
> path and is not re-capped; a separate followup will resolve that path.
>
> netdev_xmit_skip_txqueue() now returns the previous flag value so the
> save-and-clear is one call, and a no-op stub is provided when
> CONFIG_NET_EGRESS is disabled. This removes the inline #ifdef around the
> save and the two restores.
>
> A local user in a network namespace can redirect a packet from a device
> with more TX queues to one with fewer after setting a mapping valid only
> on the larger device. That reaches these reads and, under KASAN, faults
> with "slab-out-of-bounds in taprio_enqueue".
>
> Conditions to recreate the bug: with CONFIG_NET_SCH_TAPRIO=y,
> CONFIG_NET_ACT_SKBEDIT=y, CONFIG_NET_ACT_MIRRED=y,
> CONFIG_NET_CLS_MATCHALL=y, CONFIG_NET_SCH_PRIO=y and KASAN enabled,
> create qa (3 queues), qb (2 queues) and qc (1 queue) as dummy devices;
> put a taprio root on qb and clsact on all three; then add an egress
> matchall filter on every device. On qa: "action skbedit queue_mapping 2
> pipe action mirred egress redirect dev qb". On qb: "action mirred egress
> mirror dev qc". On qc: "action skbedit queue_mapping 0 pipe". Send one
> packet out qa. qc's skbedit sets the flag while qb's outer xmit is in
> flight; without the fix qb consumes it and reads its two-entry taprio
> child array with the forwarded packet's stale mapping. A qc whose skbedit
> is instead installed in a transmit-qdisc classifier (a matchall filter on
> the qc root qdisc) reaches the same code path the same way.
>
> Testing: on a KASAN build with panic_on_warn=1 the unfixed kernel panics
> with "BUG: KASAN: slab-out-of-bounds in taprio_enqueue", a read 0 bytes
> past a 16-byte taprio_init() allocation; the fixed kernel runs the
> clsact-setter and the transmit-qdisc-classifier reproducers with no report
> and no clamp notice, and a clsact skbedit-then-tc-BPF store with no report
> but the expected "selects TX queue" clamp notice from the write-back;
>
> Fixes: 2f1e85b1aee4 ("net: sched: use queue_mapping to pick tx queue")
> Reported-by: Zero Day Initiative <zdi-disclosures@trendmicro.com>
> Link: https://lore.kernel.org/netdev/CANn89iLwYx8nCVf0pCEk_MmEiyC6kQaMwCQT9WkQVeeNzNQHqQ@mail.gmail.com/
> Link: https://lore.kernel.org/netdev/179008581937.2160803.7117814290574262942@kernel.org/
> Link: https://lore.kernel.org/netdev/179033713973.2160803.4914570693994398206@kernel.org/
> Link: https://lore.kernel.org/netdev/20260925180407.63647514@kernel.org/
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Tested-by: hybris <hybris@mojatatu.ai>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> v2 -> v3:
> - netdev_xmit_skip_txqueue() returns the previous flag value, so the entry
>   save-and-clear is one call; a no-op stub is provided when
>   !CONFIG_NET_EGRESS, removing the three inline #ifdefs (Jakub Kicinski).
> - Narrow the capability claim: the capped write-back covers the
>   skip_txqueue path only; a store made later in the frame by a tc BPF
>   program on a transmit qdisc is not re-capped and is a separate followup
>   (nipa Sashiko).
> - Comment: "per-CPU (per-task on PREEMPT_RT)" (nipa Sashiko).
> - Drop the now-redundant #ifdef in act_skbedit.c.
>
> v1 -> v2:
> - Rework the root cause to the per-CPU skip_txqueue flag lifetime: it was
>   cleared before sch_handle_egress() and read after, so a nested xmit could
>   set it and the outer xmit consume it for an skb that never went through
>   skbedit (Eric Dumazet, nipa Sashiko).
> - Own the flag for the whole xmit frame: save/clear it before any of the
>   frame's egress work can recurse, and restore it only when the frame exits.
> - Retain the v1 producer-side cap (netdev_tx_queue_mapping() writes the
>   clamped value back).
> - Correct the description of the reproducer to a three-device chain
>   (qa 3q -> qb 2q taprio -> qc 1q).
>
>  include/linux/rtnetlink.h |  7 ++++++-
>  net/core/dev.c            | 33 +++++++++++++++++++++++++++------
>  net/sched/act_skbedit.c   |  2 --
>  3 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index 95729339e7a5..a54ec40d095c 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -186,7 +186,12 @@ void net_dec_ingress_queue(void);
>  #ifdef CONFIG_NET_EGRESS
>  void net_inc_egress_queue(void);
>  void net_dec_egress_queue(void);
> -void netdev_xmit_skip_txqueue(bool skip);
> +bool netdev_xmit_skip_txqueue(bool skip);
> +#else
> +static inline bool netdev_xmit_skip_txqueue(bool skip)
> +{
> +       return false;
> +}
>  #endif
>
>  void rtnetlink_init(void);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f660fccfc0db..3b114fbf7015 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4404,9 +4404,14 @@ EXPORT_SYMBOL(dev_loopback_xmit);
>  static struct netdev_queue *
>  netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
>  {
> -       int qm = skb_get_queue_mapping(skb);
> +       int queue = skb_get_queue_mapping(skb);
> +       int capped;
>
> -       return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
> +       capped = netdev_cap_txqueue(dev, queue);
> +       if (unlikely(capped != queue))
> +               skb_set_queue_mapping(skb, capped);
> +
> +       return netdev_get_tx_queue(dev, capped);
>  }
>
>  #ifndef CONFIG_PREEMPT_RT
> @@ -4415,9 +4420,13 @@ static bool netdev_xmit_txqueue_skipped(void)
>         return __this_cpu_read(softnet_data.xmit.skip_txqueue);
>  }
>
> -void netdev_xmit_skip_txqueue(bool skip)
> +bool netdev_xmit_skip_txqueue(bool skip)
>  {
> +       bool prev = netdev_xmit_txqueue_skipped();
> +
>         __this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
> +
> +       return prev;
>  }
>  EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
>
> @@ -4427,9 +4436,13 @@ static bool netdev_xmit_txqueue_skipped(void)
>         return current->net_xmit.skip_txqueue;
>  }
>
> -void netdev_xmit_skip_txqueue(bool skip)
> +bool netdev_xmit_skip_txqueue(bool skip)
>  {
> +       bool prev = netdev_xmit_txqueue_skipped();
> +
>         current->net_xmit.skip_txqueue = skip;
> +
> +       return prev;
>  }
>  EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
>  #endif
> @@ -4824,6 +4837,7 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
>         int cpu, rc = -ENOMEM;
>         bool again = false;
>         struct Qdisc *q;
> +       bool skip_txq;
>
>         skb_reset_mac_header(skb);
>         skb_assert_len(skb);
> @@ -4846,6 +4860,13 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
>         skb_update_prio(skb);
>
>         tcx_set_ingress(skb, false);
> +       /* The flag is per-CPU (per-task on PREEMPT_RT) and a nested xmit can
> +        * set it from its own clsact hook or transmit qdisc. Own it for the
> +        * whole frame: this frame cannot consume a nested xmit's flag and a
> +        * nested xmit cannot inherit this frame's.
> +        */
> +       skip_txq = netdev_xmit_skip_txqueue(false);

Hi Jamal

Moving netdev_xmit_skip_txqueue() outside of
static_branch_unlikely(&egress_needed_key) means every packet in
__dev_queue_xmit() now pays for a per-CPU (or current->) read and two
writes when CONFIG_NET_EGRESS=y, even when no egress hook is active.

Commit 2f1e85b1aee4 specifically used egress_needed_key to avoid this
fast-path cost.

Also, in __dev_queue_xmit(), txq is picked before q->enqueue() runs, so
tcf_skbedit_act() setting skip_txqueue from q->enqueue() (or from
ingress) has no effect on the current packet and only pollutes the
per-CPU flag.

I noticed that nf_skip_egress(skb, true/false) already brackets
sch_handle_egress(). If we make skb->nf_skip_egress available under
CONFIG_NET_EGRESS (instead of CONFIG_NETFILTER_SKIP_EGRESS),
tcf_skbedit_act() can only arm skip_txqueue when running in
sch_handle_egress().

Given this idea my LLM produced the following (untested) patch:

diff --git a/include/linux/netfilter_netdev.h b/include/linux/netfilter_netdev.h
index 3175073a66ba6ba61db0f6d9d62c73ca15206ef0..2a854a0bbd4ba7b5c611b7aaf90b07711e57d9e0
100644
--- a/include/linux/netfilter_netdev.h
+++ b/include/linux/netfilter_netdev.h
@@ -133,7 +133,7 @@ static inline struct sk_buff
*nf_hook_egress(struct sk_buff *skb, int *rc,

 static inline void nf_skip_egress(struct sk_buff *skb, bool skip)
 {
-#ifdef CONFIG_NETFILTER_SKIP_EGRESS
+#ifdef CONFIG_NET_EGRESS
        skb->nf_skip_egress = skip;
 #endif
 }
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 95729339e7a54819b08e124bd36126cf71dfd9a4..a54ec40d095c041191ca44f7c4334aa37fa6b891
100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -186,7 +186,12 @@ void net_dec_ingress_queue(void);
 #ifdef CONFIG_NET_EGRESS
 void net_inc_egress_queue(void);
 void net_dec_egress_queue(void);
-void netdev_xmit_skip_txqueue(bool skip);
+bool netdev_xmit_skip_txqueue(bool skip);
+#else
+static inline bool netdev_xmit_skip_txqueue(bool skip)
+{
+       return false;
+}
 #endif

 void rtnetlink_init(void);
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 84308498a3a8cd5a86364e0b15ddb35755732635..a3ff380d43f01db0b040573c9dd7acc4e2e2cda8
100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1020,7 +1020,7 @@ struct sk_buff {
 #ifdef CONFIG_NET_REDIRECT
        __u8                    from_ingress:1;
 #endif
-#ifdef CONFIG_NETFILTER_SKIP_EGRESS
+#ifdef CONFIG_NET_EGRESS
        __u8                    nf_skip_egress:1;
 #endif
 #ifdef CONFIG_SKB_DECRYPTED
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index f35bd06a6bad680586da6b8ef20efc5d6b68fd6d..1acaadb3e2cf08827bb0d94e1649cfeb9e9c85ac
100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -810,6 +810,15 @@ static inline bool skb_at_tc_ingress(const struct
sk_buff *skb)
 #endif
 }

+static inline bool skb_at_tc_egress(const struct sk_buff *skb)
+{
+#ifdef CONFIG_NET_EGRESS
+       return skb->nf_skip_egress && !skb_at_tc_ingress(skb);
+#else
+       return false;
+#endif
+}
+
 static inline bool skb_skip_tc_classify(struct sk_buff *skb)
 {
 #ifdef CONFIG_NET_CLS_ACT
diff --git a/net/core/dev.c b/net/core/dev.c
index f660fccfc0dbc56d7e1a9643525229b278bca547..8638c994bbcb0d1bf25ed603f833f105236a45bf
100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4404,9 +4404,14 @@ EXPORT_SYMBOL(dev_loopback_xmit);
 static struct netdev_queue *
 netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
 {
-       int qm = skb_get_queue_mapping(skb);
+       int queue = skb_get_queue_mapping(skb);
+       int capped;

-       return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
+       capped = netdev_cap_txqueue(dev, queue);
+       if (unlikely(capped != queue))
+               skb_set_queue_mapping(skb, capped);
+
+       return netdev_get_tx_queue(dev, capped);
 }

 #ifndef CONFIG_PREEMPT_RT
@@ -4415,9 +4420,13 @@ static bool netdev_xmit_txqueue_skipped(void)
        return __this_cpu_read(softnet_data.xmit.skip_txqueue);
 }

-void netdev_xmit_skip_txqueue(bool skip)
+bool netdev_xmit_skip_txqueue(bool skip)
 {
+       bool prev = netdev_xmit_txqueue_skipped();
+
        __this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
+
+       return prev;
 }
 EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);

@@ -4427,9 +4436,13 @@ static bool netdev_xmit_txqueue_skipped(void)
        return current->net_xmit.skip_txqueue;
 }

-void netdev_xmit_skip_txqueue(bool skip)
+bool netdev_xmit_skip_txqueue(bool skip)
 {
+       bool prev = netdev_xmit_txqueue_skipped();
+
        current->net_xmit.skip_txqueue = skip;
+
+       return prev;
 }
 EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
 #endif
@@ -4848,21 +4861,25 @@ int __dev_queue_xmit(struct sk_buff *skb,
struct net_device *sb_dev)
        tcx_set_ingress(skb, false);
 #ifdef CONFIG_NET_EGRESS
        if (static_branch_unlikely(&egress_needed_key)) {
+               bool skip_txq;
+
                if (nf_hook_egress_active()) {
                        skb = nf_hook_egress(skb, &rc, dev);
                        if (!skb)
                                goto out;
                }

-               netdev_xmit_skip_txqueue(false);
+               skip_txq = netdev_xmit_skip_txqueue(false);

                nf_skip_egress(skb, true);
                skb = sch_handle_egress(skb, &rc, dev);
-               if (!skb)
+               if (!skb) {
+                       netdev_xmit_skip_txqueue(skip_txq);
                        goto out;
+               }
                nf_skip_egress(skb, false);

-               if (netdev_xmit_txqueue_skipped())
+               if (netdev_xmit_skip_txqueue(skip_txq))
                        txq = netdev_tx_queue_mapping(dev, skb);
        }
 #endif
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index bfec6b66841031cd566d0c2bdc3d120cec41e3e4..26258b353cdeec48063413770439878d27ed60cf
100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -72,9 +72,8 @@ TC_INDIRECT_SCOPE int tcf_skbedit_act(struct sk_buff *skb,
        }
        if (params->flags & SKBEDIT_F_QUEUE_MAPPING &&
            skb->dev->real_num_tx_queues > params->queue_mapping) {
-#ifdef CONFIG_NET_EGRESS
-               netdev_xmit_skip_txqueue(true);
-#endif
+               if (skb_at_tc_egress(skb))
+                       netdev_xmit_skip_txqueue(true);
                skb_set_queue_mapping(skb, tcf_skbedit_hash(params, skb));
        }
        if (params->flags & SKBEDIT_F_MARK) {

Re: [PATCH net v3] net: cap skb->queue_mapping when the tx queue is picked

[Jamal Hadi Salim]

On Sun, Sep 27, 2026 at 11:48 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Sat, Sep 26, 2026 at 11:48 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > skbedit can set skb->queue_mapping and raise the per-CPU skip_txqueue
> > flag so __dev_queue_xmit() honours the mapping. __dev_queue_xmit()
> > cleared the flag before sch_handle_egress() and only read it afterwards,
> > so the flag was not confined to the xmit that set it: a nested xmit
> > (mirred redirect or mirror, or a drop after skbedit) could set the flag
> > and the outer xmit would consume it for an skb that never went through
> > skbedit.
> >
> > A forwarded packet still carries the ingress NIC's rx_queue + 1 in
> > skb->queue_mapping, so the outer device then indexes its tx queue state
> > with that stale value. Taprio's child array q->qdiscs[] is sized to the
> > device's queue count, so taprio_enqueue() indexes past its allocation
> > and dereferences the result as a struct Qdisc *.
> >
> > Own the flag for the whole xmit frame: save the incoming value and clear
> > it before any of the frame's egress work can recurse, and restore it only
> > when the frame exits. A transmit-qdisc classifier is a documented flag
> > producer too (it runs in q->enqueue(), after sch_handle_egress()), so the
> > flag must be owned for the whole frame, not just around the clsact hook.
> > The flag then never crosses an xmit boundary in either direction. Also
> > store the value netdev_cap_txqueue() selected back into skb->queue_mapping
> > in netdev_tx_queue_mapping(), as netdev_core_pick_tx() already does, so
> > the skip_txqueue path never hands a later reader on the xmit path a
> > mapping the device cannot serve. A store made still later in the same
> > frame, by a tc BPF program attached to a transmit qdisc, is outside this
> > path and is not re-capped; a separate followup will resolve that path.
> >
> > netdev_xmit_skip_txqueue() now returns the previous flag value so the
> > save-and-clear is one call, and a no-op stub is provided when
> > CONFIG_NET_EGRESS is disabled. This removes the inline #ifdef around the
> > save and the two restores.
> >
> > A local user in a network namespace can redirect a packet from a device
> > with more TX queues to one with fewer after setting a mapping valid only
> > on the larger device. That reaches these reads and, under KASAN, faults
> > with "slab-out-of-bounds in taprio_enqueue".
> >
> > Conditions to recreate the bug: with CONFIG_NET_SCH_TAPRIO=y,
> > CONFIG_NET_ACT_SKBEDIT=y, CONFIG_NET_ACT_MIRRED=y,
> > CONFIG_NET_CLS_MATCHALL=y, CONFIG_NET_SCH_PRIO=y and KASAN enabled,
> > create qa (3 queues), qb (2 queues) and qc (1 queue) as dummy devices;
> > put a taprio root on qb and clsact on all three; then add an egress
> > matchall filter on every device. On qa: "action skbedit queue_mapping 2
> > pipe action mirred egress redirect dev qb". On qb: "action mirred egress
> > mirror dev qc". On qc: "action skbedit queue_mapping 0 pipe". Send one
> > packet out qa. qc's skbedit sets the flag while qb's outer xmit is in
> > flight; without the fix qb consumes it and reads its two-entry taprio
> > child array with the forwarded packet's stale mapping. A qc whose skbedit
> > is instead installed in a transmit-qdisc classifier (a matchall filter on
> > the qc root qdisc) reaches the same code path the same way.
> >
> > Testing: on a KASAN build with panic_on_warn=1 the unfixed kernel panics
> > with "BUG: KASAN: slab-out-of-bounds in taprio_enqueue", a read 0 bytes
> > past a 16-byte taprio_init() allocation; the fixed kernel runs the
> > clsact-setter and the transmit-qdisc-classifier reproducers with no report
> > and no clamp notice, and a clsact skbedit-then-tc-BPF store with no report
> > but the expected "selects TX queue" clamp notice from the write-back;
> >
> > Fixes: 2f1e85b1aee4 ("net: sched: use queue_mapping to pick tx queue")
> > Reported-by: Zero Day Initiative <zdi-disclosures@trendmicro.com>
> > Link: https://lore.kernel.org/netdev/CANn89iLwYx8nCVf0pCEk_MmEiyC6kQaMwCQT9WkQVeeNzNQHqQ@mail.gmail.com/
> > Link: https://lore.kernel.org/netdev/179008581937.2160803.7117814290574262942@kernel.org/
> > Link: https://lore.kernel.org/netdev/179033713973.2160803.4914570693994398206@kernel.org/
> > Link: https://lore.kernel.org/netdev/20260925180407.63647514@kernel.org/
> > Suggested-by: Eric Dumazet <edumazet@google.com>
> > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > Tested-by: hybris <hybris@mojatatu.ai>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > ---
> > v2 -> v3:
> > - netdev_xmit_skip_txqueue() returns the previous flag value, so the entry
> >   save-and-clear is one call; a no-op stub is provided when
> >   !CONFIG_NET_EGRESS, removing the three inline #ifdefs (Jakub Kicinski).
> > - Narrow the capability claim: the capped write-back covers the
> >   skip_txqueue path only; a store made later in the frame by a tc BPF
> >   program on a transmit qdisc is not re-capped and is a separate followup
> >   (nipa Sashiko).
> > - Comment: "per-CPU (per-task on PREEMPT_RT)" (nipa Sashiko).
> > - Drop the now-redundant #ifdef in act_skbedit.c.
> >
> > v1 -> v2:
> > - Rework the root cause to the per-CPU skip_txqueue flag lifetime: it was
> >   cleared before sch_handle_egress() and read after, so a nested xmit could
> >   set it and the outer xmit consume it for an skb that never went through
> >   skbedit (Eric Dumazet, nipa Sashiko).
> > - Own the flag for the whole xmit frame: save/clear it before any of the
> >   frame's egress work can recurse, and restore it only when the frame exits.
> > - Retain the v1 producer-side cap (netdev_tx_queue_mapping() writes the
> >   clamped value back).
> > - Correct the description of the reproducer to a three-device chain
> >   (qa 3q -> qb 2q taprio -> qc 1q).
> >
> >  include/linux/rtnetlink.h |  7 ++++++-
> >  net/core/dev.c            | 33 +++++++++++++++++++++++++++------
> >  net/sched/act_skbedit.c   |  2 --
> >  3 files changed, 33 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> > index 95729339e7a5..a54ec40d095c 100644
> > --- a/include/linux/rtnetlink.h
> > +++ b/include/linux/rtnetlink.h
> > @@ -186,7 +186,12 @@ void net_dec_ingress_queue(void);
> >  #ifdef CONFIG_NET_EGRESS
> >  void net_inc_egress_queue(void);
> >  void net_dec_egress_queue(void);
> > -void netdev_xmit_skip_txqueue(bool skip);
> > +bool netdev_xmit_skip_txqueue(bool skip);
> > +#else
> > +static inline bool netdev_xmit_skip_txqueue(bool skip)
> > +{
> > +       return false;
> > +}
> >  #endif
> >
> >  void rtnetlink_init(void);
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index f660fccfc0db..3b114fbf7015 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4404,9 +4404,14 @@ EXPORT_SYMBOL(dev_loopback_xmit);
> >  static struct netdev_queue *
> >  netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
> >  {
> > -       int qm = skb_get_queue_mapping(skb);
> > +       int queue = skb_get_queue_mapping(skb);
> > +       int capped;
> >
> > -       return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
> > +       capped = netdev_cap_txqueue(dev, queue);
> > +       if (unlikely(capped != queue))
> > +               skb_set_queue_mapping(skb, capped);
> > +
> > +       return netdev_get_tx_queue(dev, capped);
> >  }
> >
> >  #ifndef CONFIG_PREEMPT_RT
> > @@ -4415,9 +4420,13 @@ static bool netdev_xmit_txqueue_skipped(void)
> >         return __this_cpu_read(softnet_data.xmit.skip_txqueue);
> >  }
> >
> > -void netdev_xmit_skip_txqueue(bool skip)
> > +bool netdev_xmit_skip_txqueue(bool skip)
> >  {
> > +       bool prev = netdev_xmit_txqueue_skipped();
> > +
> >         __this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
> > +
> > +       return prev;
> >  }
> >  EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
> >
> > @@ -4427,9 +4436,13 @@ static bool netdev_xmit_txqueue_skipped(void)
> >         return current->net_xmit.skip_txqueue;
> >  }
> >
> > -void netdev_xmit_skip_txqueue(bool skip)
> > +bool netdev_xmit_skip_txqueue(bool skip)
> >  {
> > +       bool prev = netdev_xmit_txqueue_skipped();
> > +
> >         current->net_xmit.skip_txqueue = skip;
> > +
> > +       return prev;
> >  }
> >  EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
> >  #endif
> > @@ -4824,6 +4837,7 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> >         int cpu, rc = -ENOMEM;
> >         bool again = false;
> >         struct Qdisc *q;
> > +       bool skip_txq;
> >
> >         skb_reset_mac_header(skb);
> >         skb_assert_len(skb);
> > @@ -4846,6 +4860,13 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> >         skb_update_prio(skb);
> >
> >         tcx_set_ingress(skb, false);
> > +       /* The flag is per-CPU (per-task on PREEMPT_RT) and a nested xmit can
> > +        * set it from its own clsact hook or transmit qdisc. Own it for the
> > +        * whole frame: this frame cannot consume a nested xmit's flag and a
> > +        * nested xmit cannot inherit this frame's.
> > +        */
> > +       skip_txq = netdev_xmit_skip_txqueue(false);
>
> Hi Jamal
>
> Moving netdev_xmit_skip_txqueue() outside of
> static_branch_unlikely(&egress_needed_key) means every packet in
> __dev_queue_xmit() now pays for a per-CPU (or current->) read and two
> writes when CONFIG_NET_EGRESS=y, even when no egress hook is active.
>
> Commit 2f1e85b1aee4 specifically used egress_needed_key to avoid this
> fast-path cost.
>
> Also, in __dev_queue_xmit(), txq is picked before q->enqueue() runs, so
> tcf_skbedit_act() setting skip_txqueue from q->enqueue() (or from
> ingress) has no effect on the current packet and only pollutes the
> per-CPU flag.
>
> I noticed that nf_skip_egress(skb, true/false) already brackets
> sch_handle_egress(). If we make skb->nf_skip_egress available under
> CONFIG_NET_EGRESS (instead of CONFIG_NETFILTER_SKIP_EGRESS),
> tcf_skbedit_act() can only arm skip_txqueue when running in
> sch_handle_egress().
>

Sounds reasonable. I will look into it tomorrow.

pw-bot: cr

cheers,
jamal

> Given this idea my LLM produced the following (untested) patch:
>
> diff --git a/include/linux/netfilter_netdev.h b/include/linux/netfilter_netdev.h
> index 3175073a66ba6ba61db0f6d9d62c73ca15206ef0..2a854a0bbd4ba7b5c611b7aaf90b07711e57d9e0
> 100644
> --- a/include/linux/netfilter_netdev.h
> +++ b/include/linux/netfilter_netdev.h
> @@ -133,7 +133,7 @@ static inline struct sk_buff
> *nf_hook_egress(struct sk_buff *skb, int *rc,
>
>  static inline void nf_skip_egress(struct sk_buff *skb, bool skip)
>  {
> -#ifdef CONFIG_NETFILTER_SKIP_EGRESS
> +#ifdef CONFIG_NET_EGRESS
>         skb->nf_skip_egress = skip;
>  #endif
>  }
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index 95729339e7a54819b08e124bd36126cf71dfd9a4..a54ec40d095c041191ca44f7c4334aa37fa6b891
> 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -186,7 +186,12 @@ void net_dec_ingress_queue(void);
>  #ifdef CONFIG_NET_EGRESS
>  void net_inc_egress_queue(void);
>  void net_dec_egress_queue(void);
> -void netdev_xmit_skip_txqueue(bool skip);
> +bool netdev_xmit_skip_txqueue(bool skip);
> +#else
> +static inline bool netdev_xmit_skip_txqueue(bool skip)
> +{
> +       return false;
> +}
>  #endif
>
>  void rtnetlink_init(void);
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 84308498a3a8cd5a86364e0b15ddb35755732635..a3ff380d43f01db0b040573c9dd7acc4e2e2cda8
> 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1020,7 +1020,7 @@ struct sk_buff {
>  #ifdef CONFIG_NET_REDIRECT
>         __u8                    from_ingress:1;
>  #endif
> -#ifdef CONFIG_NETFILTER_SKIP_EGRESS
> +#ifdef CONFIG_NET_EGRESS
>         __u8                    nf_skip_egress:1;
>  #endif
>  #ifdef CONFIG_SKB_DECRYPTED
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index f35bd06a6bad680586da6b8ef20efc5d6b68fd6d..1acaadb3e2cf08827bb0d94e1649cfeb9e9c85ac
> 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -810,6 +810,15 @@ static inline bool skb_at_tc_ingress(const struct
> sk_buff *skb)
>  #endif
>  }
>
> +static inline bool skb_at_tc_egress(const struct sk_buff *skb)
> +{
> +#ifdef CONFIG_NET_EGRESS
> +       return skb->nf_skip_egress && !skb_at_tc_ingress(skb);
> +#else
> +       return false;
> +#endif
> +}
> +
>  static inline bool skb_skip_tc_classify(struct sk_buff *skb)
>  {
>  #ifdef CONFIG_NET_CLS_ACT
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f660fccfc0dbc56d7e1a9643525229b278bca547..8638c994bbcb0d1bf25ed603f833f105236a45bf
> 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4404,9 +4404,14 @@ EXPORT_SYMBOL(dev_loopback_xmit);
>  static struct netdev_queue *
>  netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
>  {
> -       int qm = skb_get_queue_mapping(skb);
> +       int queue = skb_get_queue_mapping(skb);
> +       int capped;
>
> -       return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
> +       capped = netdev_cap_txqueue(dev, queue);
> +       if (unlikely(capped != queue))
> +               skb_set_queue_mapping(skb, capped);
> +
> +       return netdev_get_tx_queue(dev, capped);
>  }
>
>  #ifndef CONFIG_PREEMPT_RT
> @@ -4415,9 +4420,13 @@ static bool netdev_xmit_txqueue_skipped(void)
>         return __this_cpu_read(softnet_data.xmit.skip_txqueue);
>  }
>
> -void netdev_xmit_skip_txqueue(bool skip)
> +bool netdev_xmit_skip_txqueue(bool skip)
>  {
> +       bool prev = netdev_xmit_txqueue_skipped();
> +
>         __this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
> +
> +       return prev;
>  }
>  EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
>
> @@ -4427,9 +4436,13 @@ static bool netdev_xmit_txqueue_skipped(void)
>         return current->net_xmit.skip_txqueue;
>  }
>
> -void netdev_xmit_skip_txqueue(bool skip)
> +bool netdev_xmit_skip_txqueue(bool skip)
>  {
> +       bool prev = netdev_xmit_txqueue_skipped();
> +
>         current->net_xmit.skip_txqueue = skip;
> +
> +       return prev;
>  }
>  EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
>  #endif
> @@ -4848,21 +4861,25 @@ int __dev_queue_xmit(struct sk_buff *skb,
> struct net_device *sb_dev)
>         tcx_set_ingress(skb, false);
>  #ifdef CONFIG_NET_EGRESS
>         if (static_branch_unlikely(&egress_needed_key)) {
> +               bool skip_txq;
> +
>                 if (nf_hook_egress_active()) {
>                         skb = nf_hook_egress(skb, &rc, dev);
>                         if (!skb)
>                                 goto out;
>                 }
>
> -               netdev_xmit_skip_txqueue(false);
> +               skip_txq = netdev_xmit_skip_txqueue(false);
>
>                 nf_skip_egress(skb, true);
>                 skb = sch_handle_egress(skb, &rc, dev);
> -               if (!skb)
> +               if (!skb) {
> +                       netdev_xmit_skip_txqueue(skip_txq);
>                         goto out;
> +               }
>                 nf_skip_egress(skb, false);
>
> -               if (netdev_xmit_txqueue_skipped())
> +               if (netdev_xmit_skip_txqueue(skip_txq))
>                         txq = netdev_tx_queue_mapping(dev, skb);
>         }
>  #endif
> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> index bfec6b66841031cd566d0c2bdc3d120cec41e3e4..26258b353cdeec48063413770439878d27ed60cf
> 100644
> --- a/net/sched/act_skbedit.c
> +++ b/net/sched/act_skbedit.c
> @@ -72,9 +72,8 @@ TC_INDIRECT_SCOPE int tcf_skbedit_act(struct sk_buff *skb,
>         }
>         if (params->flags & SKBEDIT_F_QUEUE_MAPPING &&
>             skb->dev->real_num_tx_queues > params->queue_mapping) {
> -#ifdef CONFIG_NET_EGRESS
> -               netdev_xmit_skip_txqueue(true);
> -#endif
> +               if (skb_at_tc_egress(skb))
> +                       netdev_xmit_skip_txqueue(true);
>                 skb_set_queue_mapping(skb, tcf_skbedit_hash(params, skb));
>         }
>         if (params->flags & SKBEDIT_F_MARK) {