* [PATCH] powerpc/tm: do not use r13 for tabort_syscall
@ 2016-07-22 7:27 Nicholas Piggin
2016-07-25 0:57 ` Michael Neuling
2016-08-22 2:09 ` Michael Neuling
0 siblings, 2 replies; 4+ messages in thread
From: Nicholas Piggin @ 2016-07-22 7:27 UTC (permalink / raw)
To: linuxppc-dev
Cc: Nicholas Piggin, Michael Neuling, Sam Bobroff, Michael Ellerman
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 <mikey@neuling.org>
Cc: Sam Bobroff <sam.bobroff@au1.ibm.com>
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/tm: do not use r13 for tabort_syscall
2016-07-22 7:27 [PATCH] powerpc/tm: do not use r13 for tabort_syscall Nicholas Piggin
@ 2016-07-25 0:57 ` Michael Neuling
2016-08-22 2:09 ` Michael Neuling
1 sibling, 0 replies; 4+ messages in thread
From: Michael Neuling @ 2016-07-25 0:57 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Sam Bobroff, Michael Ellerman
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
> Cc: Michael Neuling <mikey@neuling.org>
> Cc: Sam Bobroff <sam.bobroff@au1.ibm.com>
> 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
> 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 */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/tm: do not use r13 for tabort_syscall
2016-07-22 7:27 [PATCH] powerpc/tm: do not use r13 for tabort_syscall Nicholas Piggin
2016-07-25 0:57 ` Michael Neuling
@ 2016-08-22 2:09 ` Michael Neuling
1 sibling, 0 replies; 4+ messages in thread
From: Michael Neuling @ 2016-08-22 2:09 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Sam Bobroff, Michael Ellerman
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 <mikey@neuling.org>
FWIW
Acked-by: Michael Neuling <mikey@neuling.org>
> Cc: Sam Bobroff <sam.bobroff@au1.ibm.com>
> 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
> 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 */
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] powerpc/tm: do not use r13 for tabort_syscall
@ 2016-07-25 4:26 Nicholas Piggin
0 siblings, 0 replies; 4+ messages in thread
From: Nicholas Piggin @ 2016-07-25 4:26 UTC (permalink / raw)
To: linuxppc-dev
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.
Fixes: b4b56f9ecab4 (powerpc/tm: Abort syscalls in active transactions)
Cc: stable@vger.kernel.org
Signed-off-by: Nick Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/entry_64.S | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 73e461a..96fd031 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
+ 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,8 +382,8 @@ tabort_syscall:
* resume after the tbegin of the aborted transaction with the
* checkpointed register state.
*/
- li r13, MSR_RI
- andc r10, r10, r13
+ li r9, MSR_RI
+ andc r10, r10, r9
mtmsrd r10, 1
mtspr SPRN_SRR0, r11
mtspr SPRN_SRR1, r12
--
2.8.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-22 2:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-22 7:27 [PATCH] powerpc/tm: do not use r13 for tabort_syscall Nicholas Piggin
2016-07-25 0:57 ` Michael Neuling
2016-08-22 2:09 ` Michael Neuling
-- strict thread matches above, loose matches on Subject: below --
2016-07-25 4:26 Nicholas Piggin
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).