linux-kernel - Re: [PATCH 6/9] lockdep: Introduce wait-type checks

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <4d95753e-a698-4564-aa8c-25737e95a61f@paulmck-laptop>
Date: Mon, 17 Nov 2025 06:37:26 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>
Cc: "bigeasy@...utronix.de" <bigeasy@...utronix.de>,
	"mingo@...nel.org" <mingo@...nel.org>,
	"peterz@...radead.org" <peterz@...radead.org>,
	"rostedt@...dmis.org" <rostedt@...dmis.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
	"joel@...lfernandes.org" <joel@...lfernandes.org>,
	"will@...nel.org" <will@...nel.org>
Subject: Re: [PATCH 6/9] lockdep: Introduce wait-type checks

On Mon, Nov 17, 2025 at 02:23:35PM +0000, Sverdlin, Alexander wrote:
> Hello Sebastian,
> 
> On Mon, 2025-11-17 at 14:55 +0100, bigeasy@...utronix.de wrote:
> > > [ BUG: Invalid wait context ]
> > > 6.18.0-rc1+git476309497a53 #1 Tainted: G           O       
> > > -----------------------------
> > > some-user-space-process/1251 is trying to lock:
> > > ffff000005cbc548 (&counter->events_list_lock){....}-{3:3}, at: counter_push_event+0x68/0x430 [counter]
> > > other info that might help us debug this:
> > > context-{2:2}
> > > no locks held by some-user-space-process/1251.
> > > stack backtrace:
> > > CPU: 0 UID: 0 PID: 1251 Comm: some-user-space-process Tainted: G           O        6.18.0-rc1+git476309497a53 #1 PREEMPT 
> > > Call trace:
> > >   show_stack+0x20/0x38 (C)
> > >   dump_stack_lvl+0x8c/0xd0
> > >   dump_stack+0x18/0x28
> > >   __lock_acquire+0x91c/0x1f78
> > >   lock_acquire+0x1c4/0x338
> > >   _raw_spin_lock_irqsave+0x68/0x98
> > >   counter_push_event+0x68/0x430 [counter]
> > >   interrupt_cnt_isr+0x40/0x78 [interrupt_cnt]
> > >   __handle_irq_event_percpu+0xa4/0x398
> > >   handle_irq_event+0x54/0xb8
> > >   handle_simple_irq+0xe4/0x128
> > >   handle_irq_desc+0x48/0x68
> > >   generic_handle_domain_irq+0x24/0x38
> > >   gpio_irq_handler+0xa0/0x140
> > >   handle_irq_desc+0x48/0x68
> > >   generic_handle_domain_irq+0x24/0x38
> > >   gic_handle_irq+0x54/0x128
> > >   call_on_irq_stack+0x30/0x70
> > >   do_interrupt_handler+0x88/0x98
> > >   el0_interrupt+0x5c/0x270
> > >   __el0_irq_handler_common+0x18/0x28
> > >   el0t_64_irq_handler+0x10/0x20
> > >   el0t_64_irq+0x198/0x1a0
> > > 
> > > This is not PREEMPT_RT, but still CONFIG_PROVE_RAW_LOCK_NESTING=y config...
> > > The relevant logic in the current v6.18-rcX is still largely unchanged
> > > from the Subject commit and...
> > > 
> > The validation is not limited to PREEMPT_RT enabled.
> > …
> > > > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > > > index 32406ef0d6a2d..4a9abf8bd9d15 100644
> > > > --- a/kernel/locking/lockdep.c
> > > > +++ b/kernel/locking/lockdep.c
> > …
> > > > +	 * Set appropriate wait type for the context; for IRQs we have to take
> > > > +	 * into account force_irqthread as that is implied by PREEMPT_RT.
> > > > +	 */
> > > > +	if (curr->hardirq_context) {
> > > > +		/*
> > > > +		 * Check if force_irqthreads will run us threaded.
> > > > +		 */
> > > > +		if (curr->hardirq_threaded)
> > > > +			curr_inner = LD_WAIT_CONFIG;
> > > > +		else
> > > > +			curr_inner = LD_WAIT_SPIN;
> > > 
> > > ... even though it's "if (curr->hardirq_threaded || curr->irq_config)" in the
> > > meanwhile, it is unclear to me, why is it conditional at all and not always
> > > "LD_WAIT_CONFIG" in hardirq context?
> > > 
> > > With !PREEMPT_RT but CONFIG_PROVE_RAW_LOCK_NESTING=y we always end up
> > > with LD_WAIT_SPIN here and it always warns if we take "spinlock_t" in hardirq
> > > context (as in the splat above). But the latter should be legal under all
> > > circumstances, isn't it?
> > 
> > Nope. The validation is performed always. The architectures which don't
> > support PREEMPT_RT are excluded for $reasons that is why
> > PROVE_RAW_LOCK_NESTING remains an option there.
> > 
> > A spinlock_t is a sleeping lock in a PREEMPT_RT context and the
> > interrupt hander are threaded. In this case a spinlock_t is okay.
> > However, if a spinlock_t is used in a non-forced-threaded environment
> > (such as in a primary handler with IRQF_NO_THREAD) then this handler
> > will not be threaded. Therefore spinlock_t is not a valid class.
> 
> I'm not in PREEMPT_RT, but now CONFIG_PROVE_RAW_LOCK_NESTING=y is forced
> for ARM64. And as I'm not in PREEMPT_RT, it's allowed to take spinlock_t
> in an IRQ, no matter what (that's what happens in the above splat, I
> believe).
> 
> So does it mean, that LOCKDEP is not usable any longer on ARM64
> (for !PREEMPT_RT kernels)?

LOCKDEP is quite usable, and in this case it is just doing its job by
preventing people from inadvertently breaking RT.  Which arm64 really
does support, as you can see by searching for "select ARCH_SUPPORTS_RT"
in arch/arm64/Kconfig.  So even arm64-specific code must follow the
PREEMPT_RT rules, which lockdep is enforcing.

So how to fix this?  One option is to use a raw_spinlock_t instead of a
spinlock_t.  This works because a raw_spinlock_t remains a spinlock in RT,
but it also restricts what you can do while holding the lock.

Does that help?

							Thanx, Paul