* [PATCH] powerpc: Use asm_goto_volatile for put_user()
@ 2020-11-04 11:17 Michael Ellerman
2020-11-04 11:20 ` Andreas Schwab
2020-11-08 10:29 ` Michael Ellerman
0 siblings, 2 replies; 3+ messages in thread
From: Michael Ellerman @ 2020-11-04 11:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: schwab
Andreas reported that commit ee0a49a6870e ("powerpc/uaccess: Switch
__put_user_size_allowed() to __put_user_asm_goto()") broke
CLONE_CHILD_SETTID.
Further inspection showed that the put_user() in schedule_tail() was
missing entirely, the store not emitted by the compiler.
<.schedule_tail>:
mflr r0
std r0,16(r1)
stdu r1,-112(r1)
bl <.finish_task_switch>
ld r9,2496(r3)
cmpdi cr7,r9,0
bne cr7,<.schedule_tail+0x60>
ld r3,392(r13)
ld r9,1392(r3)
cmpdi cr7,r9,0
beq cr7,<.schedule_tail+0x3c>
li r4,0
li r5,0
bl <.__task_pid_nr_ns>
nop
bl <.calculate_sigpending>
nop
addi r1,r1,112
ld r0,16(r1)
mtlr r0
blr
nop
nop
nop
bl <.__balance_callback>
b <.schedule_tail+0x1c>
Notice there are no stores other than to the stack. There should be a
stw in there for the store to current->set_child_tid.
This is only seen with GCC 4.9 era compilers (tested with 4.9.3 and
4.9.4), and only when CONFIG_PPC_KUAP is disabled.
When CONFIG_PPC_KUAP=y, the inline asm that's part of the isync()
and mtspr() inlined via allow_user_access() seems to be enough to
avoid the bug.
We already have a macro to work around this (or a similar bug), called
asm_volatile_goto which includes an empty asm block to tickle the
compiler into generating the right code. So use that.
With this applied the code generation looks more like it will work:
<.schedule_tail>:
mflr r0
std r31,-8(r1)
std r0,16(r1)
stdu r1,-144(r1)
std r3,112(r1)
bl <._mcount>
nop
ld r3,112(r1)
bl <.finish_task_switch>
ld r9,2624(r3)
cmpdi cr7,r9,0
bne cr7,<.schedule_tail+0xa0>
ld r3,2408(r13)
ld r31,1856(r3)
cmpdi cr7,r31,0
beq cr7,<.schedule_tail+0x80>
li r4,0
li r5,0
bl <.__task_pid_nr_ns>
nop
li r9,-1
clrldi r9,r9,12
cmpld cr7,r31,r9
bgt cr7,<.schedule_tail+0x80>
lis r9,16
rldicr r9,r9,32,31
subf r9,r31,r9
cmpldi cr7,r9,3
ble cr7,<.schedule_tail+0x80>
li r9,0
stw r3,0(r31) <-- stw
nop
bl <.calculate_sigpending>
nop
addi r1,r1,144
ld r0,16(r1)
ld r31,-8(r1)
mtlr r0
blr
nop
bl <.__balance_callback>
b <.schedule_tail+0x30>
Fixes: ee0a49a6870e ("powerpc/uaccess: Switch __put_user_size_allowed() to __put_user_asm_goto()")
Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/uaccess.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index ef5bbb705c08..501c9a79038c 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -178,7 +178,7 @@ do { \
* are no aliasing issues.
*/
#define __put_user_asm_goto(x, addr, label, op) \
- asm volatile goto( \
+ asm_volatile_goto( \
"1: " op "%U1%X1 %0,%1 # put_user\n" \
EX_TABLE(1b, %l2) \
: \
@@ -191,7 +191,7 @@ do { \
__put_user_asm_goto(x, ptr, label, "std")
#else /* __powerpc64__ */
#define __put_user_asm2_goto(x, addr, label) \
- asm volatile goto( \
+ asm_volatile_goto( \
"1: stw%X1 %0, %1\n" \
"2: stw%X1 %L0, %L1\n" \
EX_TABLE(1b, %l2) \
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] powerpc: Use asm_goto_volatile for put_user()
2020-11-04 11:17 [PATCH] powerpc: Use asm_goto_volatile for put_user() Michael Ellerman
@ 2020-11-04 11:20 ` Andreas Schwab
2020-11-08 10:29 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Andreas Schwab @ 2020-11-04 11:20 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
With that patch the kernel is working again.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] powerpc: Use asm_goto_volatile for put_user()
2020-11-04 11:17 [PATCH] powerpc: Use asm_goto_volatile for put_user() Michael Ellerman
2020-11-04 11:20 ` Andreas Schwab
@ 2020-11-08 10:29 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2020-11-08 10:29 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman; +Cc: schwab
On Wed, 4 Nov 2020 22:17:42 +1100, Michael Ellerman wrote:
> Andreas reported that commit ee0a49a6870e ("powerpc/uaccess: Switch
> __put_user_size_allowed() to __put_user_asm_goto()") broke
> CLONE_CHILD_SETTID.
>
> Further inspection showed that the put_user() in schedule_tail() was
> missing entirely, the store not emitted by the compiler.
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc: Use asm_goto_volatile for put_user()
https://git.kernel.org/powerpc/c/1344a232016dbb0492be81f8517c4bf8fc1c6610
cheers
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-08 10:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-04 11:17 [PATCH] powerpc: Use asm_goto_volatile for put_user() Michael Ellerman
2020-11-04 11:20 ` Andreas Schwab
2020-11-08 10:29 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).