[PATCH] powerpc/tm: do not use r13 for tabort_syscall

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE3632d

3 messages, 2 authors, 2016-08-22 · open the first message on its own page

[PATCH] powerpc/tm: do not use r13 for tabort_syscall

From: Nicholas Piggin <npiggin@gmail.com>
Date: 2016-07-22 07:28:07

tabort_syscall runs with RI=1, so a nested recoverable machine
check will load the paca into r13 and overwrite what we loaded
it with, because exceptions returning to privileged mode do not
restore r13.

This has survived testing with sc instruction inside transaction
(bare sc, not glibc syscall because glibc can tabort before sc).
Verified the transaction is failing failing with with
TM_CAUSE_SYSCALL.

Signed-off-by: Nick Piggin <npiggin@gmail.com>
Cc: Michael Neuling <redacted>
Cc: Sam Bobroff <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>

---

 arch/powerpc/kernel/entry_64.S | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 73e461a..387dee3 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -368,13 +368,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 tabort_syscall:
 	/* Firstly we need to enable TM in the kernel */
 	mfmsr	r10
-	li	r13, 1
-	rldimi	r10, r13, MSR_TM_LG, 63-MSR_TM_LG
-	mtmsrd	r10, 0
+	li	r9,1
+	rldimi	r10,r9,MSR_TM_LG,63-MSR_TM_LG
+	mtmsrd	r10,0
 
 	/* tabort, this dooms the transaction, nothing else */
-	li	r13, (TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT)
-	TABORT(R13)
+	li	r9,(TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT)
+	TABORT(R9)
 
 	/*
 	 * Return directly to userspace. We have corrupted user register state,
@@ -382,11 +382,11 @@ tabort_syscall:
 	 * resume after the tbegin of the aborted transaction with the
 	 * checkpointed register state.
 	 */
-	li	r13, MSR_RI
-	andc	r10, r10, r13
-	mtmsrd	r10, 1
-	mtspr	SPRN_SRR0, r11
-	mtspr	SPRN_SRR1, r12
+	li	r9,MSR_RI
+	andc	r10,r10,r9
+	mtmsrd	r10,1
+	mtspr	SPRN_SRR0,r11
+	mtspr	SPRN_SRR1,r12
 
 	rfid
 	b	.	/* prevent speculative execution */
-- 
2.8.1

Re: [PATCH] powerpc/tm: do not use r13 for tabort_syscall

From: Michael Neuling <hidden>
Date: 2016-07-25 00:57:43

On Fri, 2016-07-22 at 17:27 +1000, Nicholas Piggin wrote:
tabort_syscall runs with RI=3D1, so a nested recoverable machine
check will load the paca into r13 and overwrite what we loaded
it with, because exceptions returning to privileged mode do not
restore r13.
=20
This has survived testing with sc instruction inside transaction
(bare sc, not glibc syscall because glibc can tabort before sc).
Verified the transaction is failing failing with with
TM_CAUSE_SYSCALL.
=20
Signed-off-by: Nick Piggin <npiggin@gmail.com>
Thanks.

This looks good, but should probably be cc: stable from when the syscall tm
abort went in.

There are some random whitespace changes in here too, which if we avoid
will make the patch smaller (and easier to read).

Mikey
quoted hunk
Cc: Michael Neuling <redacted>
Cc: Sam Bobroff <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
=20
---
=20
=C2=A0arch/powerpc/kernel/entry_64.S | 20 ++++++++++----------
=C2=A01 file changed, 10 insertions(+), 10 deletions(-)
=20
diff --git a/arch/powerpc/kernel/entry_64.S
b/arch/powerpc/kernel/entry_64.S
index 73e461a..387dee3 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -368,13 +368,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
=C2=A0tabort_syscall:
=C2=A0	/* Firstly we need to enable TM in the kernel */
=C2=A0	mfmsr	r10
-	li	r13, 1
-	rldimi	r10, r13, MSR_TM_LG, 63-MSR_TM_LG
-	mtmsrd	r10, 0
+	li	r9,1
+	rldimi	r10,r9,MSR_TM_LG,63-MSR_TM_LG
+	mtmsrd	r10,0
=C2=A0
=C2=A0	/* tabort, this dooms the transaction, nothing else */
-	li	r13, (TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT)
-	TABORT(R13)
+	li	r9,(TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT)
+	TABORT(R9)
=C2=A0
=C2=A0	/*
=C2=A0	=C2=A0* Return directly to userspace. We have corrupted user regis=
ter
quoted hunk
state,
@@ -382,11 +382,11 @@ tabort_syscall:
=C2=A0	=C2=A0* resume after the tbegin of the aborted transaction with th=
e
=C2=A0	=C2=A0* checkpointed register state.
=C2=A0	=C2=A0*/
-	li	r13, MSR_RI
-	andc	r10, r10, r13
-	mtmsrd	r10, 1
-	mtspr	SPRN_SRR0, r11
-	mtspr	SPRN_SRR1, r12
+	li	r9,MSR_RI
+	andc	r10,r10,r9
+	mtmsrd	r10,1
+	mtspr	SPRN_SRR0,r11
+	mtspr	SPRN_SRR1,r12
=C2=A0
=C2=A0	rfid
=C2=A0	b	.	/* prevent speculative execution */

Re: [PATCH] powerpc/tm: do not use r13 for tabort_syscall

From: Michael Neuling <hidden>
Date: 2016-08-22 02:09:45

On Fri, 2016-07-22 at 17:27 +1000, Nicholas Piggin wrote:
tabort_syscall runs with RI=3D1, so a nested recoverable machine
check will load the paca into r13 and overwrite what we loaded
it with, because exceptions returning to privileged mode do not
restore r13.
=20
This has survived testing with sc instruction inside transaction
(bare sc, not glibc syscall because glibc can tabort before sc).
Verified the transaction is failing failing with with
TM_CAUSE_SYSCALL.
=20
Signed-off-by: Nick Piggin <npiggin@gmail.com>
Cc: Michael Neuling <redacted>
FWIW

Acked-by: Michael Neuling <redacted>
quoted hunk
Cc: Sam Bobroff <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
=20
---
=20
=C2=A0arch/powerpc/kernel/entry_64.S | 20 ++++++++++----------
=C2=A01 file changed, 10 insertions(+), 10 deletions(-)
=20
diff --git a/arch/powerpc/kernel/entry_64.S
b/arch/powerpc/kernel/entry_64.S
index 73e461a..387dee3 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -368,13 +368,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
=C2=A0tabort_syscall:
=C2=A0	/* Firstly we need to enable TM in the kernel */
=C2=A0	mfmsr	r10
-	li	r13, 1
-	rldimi	r10, r13, MSR_TM_LG, 63-MSR_TM_LG
-	mtmsrd	r10, 0
+	li	r9,1
+	rldimi	r10,r9,MSR_TM_LG,63-MSR_TM_LG
+	mtmsrd	r10,0
=C2=A0
=C2=A0	/* tabort, this dooms the transaction, nothing else */
-	li	r13, (TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT)
-	TABORT(R13)
+	li	r9,(TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT)
+	TABORT(R9)
=C2=A0
=C2=A0	/*
=C2=A0	=C2=A0* Return directly to userspace. We have corrupted user regis=
ter
quoted hunk
state,
@@ -382,11 +382,11 @@ tabort_syscall:
=C2=A0	=C2=A0* resume after the tbegin of the aborted transaction with th=
e
=C2=A0	=C2=A0* checkpointed register state.
=C2=A0	=C2=A0*/
-	li	r13, MSR_RI
-	andc	r10, r10, r13
-	mtmsrd	r10, 1
-	mtspr	SPRN_SRR0, r11
-	mtspr	SPRN_SRR1, r12
+	li	r9,MSR_RI
+	andc	r10,r10,r9
+	mtmsrd	r10,1
+	mtspr	SPRN_SRR0,r11
+	mtspr	SPRN_SRR1,r12
=C2=A0
=C2=A0	rfid
=C2=A0	b	.	/* prevent speculative execution */
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help