From: Simon Horman <horms@kernel.org>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: netdev@vger.kernel.org, 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>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
Clark Williams <clrkwllms@kernel.org>,
Steven Rostedt <rostedt@goodmis.org>,
linux-rt-devel@lists.linux.dev, stable@vger.kernel.org,
Sashiko <sashiko-bot@kernel.org>,
zdi-disclosures@trendmicro.com
Subject: Re: [PATCH net v2 1/2] net/sched: sch_teql: fix slaves_lock IRQ deadlock
Date: Wed, 28 Oct 2026 14:27:29 +0000 [thread overview]
Message-ID: <20261028142729.GW257209@horms.kernel.org> (raw)
In-Reply-To: <QDISC-Y0BY.v2.r0.20261024060625-1@mojatatu.com>
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>
next prev parent reply other threads:[~2026-10-28 14:27 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-24 14:34 [PATCH net v2 0/2] net/sched: sch_teql: fix transmit-path regressions Jamal Hadi Salim
2026-10-24 14:34 ` [PATCH net v2 1/2] net/sched: sch_teql: fix slaves_lock IRQ deadlock Jamal Hadi Salim
2026-10-24 14:34 ` [PATCH net v2 2/2] net/sched: sch_teql: fix use-after-free on lower qdisc drop Jamal Hadi Salim
2026-10-27 18:26 ` Simon Horman
2026-10-27 21:19 ` Jamal Hadi Salim
2026-10-28 14:27 ` Simon Horman
2026-10-28 14:27 ` Simon Horman [this message]
2026-10-24 14:36 ` [PATCH net v2 0/2] net/sched: sch_teql: fix transmit-path regressions netdev-bot+sinfo
2026-10-24 14:40 ` Jamal Hadi Salim
2026-10-29 10:40 ` patchwork-bot+netdevbpf
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=20261028142729.GW257209@horms.kernel.org \
--to=horms@kernel.org \
--cc=bigeasy@linutronix.de \
--cc=clrkwllms@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@kernel.org \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=linux-rt-devel@lists.linux.dev \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=rostedt@goodmis.org \
--cc=sashiko-bot@kernel.org \
--cc=stable@vger.kernel.org \
--cc=victor@mojatatu.com \
--cc=zdi-disclosures@trendmicro.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.