linux-kernel - Re: [GIT pull] locking/urgent for v5.10-rc6

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]

Date:   Wed, 2 Dec 2020 10:38:05 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Heiko Carstens <hca@...ux.ibm.com>
Cc:     Sven Schnelle <svens@...ux.ibm.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>
Subject: Re: [GIT pull] locking/urgent for v5.10-rc6

On Wed, Dec 02, 2020 at 08:54:27AM +0100, Heiko Carstens wrote:
> > > But but but...
> > > 
> > >   do_idle()			# IRQs on
> > >     local_irq_disable();	# IRQs off
> > >     defaul_idle_call()	# IRQs off
> > 	lockdep_hardirqs_on();	# IRQs off, but lockdep things they're on
> > >       arch_cpu_idle()		# IRQs off
> > >         enabled_wait()	# IRQs off
> > > 	  raw_local_save()	# still off
> > > 	  psw_idle()		# very much off
> > > 	    ext_int_handler	# get an interrupt ?!?!
> > 	      rcu_irq_enter()	# lockdep thinks IRQs are on <- FAIL
> > 
> > I can't much read s390 assembler, but ext_int_handler() has a
> > TRACE_IRQS_OFF, which would be sufficient to re-align the lockdep state
> > with the actual state, but there's some condition before it, what's that
> > test and is that right?
> 
> After digging a bit into our asm code: no, it is not right, and only
> for psw_idle() it is wrong.
> 
> What happens with the current code:
> 
> - default_idle_call() calls lockdep_hardirqs_on() before calling into
>   arch_cpu_idle()

Correct.

> - our arch_cpu_idle() calls psw_idle() which enables irqs. the irq
>   handler will call/use the SWITCH_ASYNC macro which clears the
>   interrupt enabled bits in the old program status word (_only_ for
>   psw_idle)

This is the condition at 0: that compares r13 to psw_idle_exit?

> - this again causes the interrupt handler to _not_ call TRACE_IRQS_OFF
>   and therefore lockdep thinks interrupts are enabled within the
>   interrupt handler
> 
> So I guess my patch which I sent yesterday evening should fix all that
> mess

Yes, afaict it does the right thing. Exceptions should call
TRACE_IRQS_OFF unconditionally, since the hardware will disable
interrupts upon taking an exception, irrespective of what the previous
context had.

On exception return the previous IRQ state is inspected and lockdep is
changed to match (except for NMIs, which either are ignored by lockdep
or need a little bit of extra care).