netdev - Re: netconsole: HARDIRQ-safe -> HARDIRQ-unsafe lock order warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7fc8a1db60de959fd22ae898e86683f57fb07be2.camel@gmx.de>
Date: Sat, 06 Sep 2025 04:32:41 +0200
From: Mike Galbraith <efault@....de>
To: John Ogness <john.ogness@...utronix.de>, Breno Leitao
 <leitao@...ian.org>,  Simon Horman <horms@...nel.org>, kuba@...nel.org,
 calvin@...nvd.org
Cc: Pavel Begunkov <asml.silence@...il.com>, Johannes Berg
 <johannes@...solutions.net>, paulmck@...nel.org, LKML
 <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org, boqun.feng@...il.com
Subject: Re: netconsole: HARDIRQ-safe -> HARDIRQ-unsafe lock order warning

On Fri, 2025-09-05 at 14:54 +0206, John Ogness wrote:
> 
> On 2025-08-26, Breno Leitao <leitao@...ian.org> wrote:
> > On Fri, Aug 22, 2025 at 05:54:28AM +0200, Mike Galbraith wrote:
> > > On Thu, 2025-08-21 at 10:35 -0700, Breno Leitao wrote:
> > > > > On Thu, Aug 21, 2025 at 05:51:59AM +0200, Mike Galbraith wrote:
> > >  
> > > > > > > --- a/drivers/net/netconsole.c
> > > > > > > +++ b/drivers/net/netconsole.c
> > > > > > > @@ -1952,12 +1952,12 @@ static void netcon_write_thread(struct c
> > > > > > >  static void netconsole_device_lock(struct console *con, unsigned long *flags)
> > > > > > >  {
> > > > > > >  	/* protects all the targets at the same time */
> > > > > > > -	spin_lock_irqsave(&target_list_lock, *flags);
> > > > > > > +	spin_lock(&target_list_lock);
> > > > > 
> > > > > I personally think this target_list_lock can be moved to an RCU lock.
> > > > > 
> > > > > If that is doable, then we probably make netconsole_device_lock()
> > > > > to a simple `rcu_read_lock()`, which would solve this problem as well.
> > > 
> > > The bigger issue for the nbcon patch would seem to be the seemingly
> > > required .write_atomic leading to landing here with disabled IRQs.
> 
> Using spin_lock_irqsave()/spin_unlock_irqrestore() within the
> ->device_lock() and ->device->unlock() callbacks is fine. Even with
> PREEMPT_RT this is fine. If you can use RCU to synchronize the target
> list, that is probably a nice optimization, but it is certainly not a
> requirement from the NBCON (and PREEMPT_RT/lockdep) perspective.

I was referring there to !RT+wireless locking meets IRQs disabled, ever
per lockdep, an intolerance shared with RT+spinlock_t.

> > In this case, instead of transmitting through netpoll directly in the
> > .write_atomic context, we could queue the messages for later delivery.
> 
> The ->write_atomic() callback is intended to perform immediate
> transmission. It is called with hardware interrupts disabled and is even
> expected to work from NMI context. If you are not able to implement
> these requirements, do not implement ->write_atomic(). Implementing some
> sort of deferrment mechanism is inappropriate. Such a mechanism already
> exists based on ->write_thread().

Truly atomic packet blasting would be a case of happiness, but barring
that, deferment is way better than the nothing that's available to both
RT and !RT+wireless here/now.  With a .write_atomic that's really just
.write_thread, both RT and !RT+wireless managed to successfully send a
death rattle with the WIP nbcon patch.. a progression for each of them.

> 
	-Mike