[PATCH net v2 0/2] net/sched: sch_teql: fix transmit-path regressions

[PATCH net v2 0/2] net/sched: sch_teql: fix transmit-path regressions

[Jamal Hadi Salim]

v2: resend of the two-patch series.

- Patch 1/2 is unchanged

- Patch 2/2 now fixes the drop accounting raised by the Sashiko
  review of v1 (both providers): only NETDEV_TX_OK and NET_XMIT_CN
  count as accepted transmits (CN can accompany acceptance of the
  current packet); any other consumed result, e.g. NET_XMIT_DROP,
  is charged to tx_dropped instead of being counted as a good
  transmit.  The skb-ownership fix is unchanged (Sashiko-gemini).

The v2 revision follows the review of patch 2/2
(https://lore.kernel.org/netdev/179248763469.3135.1739559328868319628@kernel.org/)
and Simon Horman's request for a response
(https://lore.kernel.org/netdev/20261023162821.GN708417@horms.kernel.org/).

Two independent sch_teql regressions on the transmit path.

Patch 1 makes slaves_lock safe for the hard-IRQ-disabled netpoll path and
for PREEMPT_RT by converting it to a raw spinlock taken with IRQs saved.

Patch 2 fixes a use-after-free when a lower virtual device consumes an skb
but returns NET_XMIT_DROP rather than NETDEV_TX_OK, and stops counting
those consumed drops as good transmits.

Jamal Hadi Salim (1):
  net/sched: sch_teql: fix slaves_lock IRQ deadlock

owen kwan (1):
  net/sched: sch_teql: fix use-after-free on lower qdisc drop

 net/sched/sch_teql.c | 60 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 22 deletions(-)

-- 
2.43.0

[PATCH net v2 1/2] net/sched: sch_teql: fix slaves_lock IRQ deadlock

[Jamal Hadi Salim]

This is a followup of commit e5b811fe7931 ("net/sched: sch_teql:
Introduce slaves_lock to avoid race condition and UAF") as reported by
Sashiko.

The issue: master->slaves_lock serializes the master's ring of slave
qdiscs.  The control paths -- teql_dequeue(), teql_destroy() and
teql_qdisc_init() -- take it with spin_lock_bh(), while
teql_master_xmit() (the master's ndo_start_xmit) takes it with plain
spin_lock().

netpoll_send_skb() wraps the transmit in local_irq_save(), and
__netpoll_send_skb() asserts lockdep_assert_irqs_disabled(), so
teql_master_xmit() reached from netconsole always acquires slaves_lock
with hard IRQs disabled.  On PREEMPT_RT spinlock_t is an rtmutex, and
Documentation/locking/locktypes.rst names

	local_irq_disable();
	spin_lock(&lock);

as a sequence that breaks there because the rtmutex requires a fully
preemptible context.  This is reachable without any panic: the nbcon
printer thread calls netconsole's write_thread() callback on every
message.  The usual remedy -- folding the IRQ disable into the lock with
spin_lock_irqsave() -- is unavailable because the local_irq_save() is
netpoll's, not ours.

On non-RT the same asymmetry leaves slaves_lock acquirable with hard IRQs
disabled on one path and held with hard IRQs enabled on another.  That is
only exploitable during the panic console flush (netconsole is
CON_NBCON_ATOMIC_UNSAFE, so console_is_usable() admits its write_atomic()
only from nbcon_atomic_flush_unsafe(), which panic() alone calls), and
even then an IRQ-saving lock does not help if the holder oopsed inside the
section or was stopped by panic_other_cpus_shutdown().  So the non-RT half
is hardening; the PREEMPT_RT half is the bug.

The Fix: take slaves_lock as a raw spinlock with IRQs saved at every
site.  Since skb_queue_purge() takes the queue's spinlock_t and may not
nest inside a raw lock on PREEMPT_RT, move its call in teql_destroy()
outside the raw section; dat->q is the dying qdisc's own queue and was
never serialized by slaves_lock, so the move is behaviour-neutral.

The new critical sections contain only RCU pointer assignments and
netdev_get_tx_queue(); skb_queue_purge(), netif_wake_queue() and
dev_reset_queue() are all outside them.  teql_destroy()'s ring walk is
O(slaves), two pointer loads and a compare per iteration, so even a large
ring is microseconds with IRQs off.  Keeping the purge conditional rather
than dropping it outright is the conservative call for a backport.

Some context: commit 6301f6a34ed8 ("net/sched: sch_teql: move
rcu_read_lock()/spin_lock() from _bh variants") hardened the parent
fix; together they addressed the master->slaves resurrection
use-after-free and the xmit-side BH warning but left the reverse
direction -- a hard IRQ re-entering xmit while a control path holds the
lock -- unaddressed.

Conditions to recreate the bug:
- CONFIG_PREEMPT_RT, CONFIG_NET_SCH_TEQL, CONFIG_DUMMY and
  CONFIG_NETCONSOLE.
- Run `ip link add dev teqlslave0 type dummy`, bring teqlslave0 up, attach
  it with `tc qdisc add dev teqlslave0 root teql0`, and bring teql0 up.
- Configure a netconsole target on teql0 with explicit local and remote
  IP addresses and a remote MAC address, then emit a console message.
  The nbcon printer thread reaches teql_master_xmit() with hard IRQs
  disabled by netpoll; taking slaves_lock as a spinlock_t is invalid on
  PREEMPT_RT. No panic or concurrent qdisc change is required.
- Reachable by an admin with CAP_NET_ADMIN; teql devices exist only in the
  initial netns (no per-netns ops), so a user namespace does not suffice.

Fixes: e5b811fe7931 ("net/sched: sch_teql: Introduce slaves_lock to avoid race condition and UAF")
Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260630150922.238714-1-jhs%40mojatatu.com
Link: https://lore.kernel.org/netdev/20260630150922.238714-1-jhs@mojatatu.com/
Cc: stable@vger.kernel.org
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---

 net/sched/sch_teql.c | 45 +++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 409ce50cc0db..c7dcec25ef42 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -53,7 +53,7 @@ struct teql_master {
 	struct Qdisc_ops qops;
 	struct net_device *dev;
 	struct Qdisc __rcu	*slaves;
-	spinlock_t		slaves_lock; /* serializes writes to ->slaves */
+	raw_spinlock_t		slaves_lock; /* serializes writes to ->slaves */
 	struct list_head master_list;
 	unsigned long	tx_bytes;
 	unsigned long	tx_packets;
@@ -92,6 +92,7 @@ teql_dequeue(struct Qdisc *sch)
 {
 	struct teql_sched_data *dat = qdisc_priv(sch);
 	struct netdev_queue *dat_queue;
+	unsigned long flags;
 	struct sk_buff *skb;
 	struct Qdisc *q;
 
@@ -102,9 +103,9 @@ teql_dequeue(struct Qdisc *sch)
 	if (skb == NULL) {
 		struct net_device *m = qdisc_dev(q);
 		if (m) {
-			spin_lock_bh(&dat->m->slaves_lock);
+			raw_spin_lock_irqsave(&dat->m->slaves_lock, flags);
 			rcu_assign_pointer(dat->m->slaves, sch);
-			spin_unlock_bh(&dat->m->slaves_lock);
+			raw_spin_unlock_irqrestore(&dat->m->slaves_lock, flags);
 			netif_wake_queue(m);
 		}
 	} else {
@@ -137,11 +138,13 @@ teql_destroy(struct Qdisc *sch)
 	struct teql_master *master = dat->m;
 	struct netdev_queue *txq = NULL;
 	bool reset_master_queue = false;
+	unsigned long flags;
+	bool purge = false;
 
 	if (!master)
 		return;
 
-	spin_lock_bh(&master->slaves_lock);
+	raw_spin_lock_irqsave(&master->slaves_lock, flags);
 	prev = rcu_dereference_protected(master->slaves,
 					 lockdep_is_held(&master->slaves_lock));
 	if (prev) {
@@ -169,12 +172,18 @@ teql_destroy(struct Qdisc *sch)
 					reset_master_queue = true;
 				}
 			}
-			skb_queue_purge(&dat->q);
+			purge = true;
 			break;
 		} while (prev != rcu_dereference_protected(master->slaves,
 							   lockdep_is_held(&master->slaves_lock)));
 	}
-	spin_unlock_bh(&master->slaves_lock);
+	raw_spin_unlock_irqrestore(&master->slaves_lock, flags);
+
+	/* skb_queue_purge() takes the queue's spinlock_t, which cannot nest
+	 * inside a raw lock on PREEMPT_RT; purge after the raw section.
+	 */
+	if (purge)
+		skb_queue_purge(&dat->q);
 
 	if (reset_master_queue)
 		dev_reset_queue(master->dev, txq, NULL);
@@ -187,6 +196,7 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 	struct teql_master *m = (struct teql_master *)sch->ops;
 	struct teql_sched_data *q = qdisc_priv(sch);
 	struct Qdisc *first;
+	unsigned long flags;
 
 	if (dev->hard_header_len > m->dev->hard_header_len)
 		return -EINVAL;
@@ -203,7 +213,7 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 
 	skb_queue_head_init(&q->q);
 
-	spin_lock_bh(&m->slaves_lock);
+	raw_spin_lock_irqsave(&m->slaves_lock, flags);
 	first = rcu_dereference_protected(m->slaves, lockdep_is_held(&m->slaves_lock));
 	if (first) {
 		if (m->dev->flags & IFF_UP) {
@@ -214,7 +224,7 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 			    (m->dev->flags & IFF_MULTICAST &&
 			     !(dev->flags & IFF_MULTICAST)) ||
 			    dev->mtu < m->dev->mtu) {
-				spin_unlock_bh(&m->slaves_lock);
+				raw_spin_unlock_irqrestore(&m->slaves_lock, flags);
 				return -EINVAL;
 			}
 		} else {
@@ -237,7 +247,7 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 		m->dev->mtu = dev->mtu;
 		m->dev->flags = (m->dev->flags&~FMASK)|(dev->flags&FMASK);
 	}
-	spin_unlock_bh(&m->slaves_lock);
+	raw_spin_unlock_irqrestore(&m->slaves_lock, flags);
 	return 0;
 }
 
@@ -302,11 +312,12 @@ static inline int teql_resolve(struct sk_buff *skb,
 static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct teql_master *master = netdev_priv(dev);
-	struct Qdisc *start, *q;
-	int busy;
-	int nores;
 	int subq = skb_get_queue_mapping(skb);
 	struct sk_buff *skb_res = NULL;
+	struct Qdisc *start, *q;
+	unsigned long flags;
+	int nores;
+	int busy;
 
 restart:
 	nores = 0;
@@ -342,13 +353,13 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 				    netdev_start_xmit(skb, slave, slave_txq, false) ==
 				    NETDEV_TX_OK) {
 					__netif_tx_unlock(slave_txq);
-					spin_lock(&master->slaves_lock);
+					raw_spin_lock_irqsave(&master->slaves_lock, flags);
 					if (rcu_dereference_protected(master->slaves,
 								      lockdep_is_held(&master->slaves_lock)) == q)
 						rcu_assign_pointer(master->slaves,
 								   rcu_dereference_protected(NEXT_SLAVE(q),
 											     lockdep_is_held(&master->slaves_lock)));
-					spin_unlock(&master->slaves_lock);
+					raw_spin_unlock_irqrestore(&master->slaves_lock, flags);
 					netif_wake_queue(dev);
 					master->tx_packets++;
 					master->tx_bytes += length;
@@ -361,13 +372,13 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 				busy = 1;
 			break;
 		case 1:
-			spin_lock(&master->slaves_lock);
+			raw_spin_lock_irqsave(&master->slaves_lock, flags);
 			if (rcu_dereference_protected(master->slaves,
 						      lockdep_is_held(&master->slaves_lock)) == q)
 				rcu_assign_pointer(master->slaves,
 						   rcu_dereference_protected(NEXT_SLAVE(q),
 									     lockdep_is_held(&master->slaves_lock)));
-			spin_unlock(&master->slaves_lock);
+			raw_spin_unlock_irqrestore(&master->slaves_lock, flags);
 			rcu_read_unlock();
 			return NETDEV_TX_OK;
 		default:
@@ -489,7 +500,7 @@ static __init void teql_master_setup(struct net_device *dev)
 	struct teql_master *master = netdev_priv(dev);
 	struct Qdisc_ops *ops = &master->qops;
 
-	spin_lock_init(&master->slaves_lock);
+	raw_spin_lock_init(&master->slaves_lock);
 	master->dev	= dev;
 	ops->priv_size  = sizeof(struct teql_sched_data);
 
-- 
2.43.0

Re: [PATCH net v2 0/2] net/sched: sch_teql: fix transmit-path regressions

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2 0/2] net/sched: sch_teql: fix transmit-path regressions

[Jamal Hadi Salim]

Patch 1 is a followup of a report by sashiko and  two was an
independent bug reported by Owen that our reproduction system verified
- i believe they found it by using an LLM to look for similar patterns
of some previously fixed bug.

cheers,
jamal

On Sat, Oct 24, 2026 at 10:36 AM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH net v2 1/2] net/sched: sch_teql: fix slaves_lock IRQ deadlock

[Simon Horman]

On Sat, Oct 24, 2026 at 10:34:14AM -0400, Jamal Hadi Salim wrote:
> This is a followup of commit e5b811fe7931 ("net/sched: sch_teql:
> Introduce slaves_lock to avoid race condition and UAF") as reported by
> Sashiko.
> 
> The issue: master->slaves_lock serializes the master's ring of slave
> qdiscs.  The control paths -- teql_dequeue(), teql_destroy() and
> teql_qdisc_init() -- take it with spin_lock_bh(), while
> teql_master_xmit() (the master's ndo_start_xmit) takes it with plain
> spin_lock().
> 
> netpoll_send_skb() wraps the transmit in local_irq_save(), and
> __netpoll_send_skb() asserts lockdep_assert_irqs_disabled(), so
> teql_master_xmit() reached from netconsole always acquires slaves_lock
> with hard IRQs disabled.  On PREEMPT_RT spinlock_t is an rtmutex, and
> Documentation/locking/locktypes.rst names
> 
> 	local_irq_disable();
> 	spin_lock(&lock);
> 
> as a sequence that breaks there because the rtmutex requires a fully
> preemptible context.  This is reachable without any panic: the nbcon
> printer thread calls netconsole's write_thread() callback on every
> message.  The usual remedy -- folding the IRQ disable into the lock with
> spin_lock_irqsave() -- is unavailable because the local_irq_save() is
> netpoll's, not ours.
> 
> On non-RT the same asymmetry leaves slaves_lock acquirable with hard IRQs
> disabled on one path and held with hard IRQs enabled on another.  That is
> only exploitable during the panic console flush (netconsole is
> CON_NBCON_ATOMIC_UNSAFE, so console_is_usable() admits its write_atomic()
> only from nbcon_atomic_flush_unsafe(), which panic() alone calls), and
> even then an IRQ-saving lock does not help if the holder oopsed inside the
> section or was stopped by panic_other_cpus_shutdown().  So the non-RT half
> is hardening; the PREEMPT_RT half is the bug.
> 
> The Fix: take slaves_lock as a raw spinlock with IRQs saved at every
> site.  Since skb_queue_purge() takes the queue's spinlock_t and may not
> nest inside a raw lock on PREEMPT_RT, move its call in teql_destroy()
> outside the raw section; dat->q is the dying qdisc's own queue and was
> never serialized by slaves_lock, so the move is behaviour-neutral.
> 
> The new critical sections contain only RCU pointer assignments and
> netdev_get_tx_queue(); skb_queue_purge(), netif_wake_queue() and
> dev_reset_queue() are all outside them.  teql_destroy()'s ring walk is
> O(slaves), two pointer loads and a compare per iteration, so even a large
> ring is microseconds with IRQs off.  Keeping the purge conditional rather
> than dropping it outright is the conservative call for a backport.
> 
> Some context: commit 6301f6a34ed8 ("net/sched: sch_teql: move
> rcu_read_lock()/spin_lock() from _bh variants") hardened the parent
> fix; together they addressed the master->slaves resurrection
> use-after-free and the xmit-side BH warning but left the reverse
> direction -- a hard IRQ re-entering xmit while a control path holds the
> lock -- unaddressed.
> 
> Conditions to recreate the bug:
> - CONFIG_PREEMPT_RT, CONFIG_NET_SCH_TEQL, CONFIG_DUMMY and
>   CONFIG_NETCONSOLE.
> - Run `ip link add dev teqlslave0 type dummy`, bring teqlslave0 up, attach
>   it with `tc qdisc add dev teqlslave0 root teql0`, and bring teql0 up.
> - Configure a netconsole target on teql0 with explicit local and remote
>   IP addresses and a remote MAC address, then emit a console message.
>   The nbcon printer thread reaches teql_master_xmit() with hard IRQs
>   disabled by netpoll; taking slaves_lock as a spinlock_t is invalid on
>   PREEMPT_RT. No panic or concurrent qdisc change is required.
> - Reachable by an admin with CAP_NET_ADMIN; teql devices exist only in the
>   initial netns (no per-netns ops), so a user namespace does not suffice.
> 
> Fixes: e5b811fe7931 ("net/sched: sch_teql: Introduce slaves_lock to avoid race condition and UAF")
> Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260630150922.238714-1-jhs%40mojatatu.com
> Link: https://lore.kernel.org/netdev/20260630150922.238714-1-jhs@mojatatu.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v2 0/2] net/sched: sch_teql: fix transmit-path regressions

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Sat, 24 Oct 2026 10:34:13 -0400 you wrote:
> v2: resend of the two-patch series.
> 
> - Patch 1/2 is unchanged
> 
> - Patch 2/2 now fixes the drop accounting raised by the Sashiko
>   review of v1 (both providers): only NETDEV_TX_OK and NET_XMIT_CN
>   count as accepted transmits (CN can accompany acceptance of the
>   current packet); any other consumed result, e.g. NET_XMIT_DROP,
>   is charged to tx_dropped instead of being counted as a good
>   transmit.  The skb-ownership fix is unchanged (Sashiko-gemini).
> 
> [...]

Here is the summary with links:
  - [net,v2,1/2] net/sched: sch_teql: fix slaves_lock IRQ deadlock
    https://git.kernel.org/netdev/net/c/8b2043279dae
  - [net,v2,2/2] net/sched: sch_teql: fix use-after-free on lower qdisc drop
    https://git.kernel.org/netdev/net/c/782d6bb8f9b8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html