[PATCH 1/3] powerpc/64s: Mask NIP before checking against SRR0

[PATCH 1/3] powerpc/64s: Mask NIP before checking against SRR0

[Michael Ellerman]

When CONFIG_PPC_RFI_SRR_DEBUG=y we check that NIP and SRR0 match when
returning from interrupts. This can trigger falsely if NIP has either of
its two low bits set via sigreturn or ptrace, while SRR0 has its low two
bits masked in hardware.

As a quick fix make sure to mask the low bits before doing the check.

Fixes: 59dc5bfca0cb ("powerpc/64s: avoid reloading (H)SRR registers if they are still valid")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/interrupt_64.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 2ad223597ca2..4fd65d39d5d3 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -30,6 +30,7 @@
 	.ifc \srr,srr
 	mfspr	r11,SPRN_SRR0
 	ld	r12,_NIP(r1)
+	clrrdi  r12,r12,2
 100:	tdne	r11,r12
 	EMIT_BUG_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
 	mfspr	r11,SPRN_SRR1
@@ -39,6 +40,7 @@
 	.else
 	mfspr	r11,SPRN_HSRR0
 	ld	r12,_NIP(r1)
+	clrrdi  r12,r12,2
 100:	tdne	r11,r12
 	EMIT_BUG_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
 	mfspr	r11,SPRN_HSRR1
-- 
2.31.1

[PATCH 2/3] powerpc/64s: Use EMIT_WARN_ENTRY for SRR debug warnings

[Michael Ellerman]

When CONFIG_PPC_RFI_SRR_DEBUG=y we check the SRR values before returning
from interrupts. This is done in asm using EMIT_BUG_ENTRY, and passing
BUGFLAG_WARNING.

However that fails to create an exception table entry for the warning,
and so do_program_check() fails the exception table search and proceeds
to call _exception(), resulting in an oops like:

  Oops: Exception in kernel mode, sig: 5 [#1]
  LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
  Modules linked in:
  CPU: 2 PID: 1204 Comm: sigreturn_unali Tainted: P                  5.16.0-rc2-00194-g91ca3d4f77c5 #12
  NIP:  c00000000000c5b0 LR: 0000000000000000 CTR: 0000000000000000
  ...
  NIP [c00000000000c5b0] system_call_common+0x150/0x268
  LR [0000000000000000] 0x0
  Call Trace:
  [c00000000db73e10] [c00000000000c558] system_call_common+0xf8/0x268 (unreliable)
  ...
  Instruction dump:
  7cc803a6 888d0931 2c240000 4082001c 38800000 988d0931 e8810170 e8a10178
  7c9a03a6 7cbb03a6 7d7a02a6 e9810170 <7f0b6088> 7d7b02a6 e9810178 7f0b6088

We should instead use EMIT_WARN_ENTRY, which creates an exception table
entry for the warning, allowing the warning to be correctly recognised,
and the code to resume after printing the warning.

Note however that because this warning is buried deep in the interrupt
return path, we are not able to recover from it (due to MSR_RI being
clear), so we still end up in die() with an unrecoverable exception.

Fixes: 59dc5bfca0cb ("powerpc/64s: avoid reloading (H)SRR registers if they are still valid")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/interrupt_64.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 4fd65d39d5d3..be5ff1521505 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -32,21 +32,21 @@
 	ld	r12,_NIP(r1)
 	clrrdi  r12,r12,2
 100:	tdne	r11,r12
-	EMIT_BUG_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
+	EMIT_WARN_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
 	mfspr	r11,SPRN_SRR1
 	ld	r12,_MSR(r1)
 100:	tdne	r11,r12
-	EMIT_BUG_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
+	EMIT_WARN_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
 	.else
 	mfspr	r11,SPRN_HSRR0
 	ld	r12,_NIP(r1)
 	clrrdi  r12,r12,2
 100:	tdne	r11,r12
-	EMIT_BUG_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
+	EMIT_WARN_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
 	mfspr	r11,SPRN_HSRR1
 	ld	r12,_MSR(r1)
 100:	tdne	r11,r12
-	EMIT_BUG_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
+	EMIT_WARN_ENTRY 100b,__FILE__,__LINE__,(BUGFLAG_WARNING | BUGFLAG_ONCE)
 	.endif
 #endif
 .endm
-- 
2.31.1

[PATCH 3/3] selftests/powerpc: Add a test of sigreturning to an unaligned address

[Michael Ellerman]

Add a test of sigreturning to an unaligned address (low two bits set).
This should have no effect because the hardware will mask those bits.
However it previously falsely triggered a warning when
CONFIG_PPC_RFI_SRR_DEBUG=y.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 .../selftests/powerpc/signal/.gitignore       |  1 +
 .../testing/selftests/powerpc/signal/Makefile |  1 +
 .../powerpc/signal/sigreturn_unaligned.c      | 43 +++++++++++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 tools/testing/selftests/powerpc/signal/sigreturn_unaligned.c

diff --git a/tools/testing/selftests/powerpc/signal/.gitignore b/tools/testing/selftests/powerpc/signal/.gitignore
index 8f6c816099a4..9d0915777fed 100644
--- a/tools/testing/selftests/powerpc/signal/.gitignore
+++ b/tools/testing/selftests/powerpc/signal/.gitignore
@@ -5,3 +5,4 @@ sigfuz
 sigreturn_vdso
 sig_sc_double_restart
 sigreturn_kernel
+sigreturn_unaligned
diff --git a/tools/testing/selftests/powerpc/signal/Makefile b/tools/testing/selftests/powerpc/signal/Makefile
index 84e201572466..f679d260afc8 100644
--- a/tools/testing/selftests/powerpc/signal/Makefile
+++ b/tools/testing/selftests/powerpc/signal/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 TEST_GEN_PROGS := signal signal_tm sigfuz sigreturn_vdso sig_sc_double_restart
 TEST_GEN_PROGS += sigreturn_kernel
+TEST_GEN_PROGS += sigreturn_unaligned
 
 CFLAGS += -maltivec
 $(OUTPUT)/signal_tm: CFLAGS += -mhtm
diff --git a/tools/testing/selftests/powerpc/signal/sigreturn_unaligned.c b/tools/testing/selftests/powerpc/signal/sigreturn_unaligned.c
new file mode 100644
index 000000000000..6e58ee4f0fdf
--- /dev/null
+++ b/tools/testing/selftests/powerpc/signal/sigreturn_unaligned.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test sigreturn to an unaligned address, ie. low 2 bits set.
+ * Nothing bad should happen.
+ * This was able to trigger warnings with CONFIG_PPC_RFI_SRR_DEBUG=y.
+ */
+
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ucontext.h>
+#include <unistd.h>
+
+#include "utils.h"
+
+
+static void sigusr1_handler(int signo, siginfo_t *info, void *ptr)
+{
+	ucontext_t *uc = ptr;
+
+	UCONTEXT_NIA(uc) |= 3;
+}
+
+static int test_sigreturn_unaligned(void)
+{
+	struct sigaction action;
+
+	memset(&action, 0, sizeof(action));
+	action.sa_sigaction = sigusr1_handler;
+	action.sa_flags = SA_SIGINFO;
+
+	FAIL_IF(sigaction(SIGUSR1, &action, NULL) == -1);
+
+	raise(SIGUSR1);
+
+	return 0;
+}
+
+int main(void)
+{
+	return test_harness(test_sigreturn_unaligned, "sigreturn_unaligned");
+}
-- 
2.31.1

Re: [PATCH 1/3] powerpc/64s: Mask NIP before checking against SRR0

[Sachin Sant]

> On 21-Dec-2021, at 7:20 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> When CONFIG_PPC_RFI_SRR_DEBUG=y we check that NIP and SRR0 match when
> returning from interrupts. This can trigger falsely if NIP has either of
> its two low bits set via sigreturn or ptrace, while SRR0 has its low two
> bits masked in hardware.
> 
> As a quick fix make sure to mask the low bits before doing the check.
> 
> Fixes: 59dc5bfca0cb ("powerpc/64s: avoid reloading (H)SRR registers if they are still valid")
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---

Tested this 3 patch series successfully (with and without PPC_RFI_SRR_DEBUG)
on Power9/Power10 LPAR as well as Power9 PowerNV. 

Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>

Re: [PATCH 1/3] powerpc/64s: Mask NIP before checking against SRR0

[Michael Ellerman]

On Wed, 22 Dec 2021 00:50:59 +1100, Michael Ellerman wrote:
> When CONFIG_PPC_RFI_SRR_DEBUG=y we check that NIP and SRR0 match when
> returning from interrupts. This can trigger falsely if NIP has either of
> its two low bits set via sigreturn or ptrace, while SRR0 has its low two
> bits masked in hardware.
> 
> As a quick fix make sure to mask the low bits before doing the check.
> 
> [...]

Applied to powerpc/next.

[1/3] powerpc/64s: Mask NIP before checking against SRR0
      https://git.kernel.org/powerpc/c/314f6c23dd8d417281eb9e8a516dd98036f2e7b3
[2/3] powerpc/64s: Use EMIT_WARN_ENTRY for SRR debug warnings
      https://git.kernel.org/powerpc/c/fd1eaaaaa6864b5fb8f99880fcefb49760b8fe4e
[3/3] selftests/powerpc: Add a test of sigreturning to an unaligned address
      https://git.kernel.org/powerpc/c/beeac538c366cd2828092adecd1edab28326c55b

cheers