linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc/64s: Make rfi_flush_fallback a little more robust
@ 2018-07-26 12:42 Michael Ellerman
  2018-08-01  2:01 ` Nicholas Piggin
  2018-08-08 14:26 ` Michael Ellerman
  0 siblings, 2 replies; 3+ messages in thread
From: Michael Ellerman @ 2018-07-26 12:42 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: npiggin, anton

Because rfi_flush_fallback runs immediately before the return to
userspace it currently runs with the user r1 (stack pointer). This
means if we oops in there we will report a bad kernel stack pointer in
the exception entry path, eg:

  Bad kernel stack pointer 7ffff7150e40 at c0000000000023b4
  Oops: Bad kernel stack pointer, sig: 6 [#1]
  LE SMP NR_CPUS=32 NUMA PowerNV
  Modules linked in:
  CPU: 0 PID: 1246 Comm: klogd Not tainted 4.18.0-rc2-gcc-7.3.1-00175-g0443f8a69ba3 #7
  NIP:  c0000000000023b4 LR: 0000000010053e00 CTR: 0000000000000040
  REGS: c0000000fffe7d40 TRAP: 4100   Not tainted  (4.18.0-rc2-gcc-7.3.1-00175-g0443f8a69ba3)
  MSR:  9000000002803031 <SF,HV,VEC,VSX,FP,ME,IR,DR,LE>  CR: 44000442  XER: 20000000
  CFAR: c00000000000bac8 IRQMASK: c0000000f1e66a80
  GPR00: 0000000002000000 00007ffff7150e40 00007fff93a99900 0000000000000020
  ...
  NIP [c0000000000023b4] rfi_flush_fallback+0x34/0x80
  LR [0000000010053e00] 0x10053e00

Although the NIP tells us where we were, and the TRAP number tells us
what happened, it would still be nicer if we could report the actual
exception rather than barfing about the stack pointer.

We an do that fairly simply by loading the kernel stack pointer on
entry and restoring the user value before returning. That way we see a
regular oops such as:

  Unrecoverable exception 4100 at c00000000000239c
  Oops: Unrecoverable exception, sig: 6 [#1]
  LE SMP NR_CPUS=32 NUMA PowerNV
  Modules linked in:
  CPU: 0 PID: 1251 Comm: klogd Not tainted 4.18.0-rc3-gcc-7.3.1-00097-g4ebfcac65acd-dirty #40
  NIP:  c00000000000239c LR: 0000000010053e00 CTR: 0000000000000040
  REGS: c0000000f1e17bb0 TRAP: 4100   Not tainted  (4.18.0-rc3-gcc-7.3.1-00097-g4ebfcac65acd-dirty)
  MSR:  9000000002803031 <SF,HV,VEC,VSX,FP,ME,IR,DR,LE>  CR: 44000442  XER: 20000000
  CFAR: c00000000000bac8 IRQMASK: 0
  ...
  NIP [c00000000000239c] rfi_flush_fallback+0x3c/0x80
  LR [0000000010053e00] 0x10053e00
  Call Trace:
  [c0000000f1e17e30] [c00000000000b9e4] system_call+0x5c/0x70 (unreliable)

Note this shouldn't make the kernel stack pointer vulnerable to a
meltdown attack, because it should be flushed from the cache before we
return to userspace. The user r1 value will be in the cache, because
we load it in the return path, but that is harmless.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/exceptions-64s.S | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index f7cc12aa3dc6..a6fa85916273 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1518,6 +1518,8 @@ TRAMP_REAL_BEGIN(stf_barrier_fallback)
 TRAMP_REAL_BEGIN(rfi_flush_fallback)
 	SET_SCRATCH0(r13);
 	GET_PACA(r13);
+	std	r1,PACA_EXRFI+EX_R12(r13)
+	ld	r1,PACAKSAVE(r13)
 	std	r9,PACA_EXRFI+EX_R9(r13)
 	std	r10,PACA_EXRFI+EX_R10(r13)
 	std	r11,PACA_EXRFI+EX_R11(r13)
@@ -1552,12 +1554,15 @@ TRAMP_REAL_BEGIN(rfi_flush_fallback)
 	ld	r9,PACA_EXRFI+EX_R9(r13)
 	ld	r10,PACA_EXRFI+EX_R10(r13)
 	ld	r11,PACA_EXRFI+EX_R11(r13)
+	ld	r1,PACA_EXRFI+EX_R12(r13)
 	GET_SCRATCH0(r13);
 	rfid
 
 TRAMP_REAL_BEGIN(hrfi_flush_fallback)
 	SET_SCRATCH0(r13);
 	GET_PACA(r13);
+	std	r1,PACA_EXRFI+EX_R12(r13)
+	ld	r1,PACAKSAVE(r13)
 	std	r9,PACA_EXRFI+EX_R9(r13)
 	std	r10,PACA_EXRFI+EX_R10(r13)
 	std	r11,PACA_EXRFI+EX_R11(r13)
@@ -1592,6 +1597,7 @@ TRAMP_REAL_BEGIN(hrfi_flush_fallback)
 	ld	r9,PACA_EXRFI+EX_R9(r13)
 	ld	r10,PACA_EXRFI+EX_R10(r13)
 	ld	r11,PACA_EXRFI+EX_R11(r13)
+	ld	r1,PACA_EXRFI+EX_R12(r13)
 	GET_SCRATCH0(r13);
 	hrfid
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] powerpc/64s: Make rfi_flush_fallback a little more robust
  2018-07-26 12:42 [PATCH] powerpc/64s: Make rfi_flush_fallback a little more robust Michael Ellerman
@ 2018-08-01  2:01 ` Nicholas Piggin
  2018-08-08 14:26 ` Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Nicholas Piggin @ 2018-08-01  2:01 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, anton

On Thu, 26 Jul 2018 22:42:44 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Because rfi_flush_fallback runs immediately before the return to
> userspace it currently runs with the user r1 (stack pointer). This
> means if we oops in there we will report a bad kernel stack pointer in
> the exception entry path, eg:
> 
>   Bad kernel stack pointer 7ffff7150e40 at c0000000000023b4
>   Oops: Bad kernel stack pointer, sig: 6 [#1]
>   LE SMP NR_CPUS=32 NUMA PowerNV
>   Modules linked in:
>   CPU: 0 PID: 1246 Comm: klogd Not tainted 4.18.0-rc2-gcc-7.3.1-00175-g0443f8a69ba3 #7
>   NIP:  c0000000000023b4 LR: 0000000010053e00 CTR: 0000000000000040
>   REGS: c0000000fffe7d40 TRAP: 4100   Not tainted  (4.18.0-rc2-gcc-7.3.1-00175-g0443f8a69ba3)
>   MSR:  9000000002803031 <SF,HV,VEC,VSX,FP,ME,IR,DR,LE>  CR: 44000442  XER: 20000000
>   CFAR: c00000000000bac8 IRQMASK: c0000000f1e66a80
>   GPR00: 0000000002000000 00007ffff7150e40 00007fff93a99900 0000000000000020
>   ...
>   NIP [c0000000000023b4] rfi_flush_fallback+0x34/0x80
>   LR [0000000010053e00] 0x10053e00
> 
> Although the NIP tells us where we were, and the TRAP number tells us
> what happened, it would still be nicer if we could report the actual
> exception rather than barfing about the stack pointer.
> 
> We an do that fairly simply by loading the kernel stack pointer on
> entry and restoring the user value before returning. That way we see a
> regular oops such as:
> 
>   Unrecoverable exception 4100 at c00000000000239c
>   Oops: Unrecoverable exception, sig: 6 [#1]
>   LE SMP NR_CPUS=32 NUMA PowerNV
>   Modules linked in:
>   CPU: 0 PID: 1251 Comm: klogd Not tainted 4.18.0-rc3-gcc-7.3.1-00097-g4ebfcac65acd-dirty #40
>   NIP:  c00000000000239c LR: 0000000010053e00 CTR: 0000000000000040
>   REGS: c0000000f1e17bb0 TRAP: 4100   Not tainted  (4.18.0-rc3-gcc-7.3.1-00097-g4ebfcac65acd-dirty)
>   MSR:  9000000002803031 <SF,HV,VEC,VSX,FP,ME,IR,DR,LE>  CR: 44000442  XER: 20000000
>   CFAR: c00000000000bac8 IRQMASK: 0
>   ...
>   NIP [c00000000000239c] rfi_flush_fallback+0x3c/0x80
>   LR [0000000010053e00] 0x10053e00
>   Call Trace:
>   [c0000000f1e17e30] [c00000000000b9e4] system_call+0x5c/0x70 (unreliable)
> 
> Note this shouldn't make the kernel stack pointer vulnerable to a
> meltdown attack, because it should be flushed from the cache before we
> return to userspace. The user r1 value will be in the cache, because
> we load it in the return path, but that is harmless.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Yeah that's a lot nicer, thanks.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  arch/powerpc/kernel/exceptions-64s.S | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index f7cc12aa3dc6..a6fa85916273 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -1518,6 +1518,8 @@ TRAMP_REAL_BEGIN(stf_barrier_fallback)
>  TRAMP_REAL_BEGIN(rfi_flush_fallback)
>  	SET_SCRATCH0(r13);
>  	GET_PACA(r13);
> +	std	r1,PACA_EXRFI+EX_R12(r13)
> +	ld	r1,PACAKSAVE(r13)
>  	std	r9,PACA_EXRFI+EX_R9(r13)
>  	std	r10,PACA_EXRFI+EX_R10(r13)
>  	std	r11,PACA_EXRFI+EX_R11(r13)
> @@ -1552,12 +1554,15 @@ TRAMP_REAL_BEGIN(rfi_flush_fallback)
>  	ld	r9,PACA_EXRFI+EX_R9(r13)
>  	ld	r10,PACA_EXRFI+EX_R10(r13)
>  	ld	r11,PACA_EXRFI+EX_R11(r13)
> +	ld	r1,PACA_EXRFI+EX_R12(r13)
>  	GET_SCRATCH0(r13);
>  	rfid
>  
>  TRAMP_REAL_BEGIN(hrfi_flush_fallback)
>  	SET_SCRATCH0(r13);
>  	GET_PACA(r13);
> +	std	r1,PACA_EXRFI+EX_R12(r13)
> +	ld	r1,PACAKSAVE(r13)
>  	std	r9,PACA_EXRFI+EX_R9(r13)
>  	std	r10,PACA_EXRFI+EX_R10(r13)
>  	std	r11,PACA_EXRFI+EX_R11(r13)
> @@ -1592,6 +1597,7 @@ TRAMP_REAL_BEGIN(hrfi_flush_fallback)
>  	ld	r9,PACA_EXRFI+EX_R9(r13)
>  	ld	r10,PACA_EXRFI+EX_R10(r13)
>  	ld	r11,PACA_EXRFI+EX_R11(r13)
> +	ld	r1,PACA_EXRFI+EX_R12(r13)
>  	GET_SCRATCH0(r13);
>  	hrfid
>  

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: powerpc/64s: Make rfi_flush_fallback a little more robust
  2018-07-26 12:42 [PATCH] powerpc/64s: Make rfi_flush_fallback a little more robust Michael Ellerman
  2018-08-01  2:01 ` Nicholas Piggin
@ 2018-08-08 14:26 ` Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2018-08-08 14:26 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: anton, npiggin

On Thu, 2018-07-26 at 12:42:44 UTC, Michael Ellerman wrote:
> Because rfi_flush_fallback runs immediately before the return to
> userspace it currently runs with the user r1 (stack pointer). This
> means if we oops in there we will report a bad kernel stack pointer in
> the exception entry path, eg:
> 
>   Bad kernel stack pointer 7ffff7150e40 at c0000000000023b4
>   Oops: Bad kernel stack pointer, sig: 6 [#1]
>   LE SMP NR_CPUS=32 NUMA PowerNV
>   Modules linked in:
>   CPU: 0 PID: 1246 Comm: klogd Not tainted 4.18.0-rc2-gcc-7.3.1-00175-g0443f8a69ba3 #7
>   NIP:  c0000000000023b4 LR: 0000000010053e00 CTR: 0000000000000040
>   REGS: c0000000fffe7d40 TRAP: 4100   Not tainted  (4.18.0-rc2-gcc-7.3.1-00175-g0443f8a69ba3)
>   MSR:  9000000002803031 <SF,HV,VEC,VSX,FP,ME,IR,DR,LE>  CR: 44000442  XER: 20000000
>   CFAR: c00000000000bac8 IRQMASK: c0000000f1e66a80
>   GPR00: 0000000002000000 00007ffff7150e40 00007fff93a99900 0000000000000020
>   ...
>   NIP [c0000000000023b4] rfi_flush_fallback+0x34/0x80
>   LR [0000000010053e00] 0x10053e00
> 
> Although the NIP tells us where we were, and the TRAP number tells us
> what happened, it would still be nicer if we could report the actual
> exception rather than barfing about the stack pointer.
> 
> We an do that fairly simply by loading the kernel stack pointer on
> entry and restoring the user value before returning. That way we see a
> regular oops such as:
> 
>   Unrecoverable exception 4100 at c00000000000239c
>   Oops: Unrecoverable exception, sig: 6 [#1]
>   LE SMP NR_CPUS=32 NUMA PowerNV
>   Modules linked in:
>   CPU: 0 PID: 1251 Comm: klogd Not tainted 4.18.0-rc3-gcc-7.3.1-00097-g4ebfcac65acd-dirty #40
>   NIP:  c00000000000239c LR: 0000000010053e00 CTR: 0000000000000040
>   REGS: c0000000f1e17bb0 TRAP: 4100   Not tainted  (4.18.0-rc3-gcc-7.3.1-00097-g4ebfcac65acd-dirty)
>   MSR:  9000000002803031 <SF,HV,VEC,VSX,FP,ME,IR,DR,LE>  CR: 44000442  XER: 20000000
>   CFAR: c00000000000bac8 IRQMASK: 0
>   ...
>   NIP [c00000000000239c] rfi_flush_fallback+0x3c/0x80
>   LR [0000000010053e00] 0x10053e00
>   Call Trace:
>   [c0000000f1e17e30] [c00000000000b9e4] system_call+0x5c/0x70 (unreliable)
> 
> Note this shouldn't make the kernel stack pointer vulnerable to a
> meltdown attack, because it should be flushed from the cache before we
> return to userspace. The user r1 value will be in the cache, because
> we load it in the return path, but that is harmless.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next.

https://git.kernel.org/powerpc/c/78ee9946371f5848ddfc88ab1a4386

cheers

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-08-08 14:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-26 12:42 [PATCH] powerpc/64s: Make rfi_flush_fallback a little more robust Michael Ellerman
2018-08-01  2:01 ` Nicholas Piggin
2018-08-08 14:26 ` 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).