linux-kernel - Re: [PATCH] drivers: cpuidle: Disable preemption before get_lppaca function call in pseries_idle

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: <1500585713.10674.10.camel@kernel.crashing.org>
Date:   Fri, 21 Jul 2017 07:21:53 +1000
From:   Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:     Victor Aoqui <victora@...ux.vnet.ibm.com>, rjw@...ysocki.net,
        daniel.lezcano@...aro.org, paulus@...ba.org, mpe@...erman.id.au,
        linux-pm@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-kernel@...r.kernel.org
Cc:     ltc-interlock@...ts.linux.ibm.com, victora@...ibm.com,
        mauricfo@...ux.vnet.ibm.com
Subject: Re: [PATCH] drivers: cpuidle: Disable preemption before get_lppaca
 function call in pseries_idle_probe function

On Thu, 2017-07-20 at 14:57 -0300, Victor Aoqui wrote:
> When CONFIG_PREEMPT=y, the following warning shows up:
> 
> BUG: using smp_processor_id() in preemptible [00000000] code: swapper/0/1
> caller is pseries_processor_idle_init+0x58/0x21c
> 
> This warning shows up because preemption cannot occur when using
> get_paca(), otherwise the paca_struct it points to may be the wrong one
> just after.
> 
> For this reason, preemption needs to be disabled before
> lppaca_shared_proc(get_lppaca()).

Also chekc the generated assembly. We had all sort of interesting
issues where gcc would copy the paca pointer or the lppaca pointer
to a GPR *outside* of the preempt disabled section...

In that specific case it's not a big deal but overall, I am not
comfortable with PREEMPT on powerpc until we do something a bit
more drastic...

I would like to remove all such direct accesses to paca, instead have a
"new" get_paca() written in asm that does the preempt disable then
returns the PACA in a GPR (not directly use r13, hide that from gcc),
and which is paired with a put_paca().

The few places where we want to directly access r13 should be hand
written in asm too to hide r13 from gcc, for accessing the irq_happened
in the fast path of local_irq_enable/disable/... we should do the same
with lock tokens.

Ben.