From: Russell Currey <hidden> Date: 2019-02-06 06:39:07
Without restoring the IAMR after idle, execution prevention on POWER9
with Radix MMU is overwritten and the kernel can freely execute userspace without
faulting.
This is necessary when returning from any stop state that modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
---
arch/powerpc/include/asm/cpuidle.h | 1 +
arch/powerpc/kernel/asm-offsets.c | 1 +
arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
3 files changed, 22 insertions(+)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2019-02-07 04:31:07
Russell Currey [off-list ref] writes:
Without restoring the IAMR after idle, execution prevention on POWER9
with Radix MMU is overwritten and the kernel can freely execute userspace without
faulting.
This is necessary when returning from any stop state that modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
space")
We have space for a full pt_regs on the stack, and we're not using it
all.
We don't have a specific slot for the IAMR (we may want to in future),
but for now you could follow the time-honoured tradition of (ab)using
the _DAR slot, with an appropriate comment.
cheers
From: Nicholas Piggin <npiggin@gmail.com> Date: 2019-02-07 05:10:30
Russell Currey's on February 6, 2019 4:28 pm:
Without restoring the IAMR after idle, execution prevention on POWER9
with Radix MMU is overwritten and the kernel can freely execute userspace without
faulting.
This is necessary when returning from any stop state that modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
Good catch and debugging. This really should be a quirk, we don't want
to have to restore this thing on a thread switch.
Can we put it under a CONFIG option if we're not using IAMR?
Sigh, good old isync. Suspect you'll get away without it, mtmsrd L=0
just below is architecturally guaranteeing a CSI, so just add a comment
there, might save a flush.
We have space for a full pt_regs on the stack, and we're not using it
all.
We don't have a specific slot for the IAMR (we may want to in
future),
but for now you could follow the time-honoured tradition of (ab)using
the _DAR slot, with an appropriate comment.
I read this, then did it, and when writing the comment I thought I was
clever using "(ab)use". I then reread this and realised I just
subconsciously stole it.
Thanks for the review.
From: Russell Currey <hidden> Date: 2019-02-07 06:35:16
On Thu, 2019-02-07 at 15:08 +1000, Nicholas Piggin wrote:
Russell Currey's on February 6, 2019 4:28 pm:
quoted
Without restoring the IAMR after idle, execution prevention on
POWER9
with Radix MMU is overwritten and the kernel can freely execute
userspace without
faulting.
This is necessary when returning from any stop state that modifies
user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver
and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where
it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of
user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
Good catch and debugging. This really should be a quirk, we don't
want
to have to restore this thing on a thread switch.
Can we put it under a CONFIG option if we're not using IAMR?
I don't exactly know when we do or don't use the IAMR (since the only
thing I've used it for is radix). When wouldn't we care about
restoring it on hash?
Sigh, good old isync. Suspect you'll get away without it, mtmsrd L=0
just below is architecturally guaranteeing a CSI, so just add a
comment
there, might save a flush.
Makes sense, I wanted to be super safe with this, will drop the isync
and try to execute userspace nonstop and see if there are any (non)
failures.
On Thu, 2019-02-07 at 15:08 +1000, Nicholas Piggin wrote:
quoted
Russell Currey's on February 6, 2019 4:28 pm:
quoted
Without restoring the IAMR after idle, execution prevention on
POWER9
with Radix MMU is overwritten and the kernel can freely execute
userspace without
faulting.
This is necessary when returning from any stop state that modifies
user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver
and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where
it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of
user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
Good catch and debugging. This really should be a quirk, we don't
want
to have to restore this thing on a thread switch.
Can we put it under a CONFIG option if we're not using IAMR?
I don't exactly know when we do or don't use the IAMR (since the only
thing I've used it for is radix). When wouldn't we care about
restoring it on hash?
On hash it's used for memory protection keys (code is in
arch/powerpc/mm/pkeys.c). The kernel doesn't use protection keys, but
userspace apps may use it explicitly via specific syscalls
(pkey_alloc(), pkey_mprotect, pkey_free()).
Also, the kernel may use a protection key if the process does an
mmap(PROT_EXEC).
--
Thiago Jung Bauermann
IBM Linux Technology Center
From: Russell Currey <hidden> Date: 2019-02-07 22:40:11
On Thu, 2019-02-07 at 14:37 -0200, Thiago Jung Bauermann wrote:
Russell Currey [off-list ref] writes:
quoted
On Thu, 2019-02-07 at 15:08 +1000, Nicholas Piggin wrote:
quoted
Russell Currey's on February 6, 2019 4:28 pm:
quoted
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel
execution of
user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
Good catch and debugging. This really should be a quirk, we don't
want
to have to restore this thing on a thread switch.
Can we put it under a CONFIG option if we're not using IAMR?
I don't exactly know when we do or don't use the IAMR (since the
only
thing I've used it for is radix). When wouldn't we care about
restoring it on hash?
On hash it's used for memory protection keys (code is in
arch/powerpc/mm/pkeys.c). The kernel doesn't use protection keys, but
userspace apps may use it explicitly via specific syscalls
(pkey_alloc(), pkey_mprotect, pkey_free()).
Also, the kernel may use a protection key if the process does an
mmap(PROT_EXEC).
I don't understand how this would work, though - in this case we
wouldn't know on boot if we were going to use the IAMR or not. On
radix (unless booting with nosmep) it would always be used, but on hash
it seems it depends on what userspace does. How exactly would a
runtime toggle of "IAMR in use" work?
With a CONFIG option it would have to depend on PPC_MEM_KEYS ||
PPC_RADIX_MMU, but those are (pretty much) always going to be on in P8
and P9, which I already check for.
--
Thiago Jung Bauermann
IBM Linux Technology Center
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2019-02-08 01:06:50
Nicholas Piggin [off-list ref] writes:
Russell Currey's on February 6, 2019 4:28 pm:
quoted
Without restoring the IAMR after idle, execution prevention on POWER9
with Radix MMU is overwritten and the kernel can freely execute userspace without
faulting.
This is necessary when returning from any stop state that modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
Good catch and debugging. This really should be a quirk, we don't want
to have to restore this thing on a thread switch.
I'm not sure I follow. We don't context switch it on Radix, but we do
on hash if pkeys are enabled.
Can we put it under a CONFIG option if we're not using IAMR?
We'll always be using it with Radix, and we might be using it for pkeys
on hash, unless pkeys are compiled out. But I don't really expect anyone
to be running with pkeys compiled out.
So I think the only case we could optimise is that we're on hash and the
current thread has an IAMR of 0, then we could just not restore
(assuming we come out of idle with IAMR=0).
But maybe I'm not understanding.
cheers
From: Nicholas Piggin <npiggin@gmail.com> Date: 2019-02-19 04:23:39
Michael Ellerman's on February 8, 2019 11:04 am:
Nicholas Piggin [off-list ref] writes:
quoted
Russell Currey's on February 6, 2019 4:28 pm:
quoted
Without restoring the IAMR after idle, execution prevention on POWER9
with Radix MMU is overwritten and the kernel can freely execute userspace without
faulting.
This is necessary when returning from any stop state that modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
Good catch and debugging. This really should be a quirk, we don't want
to have to restore this thing on a thread switch.
I'm not sure I follow. We don't context switch it on Radix, but we do
on hash if pkeys are enabled.
Badly worded, I mean a hardware quirk. It should follow thread
switches. Still, avoiding it for the no-loss case is better than
nothing. We can just revisit it as an optimization if future
hardware does not require the restore.
quoted
Can we put it under a CONFIG option if we're not using IAMR?
We'll always be using it with Radix, and we might be using it for pkeys
on hash, unless pkeys are compiled out. But I don't really expect anyone
to be running with pkeys compiled out.
So I think the only case we could optimise is that we're on hash and the
current thread has an IAMR of 0, then we could just not restore
(assuming we come out of idle with IAMR=0).
But maybe I'm not understanding.
Nah it sounds like more trouble than it's worth in that case.
Thanks,
Nick
On Tue, Feb 19, 2019 at 02:21:04PM +1000, Nicholas Piggin wrote:
Michael Ellerman's on February 8, 2019 11:04 am:
quoted
Nicholas Piggin [off-list ref] writes:
quoted
Russell Currey's on February 6, 2019 4:28 pm:
quoted
Without restoring the IAMR after idle, execution prevention on POWER9
with Radix MMU is overwritten and the kernel can freely execute userspace without
faulting.
This is necessary when returning from any stop state that modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
Good catch and debugging. This really should be a quirk, we don't want
to have to restore this thing on a thread switch.
I'm not sure I follow. We don't context switch it on Radix, but we do
on hash if pkeys are enabled.
Badly worded, I mean a hardware quirk. It should follow thread
switches. Still, avoiding it for the no-loss case is better than
nothing. We can just revisit it as an optimization if future
hardware does not require the restore.
Apparently, the POWER9 Processor User’s Manual v2.0 documents that
IAMR can be lost, and that is not just the end.
Pasting excerpt from "Section 23.5.9.2 State Loss and Restoration,Page 309"
On the POWER9 core, the only state that can be lost for
Stop levels less than four, when PSSCR[ESL] = ‘1’ are the
following SPRs: CR, FPSCR, VSCR, XER, DSCR, AMR, IAMR, UAMOR,
AMOR, DAWR, DAWRX.
My observation is that AMOR is being used in kernel as of today
and AMOR is also lost (recreated in similar scenarios where
IAMR is lost).
On Wed, Feb 06, 2019 at 05:28:37PM +1100, Russell Currey wrote:
quoted hunk
Without restoring the IAMR after idle, execution prevention on POWER9
with Radix MMU is overwritten and the kernel can freely execute userspace without
faulting.
This is necessary when returning from any stop state that modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
---
arch/powerpc/include/asm/cpuidle.h | 1 +
arch/powerpc/kernel/asm-offsets.c | 1 +
arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
3 files changed, 22 insertions(+)
On Wed, Feb 06, 2019 at 05:28:37PM +1100, Russell Currey wrote:
quoted hunk
Without restoring the IAMR after idle, execution prevention on POWER9
with Radix MMU is overwritten and the kernel can freely execute userspace without
faulting.
This is necessary when returning from any stop state that modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
---
arch/powerpc/include/asm/cpuidle.h | 1 +
arch/powerpc/kernel/asm-offsets.c | 1 +
arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
3 files changed, 22 insertions(+)
pnv_wakeup_noloss gets called from two paths:
1) cpu wakes up at 0x100 (ESL=EC=1)
2) cpu wakes up at next instruction (ESL=EC=0)
We know for the fact that its not lost with ESL=EC=0 , so we
should put it somewhere else.
I would like to put the restore code in pnv_restore_hyp_resource_arch300
it already has some work arounds we can add another.
By this time we cr3 has comparison of SRR1[46:47] with 2
542 pnv_restore_hyp_resource_arch300:
543 /*
544 * Workaround for POWER9, if we lost resources, the ERAT
545 * might have been mixed up and needs flushing. We also need
546 * to reload MMCR0 (see comment above). We also need to set
547 * then clear bit 60 in MMCRA to ensure the PMU starts running.
548 */
549 blt cr3,1f
550 BEGIN_FTR_SECTION
551 PPC_INVALIDATE_ERAT
552 ld r1,PACAR1(r13)
553 ld r4,_MMCR0(r1)
554 mtspr SPRN_MMCR0,r4
555 END_FTR_SECTION_IFCLR(CPU_FTR_POWER9_DD2_1)
556 mfspr r4,SPRN_MMCRA
557 ori r4,r4,(1 << (63-60))
558 mtspr SPRN_MMCRA,r4
559 xori r4,r4,(1 << (63-60))
560 mtspr SPRN_MMCRA,r4
+ 561 BEGIN_FTR_SECTION
+ 562 ld r4,STOP_IAMR(r13)
+ 563 mtspr SPRN_IAMR,r4
+ 564 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
565 1:
566 /*
I have a patchset to handle both AMOR and IAMR,
need to test it on power8 before posting.
From: Russell Currey <hidden> Date: 2019-02-20 11:23:19
On Wed, 2019-02-20 at 11:34 +0530, Akshay Adiga wrote:
On Tue, Feb 19, 2019 at 02:21:04PM +1000, Nicholas Piggin wrote:
quoted
Michael Ellerman's on February 8, 2019 11:04 am:
quoted
Nicholas Piggin [off-list ref] writes:
quoted
Russell Currey's on February 6, 2019 4:28 pm:
quoted
Without restoring the IAMR after idle, execution prevention
on POWER9
with Radix MMU is overwritten and the kernel can freely
execute userspace without
faulting.
This is necessary when returning from any stop state that
modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm
driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-
crash/DIRECT
which won't fault, then boot the kernel with powersave=off,
where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel
execution of user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
Good catch and debugging. This really should be a quirk, we
don't want
to have to restore this thing on a thread switch.
I'm not sure I follow. We don't context switch it on Radix, but
we do
on hash if pkeys are enabled.
Badly worded, I mean a hardware quirk. It should follow thread
switches. Still, avoiding it for the no-loss case is better than
nothing. We can just revisit it as an optimization if future
hardware does not require the restore.
Apparently, the POWER9 Processor User’s Manual v2.0 documents that
IAMR can be lost, and that is not just the end.
Pasting excerpt from "Section 23.5.9.2 State Loss and
Restoration,Page 309"
On the POWER9 core, the only state that can be lost for
Stop levels less than four, when PSSCR[ESL] = ‘1’ are the
following SPRs: CR, FPSCR, VSCR, XER, DSCR, AMR, IAMR, UAMOR,
AMOR, DAWR, DAWRX.
My observation is that AMOR is being used in kernel as of today
and AMOR is also lost (recreated in similar scenarios where
IAMR is lost).
I can add AMOR to this patch (or you can send a patch, either way).
From: Russell Currey <hidden> Date: 2019-02-20 11:25:21
On Wed, 2019-02-20 at 14:28 +0530, Akshay Adiga wrote:
On Wed, Feb 06, 2019 at 05:28:37PM +1100, Russell Currey wrote:
quoted
Without restoring the IAMR after idle, execution prevention on
POWER9
with Radix MMU is overwritten and the kernel can freely execute
userspace without
faulting.
This is necessary when returning from any stop state that modifies
user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver
and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where
it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of
user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
---
arch/powerpc/include/asm/cpuidle.h | 1 +
arch/powerpc/kernel/asm-offsets.c | 1 +
arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
3 files changed, 22 insertions(+)
diff --git a/arch/powerpc/include/asm/cpuidle.h
b/arch/powerpc/include/asm/cpuidle.h
index 43e5f31fe64d..ad67dbe59498 100644
pnv_wakeup_noloss gets called from two paths:
1) cpu wakes up at 0x100 (ESL=EC=1)
2) cpu wakes up at next instruction (ESL=EC=0)
In v2 I drop it from noloss, is that still correct?
We know for the fact that its not lost with ESL=EC=0 , so we
should put it somewhere else.
I would like to put the restore code in
pnv_restore_hyp_resource_arch300
it already has some work arounds we can add another.
By this time we cr3 has comparison of SRR1[46:47] with 2
542 pnv_restore_hyp_resource_arch300:
543 /*
544 * Workaround for POWER9, if we lost resources, the
ERAT
545 * might have been mixed up and needs flushing. We also
need
546 * to reload MMCR0 (see comment above). We also need to
set
547 * then clear bit 60 in MMCRA to ensure the PMU starts
running.
548 */
549 blt cr3,1f
550 BEGIN_FTR_SECTION
551 PPC_INVALIDATE_ERAT
552 ld r1,PACAR1(r13)
553 ld r4,_MMCR0(r1)
554 mtspr SPRN_MMCR0,r4
555 END_FTR_SECTION_IFCLR(CPU_FTR_POWER9_DD2_1)
556 mfspr r4,SPRN_MMCRA
557 ori r4,r4,(1 << (63-60))
558 mtspr SPRN_MMCRA,r4
559 xori r4,r4,(1 << (63-60))
560 mtspr SPRN_MMCRA,r4
+ 561 BEGIN_FTR_SECTION
+ 562 ld r4,STOP_IAMR(r13)
+ 563 mtspr SPRN_IAMR,r4
+ 564 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
565 1:
566 /*
I have a patchset to handle both AMOR and IAMR,
need to test it on power8 before posting.
From: Russell Currey <hidden> Date: 2019-02-20 11:28:12
On Wed, 2019-02-20 at 12:45 +0530, Akshay Adiga wrote:
On Wed, Feb 06, 2019 at 05:28:37PM +1100, Russell Currey wrote:
quoted
Without restoring the IAMR after idle, execution prevention on
POWER9
with Radix MMU is overwritten and the kernel can freely execute
userspace without
faulting.
This is necessary when returning from any stop state that modifies
user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver
and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where
it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of
user
space")
Cc: <redacted>
Signed-off-by: Russell Currey <redacted>
---
arch/powerpc/include/asm/cpuidle.h | 1 +
arch/powerpc/kernel/asm-offsets.c | 1 +
arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
3 files changed, 22 insertions(+)
diff --git a/arch/powerpc/include/asm/cpuidle.h
b/arch/powerpc/include/asm/cpuidle.h
index 43e5f31fe64d..ad67dbe59498 100644
Are we trying to add for both power8 and power9 ?
power9 would be CPU_FTR_ARCH_300.
If I recall correctly I had this at P9 only but for some reason I
changed it - the reason is probably just that I had it confused for
something else. Michael can you confirm this should be P9 only?