linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 00/13] Add DEXCR support
@ 2022-11-28  2:44 Benjamin Gray
  2022-11-28  2:44 ` [RFC PATCH 01/13] powerpc/book3s: Add missing <linux/sched.h> include Benjamin Gray
                   ` (13 more replies)
  0 siblings, 14 replies; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

This series is based on initial work by Chris Riedl that was not sent
to the list.

Adds a kernel interface for userspace to interact with the DEXCR.
The DEXCR is a SPR that allows control over various execution
'aspects', such as indirect branch prediction and enabling the
hashst/hashchk instructions. Further details are in ISA 3.1B
Book 3 chapter 12.

This RFC proposes an interface for users to interact with the DEXCR.
It aims to support

* Querying supported aspects
* Getting/setting aspects on a per-process level
* Allowing global overrides across all processes

There are some parts that I'm not sure on the best way to approach (hence RFC):

* The feature names in arch/powerpc/kernel/dt_cpu_ftrs.c appear to be unimplemented
  in skiboot, so are being defined by this series. Is being so verbose fine?
* What aspects should be editable by a process? E.g., SBHE has
  effects that potentially bleed into other processes. Should
  it only be system wide configurable?
* Should configuring certain aspects for the process be non-privileged? E.g.,
  Is there harm in always allowing configuration of IBRTPD, SRAPD? The *FORCE_SET*
  action prevents further process local changes regardless of privilege.
* The tests fail Patchwork CI because of the new prctl macros, and the CI
  doesn't run headers_install and add -isystem <buildpath>/usr/include to
  the make command.
* On handling an exception, I don't check if the NPHIE bit is enabled in the DEXCR.
  To do so would require reading both the DEXCR and HDEXCR, for little gain (it
  should only matter that the current instruction was a hashchk. If so, the only
  reason it would cause an exception is the failed check. If the instruction is
  rewritten between exception and check we'd be wrong anyway).

The series is based on the earlier selftest utils series[1], so the tests won't build
at all without applying that first. The kernel side should build fine on ppc/next
247f34f7b80357943234f93f247a1ae6b6c3a740 though.

[1]: https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20221122231103.15829-1-bgray@linux.ibm.com/

Benjamin Gray (13):
  powerpc/book3s: Add missing <linux/sched.h> include
  powerpc: Add initial Dynamic Execution Control Register (DEXCR)
    support
  powerpc/dexcr: Handle hashchk exception
  powerpc/dexcr: Support userspace ROP protection
  prctl: Define PowerPC DEXCR interface
  powerpc/dexcr: Add prctl implementation
  powerpc/dexcr: Add sysctl entry for SBHE system override
  powerpc/dexcr: Add enforced userspace ROP protection config
  selftests/powerpc: Add more utility macros
  selftests/powerpc: Add hashst/hashchk test
  selftests/powerpc: Add DEXCR prctl, sysctl interface test
  selftests/powerpc: Add DEXCR status utility lsdexcr
  Documentation: Document PowerPC kernel DEXCR interface

 Documentation/powerpc/dexcr.rst               | 183 +++++++++++
 Documentation/powerpc/index.rst               |   1 +
 arch/powerpc/Kconfig                          |   5 +
 arch/powerpc/include/asm/book3s/64/kexec.h    |   6 +
 arch/powerpc/include/asm/book3s/64/kup.h      |   1 +
 arch/powerpc/include/asm/cputable.h           |   8 +-
 arch/powerpc/include/asm/ppc-opcode.h         |   1 +
 arch/powerpc/include/asm/processor.h          |  33 ++
 arch/powerpc/include/asm/reg.h                |   7 +
 arch/powerpc/kernel/Makefile                  |   1 +
 arch/powerpc/kernel/dexcr.c                   | 310 ++++++++++++++++++
 arch/powerpc/kernel/dt_cpu_ftrs.c             |   4 +
 arch/powerpc/kernel/process.c                 |  31 +-
 arch/powerpc/kernel/prom.c                    |   4 +
 arch/powerpc/kernel/traps.c                   |   6 +
 include/uapi/linux/prctl.h                    |  14 +
 kernel/sys.c                                  |  16 +
 tools/testing/selftests/powerpc/Makefile      |   1 +
 .../selftests/powerpc/dexcr/.gitignore        |   3 +
 .../testing/selftests/powerpc/dexcr/Makefile  |  11 +
 tools/testing/selftests/powerpc/dexcr/cap.c   |  72 ++++
 tools/testing/selftests/powerpc/dexcr/cap.h   |  18 +
 tools/testing/selftests/powerpc/dexcr/dexcr.c | 118 +++++++
 tools/testing/selftests/powerpc/dexcr/dexcr.h |  54 +++
 .../selftests/powerpc/dexcr/dexcr_test.c      | 241 ++++++++++++++
 .../selftests/powerpc/dexcr/hashchk_test.c    | 229 +++++++++++++
 .../testing/selftests/powerpc/dexcr/lsdexcr.c | 178 ++++++++++
 tools/testing/selftests/powerpc/include/reg.h |   4 +
 .../testing/selftests/powerpc/include/utils.h |  44 +++
 29 files changed, 1602 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/powerpc/dexcr.rst
 create mode 100644 arch/powerpc/kernel/dexcr.c
 create mode 100644 tools/testing/selftests/powerpc/dexcr/.gitignore
 create mode 100644 tools/testing/selftests/powerpc/dexcr/Makefile
 create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.c
 create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.h
 create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr.c
 create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr.h
 create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr_test.c
 create mode 100644 tools/testing/selftests/powerpc/dexcr/hashchk_test.c
 create mode 100644 tools/testing/selftests/powerpc/dexcr/lsdexcr.c


base-commit: 9dc58a6040662faaf24c8932861f485670fce7ff
--
2.38.1

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

* [RFC PATCH 01/13] powerpc/book3s: Add missing <linux/sched.h> include
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2023-03-07  4:28   ` Nicholas Piggin
  2022-11-28  2:44 ` [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support Benjamin Gray
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

The functions here use struct thread_struct fields, so need to import
the full definition from <linux/sched.h>. The <asm/current.h> header
that defines current only forward declares struct thread_struct.

Failing to include this <linux/sched.h> header leads to a compilation
error when a translation unit does not also include <linux/sched.h>
indirectly.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/kup.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index 54cf46808157..84c09e546115 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -194,6 +194,7 @@
 #else /* !__ASSEMBLY__ */
 
 #include <linux/jump_label.h>
+#include <linux/sched.h>
 
 DECLARE_STATIC_KEY_FALSE(uaccess_flush_key);
 
-- 
2.38.1


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

* [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
  2022-11-28  2:44 ` [RFC PATCH 01/13] powerpc/book3s: Add missing <linux/sched.h> include Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2023-03-07  4:45   ` Nicholas Piggin
  2022-11-28  2:44 ` [RFC PATCH 03/13] powerpc/dexcr: Handle hashchk exception Benjamin Gray
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

ISA 3.1B introduces the Dynamic Execution Control Register (DEXCR). It
is a per-cpu register that allows control over various CPU behaviours
including branch hint usage, indirect branch speculation, and
hashst/hashchk support.

Though introduced in 3.1B, no CPUs using 3.1 were released, so
CPU_FTR_ARCH_31 is used to determine support for the register itself.
Support for each DEXCR bit (aspect) is reported separately by the
firmware.

Add various definitions and basic support for the DEXCR in the kernel.
Right now it just initialises and maintains the DEXCR on process
creation/swap, and clears it in reset_sprs().

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/kexec.h |  3 +++
 arch/powerpc/include/asm/cputable.h        |  8 ++++++-
 arch/powerpc/include/asm/processor.h       | 13 +++++++++++
 arch/powerpc/include/asm/reg.h             |  6 ++++++
 arch/powerpc/kernel/Makefile               |  1 +
 arch/powerpc/kernel/dexcr.c                | 25 ++++++++++++++++++++++
 arch/powerpc/kernel/dt_cpu_ftrs.c          |  4 ++++
 arch/powerpc/kernel/process.c              | 13 ++++++++++-
 arch/powerpc/kernel/prom.c                 |  4 ++++
 9 files changed, 75 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/kernel/dexcr.c

diff --git a/arch/powerpc/include/asm/book3s/64/kexec.h b/arch/powerpc/include/asm/book3s/64/kexec.h
index d4b9d476ecba..563baf94a962 100644
--- a/arch/powerpc/include/asm/book3s/64/kexec.h
+++ b/arch/powerpc/include/asm/book3s/64/kexec.h
@@ -21,6 +21,9 @@ static inline void reset_sprs(void)
 			plpar_set_ciabr(0);
 	}
 
+	if (cpu_has_feature(CPU_FTR_ARCH_31))
+		mtspr(SPRN_DEXCR, 0);
+
 	/*  Do we need isync()? We are going via a kexec reset */
 	isync();
 }
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 757dbded11dc..03bc192f2d8b 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -192,6 +192,10 @@ static inline void cpu_feature_keys_init(void) { }
 #define CPU_FTR_P9_RADIX_PREFETCH_BUG	LONG_ASM_CONST(0x0002000000000000)
 #define CPU_FTR_ARCH_31			LONG_ASM_CONST(0x0004000000000000)
 #define CPU_FTR_DAWR1			LONG_ASM_CONST(0x0008000000000000)
+#define CPU_FTR_DEXCR_SBHE		LONG_ASM_CONST(0x0010000000000000)
+#define CPU_FTR_DEXCR_IBRTPD		LONG_ASM_CONST(0x0020000000000000)
+#define CPU_FTR_DEXCR_SRAPD		LONG_ASM_CONST(0x0040000000000000)
+#define CPU_FTR_DEXCR_NPHIE		LONG_ASM_CONST(0x0080000000000000)
 
 #ifndef __ASSEMBLY__
 
@@ -451,7 +455,9 @@ static inline void cpu_feature_keys_init(void) { }
 	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
 	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
 	    CPU_FTR_ARCH_300 | CPU_FTR_ARCH_31 | \
-	    CPU_FTR_DAWR | CPU_FTR_DAWR1)
+	    CPU_FTR_DAWR | CPU_FTR_DAWR1 | \
+	    CPU_FTR_DEXCR_SBHE | CPU_FTR_DEXCR_IBRTPD | CPU_FTR_DEXCR_SRAPD | \
+	    CPU_FTR_DEXCR_NPHIE)
 #define CPU_FTRS_CELL	(CPU_FTR_LWSYNC | \
 	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
 	    CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 631802999d59..0a8a793b8b8b 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -446,6 +446,19 @@ int exit_vmx_usercopy(void);
 int enter_vmx_ops(void);
 void *exit_vmx_ops(void *dest);
 
+#ifdef CONFIG_PPC_BOOK3S_64
+
+unsigned long get_thread_dexcr(struct thread_struct const *t);
+
+#else
+
+static inline unsigned long get_thread_dexcr(struct thread_struct const *t)
+{
+	return 0;
+}
+
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
 #endif /* __KERNEL__ */
 #endif /* __ASSEMBLY__ */
 #endif /* _ASM_POWERPC_PROCESSOR_H */
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 1e8b2e04e626..cdd1f174c399 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -385,6 +385,12 @@
 #define SPRN_HSRR0	0x13A	/* Hypervisor Save/Restore 0 */
 #define SPRN_HSRR1	0x13B	/* Hypervisor Save/Restore 1 */
 #define SPRN_ASDR	0x330	/* Access segment descriptor register */
+#define SPRN_DEXCR	0x33C	/* Dynamic execution control register */
+#define   DEXCR_PRO_MASK(aspect)	__MASK(63 - (32 + (aspect)))	/* Aspect number to problem state aspect mask */
+#define   DEXCR_PRO_SBHE		DEXCR_PRO_MASK(0)	/* Speculative Branch Hint Enable */
+#define   DEXCR_PRO_IBRTPD		DEXCR_PRO_MASK(3)	/* Indirect Branch Recurrent Target Prediction Disable */
+#define   DEXCR_PRO_SRAPD		DEXCR_PRO_MASK(4)	/* Subroutine Return Address Prediction Disable */
+#define   DEXCR_PRO_NPHIE		DEXCR_PRO_MASK(5)	/* Non-Privileged Hash Instruction Enable */
 #define SPRN_IC		0x350	/* Virtual Instruction Count */
 #define SPRN_VTB	0x351	/* Virtual Time Base */
 #define SPRN_LDBAR	0x352	/* LD Base Address Register */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 9b6146056e48..b112315cfdc2 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_VDSO32)		+= vdso32_wrapper.o
 obj-$(CONFIG_PPC_WATCHDOG)	+= watchdog.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
 obj-$(CONFIG_PPC_DAWR)		+= dawr.o
+obj-$(CONFIG_PPC_BOOK3S_64)	+= dexcr.o
 obj-$(CONFIG_PPC_BOOK3S_64)	+= cpu_setup_ppc970.o cpu_setup_pa6t.o
 obj-$(CONFIG_PPC_BOOK3S_64)	+= cpu_setup_power.o
 obj-$(CONFIG_PPC_BOOK3S_64)	+= mce.o mce_power.o
diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
new file mode 100644
index 000000000000..32a0a69ff638
--- /dev/null
+++ b/arch/powerpc/kernel/dexcr.c
@@ -0,0 +1,25 @@
+#include <linux/cache.h>
+#include <linux/init.h>
+
+#include <asm/cpu_has_feature.h>
+#include <asm/cputable.h>
+#include <asm/processor.h>
+#include <asm/reg.h>
+
+#define DEFAULT_DEXCR	0
+
+static int __init dexcr_init(void)
+{
+	if (!early_cpu_has_feature(CPU_FTR_ARCH_31))
+		return 0;
+
+	mtspr(SPRN_DEXCR, DEFAULT_DEXCR);
+
+	return 0;
+}
+early_initcall(dexcr_init);
+
+unsigned long get_thread_dexcr(struct thread_struct const *t)
+{
+	return DEFAULT_DEXCR;
+}
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index c3fb9fdf5bd7..896a48211a37 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -661,6 +661,10 @@ static struct dt_cpu_feature_match __initdata
 	{"prefix-instructions", feat_enable, 0},
 	{"matrix-multiply-assist", feat_enable_mma, 0},
 	{"debug-facilities-v31", feat_enable, CPU_FTR_DAWR1},
+	{"dexcr-speculative-branch-hint-enable", feat_enable, CPU_FTR_DEXCR_SBHE},
+	{"dexcr-indirect-branch-recurrent-target-prediction-disable", feat_enable, CPU_FTR_DEXCR_IBRTPD},
+	{"dexcr-subroutine-return-address-prediction-disable", feat_enable, CPU_FTR_DEXCR_SRAPD},
+	{"dexcr-non-privileged-hash-instruction-enable", feat_enable, CPU_FTR_DEXCR_NPHIE},
 };
 
 static bool __initdata using_dt_cpu_ftrs;
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 67da147fe34d..17d26f652b80 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1228,6 +1228,13 @@ static inline void restore_sprs(struct thread_struct *old_thread,
 	if (cpu_has_feature(CPU_FTR_P9_TIDR) &&
 	    old_thread->tidr != new_thread->tidr)
 		mtspr(SPRN_TIDR, new_thread->tidr);
+
+	if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+		unsigned long new_dexcr = get_thread_dexcr(new_thread);
+
+		if (new_dexcr != get_thread_dexcr(old_thread))
+			mtspr(SPRN_DEXCR, new_dexcr);
+	}
 #endif
 
 }
@@ -1802,7 +1809,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 
 	setup_ksp_vsid(p, sp);
 
-#ifdef CONFIG_PPC64 
+#ifdef CONFIG_PPC64
 	if (cpu_has_feature(CPU_FTR_DSCR)) {
 		p->thread.dscr_inherit = current->thread.dscr_inherit;
 		p->thread.dscr = mfspr(SPRN_DSCR);
@@ -1939,6 +1946,10 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
 	current->thread.tm_tfiar = 0;
 	current->thread.load_tm = 0;
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+#ifdef CONFIG_PPC_BOOK3S_64
+	if (cpu_has_feature(CPU_FTR_ARCH_31))
+		mtspr(SPRN_DEXCR, get_thread_dexcr(&current->thread));
+#endif /* CONFIG_PPC_BOOK3S_64 */
 }
 EXPORT_SYMBOL(start_thread);
 
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 1eed87d954ba..eff250e1ae9a 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -180,6 +180,10 @@ static struct ibm_feature ibm_pa_features[] __initdata = {
 	  .cpu_user_ftrs2 = PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_HTM_NOSC_COMP },
 
 	{ .pabyte = 64, .pabit = 0, .cpu_features = CPU_FTR_DAWR1 },
+	{ .pabyte = 68, .pabit = 0, .cpu_features = CPU_FTR_DEXCR_SBHE },
+	{ .pabyte = 68, .pabit = 3, .cpu_features = CPU_FTR_DEXCR_IBRTPD },
+	{ .pabyte = 68, .pabit = 4, .cpu_features = CPU_FTR_DEXCR_SRAPD },
+	{ .pabyte = 68, .pabit = 5, .cpu_features = CPU_FTR_DEXCR_NPHIE },
 };
 
 /*
-- 
2.38.1


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

* [RFC PATCH 03/13] powerpc/dexcr: Handle hashchk exception
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
  2022-11-28  2:44 ` [RFC PATCH 01/13] powerpc/book3s: Add missing <linux/sched.h> include Benjamin Gray
  2022-11-28  2:44 ` [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2022-11-29 10:39   ` Nicholas Piggin
  2022-11-28  2:44 ` [RFC PATCH 04/13] powerpc/dexcr: Support userspace ROP protection Benjamin Gray
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

Recognise and pass the appropriate signal to the user program when a
hashchk instruction triggers. This is independent of allowing
configuration of DEXCR[NPHIE], as a hypervisor can enforce this aspect
regardless of the kernel.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h |  1 +
 arch/powerpc/include/asm/processor.h  |  6 ++++++
 arch/powerpc/kernel/dexcr.c           | 22 ++++++++++++++++++++++
 arch/powerpc/kernel/traps.c           |  6 ++++++
 4 files changed, 35 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 21e33e46f4b8..89b316466ed1 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -215,6 +215,7 @@
 #define OP_31_XOP_STFSX	    663
 #define OP_31_XOP_STFSUX    695
 #define OP_31_XOP_STFDX     727
+#define OP_31_XOP_HASHCHK   754
 #define OP_31_XOP_STFDUX    759
 #define OP_31_XOP_LHBRX     790
 #define OP_31_XOP_LFIWAX    855
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 0a8a793b8b8b..c17ec1e44c86 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -448,10 +448,16 @@ void *exit_vmx_ops(void *dest);
 
 #ifdef CONFIG_PPC_BOOK3S_64
 
+bool is_hashchk_trap(struct pt_regs const *regs);
 unsigned long get_thread_dexcr(struct thread_struct const *t);
 
 #else
 
+static inline bool is_hashchk_trap(struct pt_regs const *regs)
+{
+	return false;
+}
+
 static inline unsigned long get_thread_dexcr(struct thread_struct const *t)
 {
 	return 0;
diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
index 32a0a69ff638..11515e67afac 100644
--- a/arch/powerpc/kernel/dexcr.c
+++ b/arch/powerpc/kernel/dexcr.c
@@ -3,6 +3,9 @@
 
 #include <asm/cpu_has_feature.h>
 #include <asm/cputable.h>
+#include <asm/disassemble.h>
+#include <asm/inst.h>
+#include <asm/ppc-opcode.h>
 #include <asm/processor.h>
 #include <asm/reg.h>
 
@@ -19,6 +22,25 @@ static int __init dexcr_init(void)
 }
 early_initcall(dexcr_init);
 
+bool is_hashchk_trap(struct pt_regs const *regs)
+{
+	ppc_inst_t insn;
+
+	if (!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
+		return false;
+
+	if (get_user_instr(insn, (void __user *)regs->nip)) {
+		WARN_ON(1);
+		return false;
+	}
+
+	if (ppc_inst_primary_opcode(insn) == 31 &&
+	    get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK)
+		return true;
+
+	return false;
+}
+
 unsigned long get_thread_dexcr(struct thread_struct const *t)
 {
 	return DEFAULT_DEXCR;
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 9bdd79aa51cf..b83f5b382f24 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1516,6 +1516,12 @@ static void do_program_check(struct pt_regs *regs)
 				return;
 			}
 		}
+
+		if (user_mode(regs) && is_hashchk_trap(regs)) {
+			_exception(SIGILL, regs, ILL_ILLOPN, regs->nip);
+			return;
+		}
+
 		_exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip);
 		return;
 	}
-- 
2.38.1


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

* [RFC PATCH 04/13] powerpc/dexcr: Support userspace ROP protection
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (2 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 03/13] powerpc/dexcr: Handle hashchk exception Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2023-03-07  5:05   ` Nicholas Piggin
  2022-11-28  2:44 ` [RFC PATCH 05/13] prctl: Define PowerPC DEXCR interface Benjamin Gray
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

The ISA 3.1B hashst and hashchk instructions use a per-cpu SPR HASHKEYR
to hold a key used in the hash calculation. This key should be different
for each process to make it harder for a malicious process to recreate
valid hash values for a victim process.

Add support for storing a per-thread hash key, and setting/clearing
HASHKEYR appropriately.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/kexec.h |  3 +++
 arch/powerpc/include/asm/processor.h       |  1 +
 arch/powerpc/include/asm/reg.h             |  1 +
 arch/powerpc/kernel/process.c              | 12 ++++++++++++
 4 files changed, 17 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/64/kexec.h b/arch/powerpc/include/asm/book3s/64/kexec.h
index 563baf94a962..163de935df28 100644
--- a/arch/powerpc/include/asm/book3s/64/kexec.h
+++ b/arch/powerpc/include/asm/book3s/64/kexec.h
@@ -24,6 +24,9 @@ static inline void reset_sprs(void)
 	if (cpu_has_feature(CPU_FTR_ARCH_31))
 		mtspr(SPRN_DEXCR, 0);
 
+	if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
+		mtspr(SPRN_HASHKEYR, 0);
+
 	/*  Do we need isync()? We are going via a kexec reset */
 	isync();
 }
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index c17ec1e44c86..2381217c95dc 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -264,6 +264,7 @@ struct thread_struct {
 	unsigned long   mmcr3;
 	unsigned long   sier2;
 	unsigned long   sier3;
+	unsigned long	hashkeyr;
 
 #endif
 };
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index cdd1f174c399..854664cf844f 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -384,6 +384,7 @@
 #define SPRN_HRMOR	0x139	/* Real mode offset register */
 #define SPRN_HSRR0	0x13A	/* Hypervisor Save/Restore 0 */
 #define SPRN_HSRR1	0x13B	/* Hypervisor Save/Restore 1 */
+#define SPRN_HASHKEYR	0x1D4	/* Non-privileged hashst/hashchk key register */
 #define SPRN_ASDR	0x330	/* Access segment descriptor register */
 #define SPRN_DEXCR	0x33C	/* Dynamic execution control register */
 #define   DEXCR_PRO_MASK(aspect)	__MASK(63 - (32 + (aspect)))	/* Aspect number to problem state aspect mask */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 17d26f652b80..4d7b0c7641d0 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1229,6 +1229,9 @@ static inline void restore_sprs(struct thread_struct *old_thread,
 	    old_thread->tidr != new_thread->tidr)
 		mtspr(SPRN_TIDR, new_thread->tidr);
 
+	if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
+		mtspr(SPRN_HASHKEYR, new_thread->hashkeyr);
+
 	if (cpu_has_feature(CPU_FTR_ARCH_31)) {
 		unsigned long new_dexcr = get_thread_dexcr(new_thread);
 
@@ -1818,6 +1821,10 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 		childregs->ppr = DEFAULT_PPR;
 
 	p->thread.tidr = 0;
+#endif
+#ifdef CONFIG_PPC_BOOK3S_64
+	if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
+		p->thread.hashkeyr = current->thread.hashkeyr;
 #endif
 	/*
 	 * Run with the current AMR value of the kernel
@@ -1947,6 +1954,11 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
 	current->thread.load_tm = 0;
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
 #ifdef CONFIG_PPC_BOOK3S_64
+	if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE)) {
+		current->thread.hashkeyr = get_random_long();
+		mtspr(SPRN_HASHKEYR, current->thread.hashkeyr);
+	}
+
 	if (cpu_has_feature(CPU_FTR_ARCH_31))
 		mtspr(SPRN_DEXCR, get_thread_dexcr(&current->thread));
 #endif /* CONFIG_PPC_BOOK3S_64 */
-- 
2.38.1


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

* [RFC PATCH 05/13] prctl: Define PowerPC DEXCR interface
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (3 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 04/13] powerpc/dexcr: Support userspace ROP protection Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2023-03-07  5:07   ` Nicholas Piggin
  2022-11-28  2:44 ` [RFC PATCH 06/13] powerpc/dexcr: Add prctl implementation Benjamin Gray
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

Adds the definitions and generic handler for prctl control of the
PowerPC Dynamic Execution Control Register (DEXCR).

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 include/uapi/linux/prctl.h | 14 ++++++++++++++
 kernel/sys.c               | 16 ++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index a5e06dcbba13..b4720e8de6f3 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -281,6 +281,20 @@ struct prctl_mm_map {
 # define PR_SME_VL_LEN_MASK		0xffff
 # define PR_SME_VL_INHERIT		(1 << 17) /* inherit across exec */
 
+/* PowerPC Dynamic Execution Control Register (DEXCR) controls */
+#define PR_PPC_GET_DEXCR		65
+#define PR_PPC_SET_DEXCR		66
+/* DEXCR aspect to act on */
+# define PR_PPC_DEXCR_SBHE		0 /* Speculative branch hint enable */
+# define PR_PPC_DEXCR_IBRTPD		1 /* Indirect branch recurrent target prediction disable */
+# define PR_PPC_DEXCR_SRAPD		2 /* Subroutine return address prediction disable */
+# define PR_PPC_DEXCR_NPHIE		3 /* Non-privileged hash instruction enable */
+/* Action to apply / return */
+# define PR_PPC_DEXCR_PRCTL		(1 << 0)
+# define PR_PPC_DEXCR_SET_ASPECT	(1 << 1)
+# define PR_PPC_DEXCR_FORCE_SET_ASPECT	(1 << 2)
+# define PR_PPC_DEXCR_CLEAR_ASPECT	(1 << 3)
+
 #define PR_SET_VMA		0x53564d41
 # define PR_SET_VMA_ANON_NAME		0
 
diff --git a/kernel/sys.c b/kernel/sys.c
index 5fd54bf0e886..55b8f7369059 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -139,6 +139,12 @@
 #ifndef GET_TAGGED_ADDR_CTRL
 # define GET_TAGGED_ADDR_CTRL()		(-EINVAL)
 #endif
+#ifndef PPC_GET_DEXCR_ASPECT
+# define PPC_GET_DEXCR_ASPECT(a, b)	(-EINVAL)
+#endif
+#ifndef PPC_SET_DEXCR_ASPECT
+# define PPC_SET_DEXCR_ASPECT(a, b, c)	(-EINVAL)
+#endif
 
 /*
  * this is where the system-wide overflow UID and GID are defined, for
@@ -2623,6 +2629,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		error = sched_core_share_pid(arg2, arg3, arg4, arg5);
 		break;
 #endif
+	case PR_PPC_GET_DEXCR:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = PPC_GET_DEXCR_ASPECT(me, arg2);
+		break;
+	case PR_PPC_SET_DEXCR:
+		if (arg4 || arg5)
+			return -EINVAL;
+		error = PPC_SET_DEXCR_ASPECT(me, arg2, arg3);
+		break;
 	case PR_SET_VMA:
 		error = prctl_set_vma(arg2, arg3, arg4, arg5);
 		break;
-- 
2.38.1


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

* [RFC PATCH 06/13] powerpc/dexcr: Add prctl implementation
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (4 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 05/13] prctl: Define PowerPC DEXCR interface Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2023-03-07  5:12   ` Nicholas Piggin
  2022-11-28  2:44 ` [RFC PATCH 07/13] powerpc/dexcr: Add sysctl entry for SBHE system override Benjamin Gray
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

Adds an initial prctl interface implementation. Unprivileged processes
can query the current prctl setting, including whether an aspect is
implemented by the hardware or is permitted to be modified by a setter
prctl. Editable aspects can be changed by a CAP_SYS_ADMIN privileged
process.

The prctl setting represents what the process itself has requested, and
does not account for any overrides. Either the kernel or a hypervisor
may enforce a different setting for an aspect.

Userspace can access a readonly view of the current DEXCR via SPR 812,
and a readonly view of the aspects enforced by the hypervisor via
SPR 455. A bitwise OR of these two SPRs will give the effective
DEXCR aspect state of the process.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/processor.h |  13 +++
 arch/powerpc/kernel/dexcr.c          | 133 ++++++++++++++++++++++++++-
 arch/powerpc/kernel/process.c        |   6 ++
 3 files changed, 151 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 2381217c95dc..4c995258f668 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -265,6 +265,9 @@ struct thread_struct {
 	unsigned long   sier2;
 	unsigned long   sier3;
 	unsigned long	hashkeyr;
+	unsigned int	dexcr_override;
+	unsigned int	dexcr_mask;
+	unsigned int	dexcr_forced;
 
 #endif
 };
@@ -338,6 +341,16 @@ extern int set_endian(struct task_struct *tsk, unsigned int val);
 extern int get_unalign_ctl(struct task_struct *tsk, unsigned long adr);
 extern int set_unalign_ctl(struct task_struct *tsk, unsigned int val);
 
+#ifdef CONFIG_PPC_BOOK3S_64
+
+#define PPC_GET_DEXCR_ASPECT(tsk, asp) dexcr_prctl_get((tsk), (asp))
+#define PPC_SET_DEXCR_ASPECT(tsk, asp, val) dexcr_prctl_set((tsk), (asp), (val))
+
+int dexcr_prctl_get(struct task_struct *tsk, unsigned long asp);
+int dexcr_prctl_set(struct task_struct *tsk, unsigned long asp, unsigned long val);
+
+#endif
+
 extern void load_fp_state(struct thread_fp_state *fp);
 extern void store_fp_state(struct thread_fp_state *fp);
 extern void load_vr_state(struct thread_vr_state *vr);
diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
index 11515e67afac..9290beed722a 100644
--- a/arch/powerpc/kernel/dexcr.c
+++ b/arch/powerpc/kernel/dexcr.c
@@ -1,5 +1,8 @@
 #include <linux/cache.h>
+#include <linux/capability.h>
 #include <linux/init.h>
+#include <linux/prctl.h>
+#include <linux/sched.h>
 
 #include <asm/cpu_has_feature.h>
 #include <asm/cputable.h>
@@ -11,6 +14,10 @@
 
 #define DEFAULT_DEXCR	0
 
+/* Allow process configuration of these by default */
+#define DEXCR_PRCTL_EDITABLE (DEXCR_PRO_SBHE | DEXCR_PRO_IBRTPD | \
+			      DEXCR_PRO_SRAPD | DEXCR_PRO_NPHIE)
+
 static int __init dexcr_init(void)
 {
 	if (!early_cpu_has_feature(CPU_FTR_ARCH_31))
@@ -43,5 +50,129 @@ bool is_hashchk_trap(struct pt_regs const *regs)
 
 unsigned long get_thread_dexcr(struct thread_struct const *t)
 {
-	return DEFAULT_DEXCR;
+	unsigned long dexcr = DEFAULT_DEXCR;
+
+	/* Apply prctl overrides */
+	dexcr = (dexcr & ~t->dexcr_mask) | t->dexcr_override;
+
+	return dexcr;
+}
+
+static void update_dexcr_on_cpu(void *info)
+{
+	mtspr(SPRN_DEXCR, get_thread_dexcr(&current->thread));
+}
+
+static int dexcr_aspect_get(struct task_struct *task, unsigned int aspect)
+{
+	int ret = 0;
+
+	if (aspect & DEXCR_PRCTL_EDITABLE)
+		ret |= PR_PPC_DEXCR_PRCTL;
+
+	if (aspect & task->thread.dexcr_mask) {
+		if (aspect & task->thread.dexcr_override) {
+			if (aspect & task->thread.dexcr_forced)
+				ret |= PR_PPC_DEXCR_FORCE_SET_ASPECT;
+			else
+				ret |= PR_PPC_DEXCR_SET_ASPECT;
+		} else {
+			ret |= PR_PPC_DEXCR_CLEAR_ASPECT;
+		}
+	}
+
+	return ret;
+}
+
+int dexcr_prctl_get(struct task_struct *task, unsigned long which)
+{
+	switch (which) {
+	case PR_PPC_DEXCR_SBHE:
+		if (!cpu_has_feature(CPU_FTR_DEXCR_SBHE))
+			return -ENODEV;
+		return dexcr_aspect_get(task, DEXCR_PRO_SBHE);
+	case PR_PPC_DEXCR_IBRTPD:
+		if (!cpu_has_feature(CPU_FTR_DEXCR_IBRTPD))
+			return -ENODEV;
+		return dexcr_aspect_get(task, DEXCR_PRO_IBRTPD);
+	case PR_PPC_DEXCR_SRAPD:
+		if (!cpu_has_feature(CPU_FTR_DEXCR_SRAPD))
+			return -ENODEV;
+		return dexcr_aspect_get(task, DEXCR_PRO_SRAPD);
+	case PR_PPC_DEXCR_NPHIE:
+		if (!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
+			return -ENODEV;
+		return dexcr_aspect_get(task, DEXCR_PRO_NPHIE);
+	default:
+		return -ENODEV;
+	}
+}
+
+static int dexcr_aspect_set(struct task_struct *task, unsigned int aspect, unsigned long ctrl)
+{
+	if (!(aspect & DEXCR_PRCTL_EDITABLE))
+		return -ENXIO;  /* Aspect is not allowed to be changed by prctl */
+
+	if (aspect & task->thread.dexcr_forced)
+		return -EPERM;  /* Aspect has been forced to current state */
+
+	switch (ctrl) {
+	case PR_PPC_DEXCR_SET_ASPECT:
+		task->thread.dexcr_mask |= aspect;
+		task->thread.dexcr_override |= aspect;
+		break;
+	case PR_PPC_DEXCR_FORCE_SET_ASPECT:
+		task->thread.dexcr_mask |= aspect;
+		task->thread.dexcr_override |= aspect;
+		task->thread.dexcr_forced |= aspect;
+		break;
+	case PR_PPC_DEXCR_CLEAR_ASPECT:
+		task->thread.dexcr_mask |= aspect;
+		task->thread.dexcr_override &= ~aspect;
+		break;
+	default:
+		return -ERANGE;
+	}
+
+	return 0;
+}
+
+int dexcr_prctl_set(struct task_struct *task, unsigned long which, unsigned long ctrl)
+{
+	int err = 0;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	switch (which) {
+	case PR_PPC_DEXCR_SBHE:
+		if (!cpu_has_feature(CPU_FTR_DEXCR_SBHE))
+			return -ENODEV;
+		err = dexcr_aspect_set(task, DEXCR_PRO_SBHE, ctrl);
+		break;
+	case PR_PPC_DEXCR_IBRTPD:
+		if (!cpu_has_feature(CPU_FTR_DEXCR_IBRTPD))
+			return -ENODEV;
+		err = dexcr_aspect_set(task, DEXCR_PRO_IBRTPD, ctrl);
+		break;
+	case PR_PPC_DEXCR_SRAPD:
+		if (!cpu_has_feature(CPU_FTR_DEXCR_SRAPD))
+			return -ENODEV;
+		err = dexcr_aspect_set(task, DEXCR_PRO_SRAPD, ctrl);
+		break;
+	case PR_PPC_DEXCR_NPHIE:
+		if (!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
+			return -ENODEV;
+		err = dexcr_aspect_set(task, DEXCR_PRO_NPHIE, ctrl);
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	if (err)
+		return err;
+
+	update_dexcr_on_cpu(NULL);
+
+	return 0;
 }
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 4d7b0c7641d0..a280842750f9 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1825,6 +1825,12 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 #ifdef CONFIG_PPC_BOOK3S_64
 	if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
 		p->thread.hashkeyr = current->thread.hashkeyr;
+
+	if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+		p->thread.dexcr_override = current->thread.dexcr_override;
+		p->thread.dexcr_mask = current->thread.dexcr_mask;
+		p->thread.dexcr_forced = current->thread.dexcr_forced;
+	}
 #endif
 	/*
 	 * Run with the current AMR value of the kernel
-- 
2.38.1


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

* [RFC PATCH 07/13] powerpc/dexcr: Add sysctl entry for SBHE system override
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (5 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 06/13] powerpc/dexcr: Add prctl implementation Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2023-03-07  5:30   ` Nicholas Piggin
  2022-11-28  2:44 ` [RFC PATCH 08/13] powerpc/dexcr: Add enforced userspace ROP protection config Benjamin Gray
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

The DEXCR Speculative Branch Hint Enable (SBHE) aspect controls whether
the hints provided by BO field of Branch instructions are obeyed during
speculative execution.

SBHE behaviour per ISA 3.1B:

0:	The hints provided by BO field of Branch instructions may be
	ignored during speculative execution

1:	The hints provided by BO field of Branch instructions are obeyed
	during speculative execution

Add a sysctl entry to allow changing this aspect globally in the system
at runtime:

	/proc/sys/kernel/speculative_branch_hint_enable

Three values are supported:

-1:	Disable DEXCR SBHE sysctl override
 0:	Override and set DEXCR[SBHE] aspect to 0
 1:	Override and set DEXCR[SBHE] aspect to 1

Internally, introduces a mechanism to apply arbitrary system wide
overrides on top of the prctl() config.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/kernel/dexcr.c | 125 ++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)

diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
index 9290beed722a..8239bcc92026 100644
--- a/arch/powerpc/kernel/dexcr.c
+++ b/arch/powerpc/kernel/dexcr.c
@@ -1,8 +1,11 @@
 #include <linux/cache.h>
 #include <linux/capability.h>
+#include <linux/cpu.h>
 #include <linux/init.h>
 #include <linux/prctl.h>
 #include <linux/sched.h>
+#include <linux/spinlock.h>
+#include <linux/sysctl.h>
 
 #include <asm/cpu_has_feature.h>
 #include <asm/cputable.h>
@@ -18,6 +21,58 @@
 #define DEXCR_PRCTL_EDITABLE (DEXCR_PRO_SBHE | DEXCR_PRO_IBRTPD | \
 			      DEXCR_PRO_SRAPD | DEXCR_PRO_NPHIE)
 
+/*
+ * Lock to protect system DEXCR override from concurrent updates.
+ * RCU semantics: writers take lock, readers are unlocked.
+ * Writers ensure the memory update is atomic, readers read
+ * atomically.
+ */
+static DEFINE_SPINLOCK(dexcr_sys_enforced_write_lock);
+
+struct mask_override {
+	union {
+		struct {
+			unsigned int mask;
+			unsigned int override;
+		};
+
+		/* Raw access for atomic read/write */
+		unsigned long all;
+	};
+};
+
+static struct mask_override dexcr_sys_enforced;
+
+static int spec_branch_hint_enable = -1;
+
+static void update_userspace_system_dexcr(unsigned int pro_mask, int value)
+{
+	struct mask_override update = { .all = 0 };
+
+	switch (value) {
+	case -1:  /* Clear the mask bit, clear the override bit */
+		break;
+	case 0:  /* Set the mask bit, clear the override bit */
+		update.mask |= pro_mask;
+		break;
+	case 1:  /* Set the mask bit, set the override bit */
+		update.mask |= pro_mask;
+		update.override |= pro_mask;
+		break;
+	}
+
+	spin_lock(&dexcr_sys_enforced_write_lock);
+
+	/* Use the existing values for the non-updated bits */
+	update.mask |= dexcr_sys_enforced.mask & ~pro_mask;
+	update.override |= dexcr_sys_enforced.override & ~pro_mask;
+
+	/* Atomically update system enforced aspects */
+	WRITE_ONCE(dexcr_sys_enforced.all, update.all);
+
+	spin_unlock(&dexcr_sys_enforced_write_lock);
+}
+
 static int __init dexcr_init(void)
 {
 	if (!early_cpu_has_feature(CPU_FTR_ARCH_31))
@@ -25,6 +80,9 @@ static int __init dexcr_init(void)
 
 	mtspr(SPRN_DEXCR, DEFAULT_DEXCR);
 
+	if (early_cpu_has_feature(CPU_FTR_DEXCR_SBHE))
+		update_userspace_system_dexcr(DEXCR_PRO_SBHE, spec_branch_hint_enable);
+
 	return 0;
 }
 early_initcall(dexcr_init);
@@ -52,9 +110,15 @@ unsigned long get_thread_dexcr(struct thread_struct const *t)
 {
 	unsigned long dexcr = DEFAULT_DEXCR;
 
+	/* Atomically read enforced mask & override */
+	struct mask_override enforced = READ_ONCE(dexcr_sys_enforced);
+
 	/* Apply prctl overrides */
 	dexcr = (dexcr & ~t->dexcr_mask) | t->dexcr_override;
 
+	/* Apply system overrides */
+	dexcr = (dexcr & ~enforced.mask) | enforced.override;
+
 	return dexcr;
 }
 
@@ -176,3 +240,64 @@ int dexcr_prctl_set(struct task_struct *task, unsigned long which, unsigned long
 
 	return 0;
 }
+
+#ifdef CONFIG_SYSCTL
+
+static const int min_sysctl_val = -1;
+
+static int sysctl_dexcr_sbhe_handler(struct ctl_table *table, int write,
+				     void *buf, size_t *lenp, loff_t *ppos)
+{
+	int err;
+	int prev = spec_branch_hint_enable;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (!cpu_has_feature(CPU_FTR_DEXCR_SBHE))
+		return -ENODEV;
+
+	err = proc_dointvec_minmax(table, write, buf, lenp, ppos);
+	if (err)
+		return err;
+
+	if (prev != spec_branch_hint_enable && write) {
+		update_userspace_system_dexcr(DEXCR_PRO_SBHE, spec_branch_hint_enable);
+		cpus_read_lock();
+		on_each_cpu(update_dexcr_on_cpu, NULL, 1);
+		cpus_read_unlock();
+	}
+
+	return 0;
+}
+
+static struct ctl_table dexcr_sbhe_ctl_table[] = {
+	{
+		.procname	= "speculative_branch_hint_enable",
+		.data		= &spec_branch_hint_enable,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= sysctl_dexcr_sbhe_handler,
+		.extra1		= (void *)&min_sysctl_val,
+		.extra2		= SYSCTL_ONE,
+	},
+	{}
+};
+
+static struct ctl_table dexcr_sbhe_ctl_root[] = {
+	{
+		.procname	= "kernel",
+		.mode		= 0555,
+		.child		= dexcr_sbhe_ctl_table,
+	},
+	{}
+};
+
+static int __init register_dexcr_aspects_sysctl(void)
+{
+	register_sysctl_table(dexcr_sbhe_ctl_root);
+	return 0;
+}
+device_initcall(register_dexcr_aspects_sysctl);
+
+#endif /* CONFIG_SYSCTL */
-- 
2.38.1


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

* [RFC PATCH 08/13] powerpc/dexcr: Add enforced userspace ROP protection config
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (6 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 07/13] powerpc/dexcr: Add sysctl entry for SBHE system override Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2022-11-28  2:44 ` [RFC PATCH 09/13] selftests/powerpc: Add more utility macros Benjamin Gray
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

The DEXCR Non-Privileged Hash Instruction Enable (NPHIE) aspect controls
whether the hashst and hashchk instructions are treated as no-ops by the
CPU.

NPHIE behaviour per ISA 3.1B:

0:	hashst and hashchk instructions are executed as no-ops
	(even when allowed by PCR)

1:	hashst and hashchk instructions are executed normally
	(if allowed by PCR)

Currently this aspect may be set per-process by prctl() or enforced
globally by the hypervisor.

Add a kernel config option PPC_USER_ROP_PROTECT to enforce DEXCR[NPHIE]
globally regardless of prctl() or hypervisor. If set, don't report
NPHIE as editable via prctl(), as the prctl() value can never take
effect.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/Kconfig        |  5 +++++
 arch/powerpc/kernel/dexcr.c | 15 +++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 699df27b0e2f..ba3458d07744 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -434,6 +434,11 @@ config PGTABLE_LEVELS
 	default 2 if !PPC64
 	default 4
 
+config PPC_USER_ROP_PROTECT
+	bool
+	depends on PPC_BOOK3S_64
+	default y
+
 source "arch/powerpc/sysdev/Kconfig"
 source "arch/powerpc/platforms/Kconfig"
 
diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
index 8239bcc92026..394140fc23aa 100644
--- a/arch/powerpc/kernel/dexcr.c
+++ b/arch/powerpc/kernel/dexcr.c
@@ -2,6 +2,7 @@
 #include <linux/capability.h>
 #include <linux/cpu.h>
 #include <linux/init.h>
+#include <linux/kconfig.h>
 #include <linux/prctl.h>
 #include <linux/sched.h>
 #include <linux/spinlock.h>
@@ -18,8 +19,8 @@
 #define DEFAULT_DEXCR	0
 
 /* Allow process configuration of these by default */
-#define DEXCR_PRCTL_EDITABLE (DEXCR_PRO_SBHE | DEXCR_PRO_IBRTPD | \
-			      DEXCR_PRO_SRAPD | DEXCR_PRO_NPHIE)
+static unsigned long dexcr_prctl_editable __ro_after_init =
+	DEXCR_PRO_SBHE | DEXCR_PRO_IBRTPD | DEXCR_PRO_SRAPD | DEXCR_PRO_NPHIE;
 
 /*
  * Lock to protect system DEXCR override from concurrent updates.
@@ -83,6 +84,12 @@ static int __init dexcr_init(void)
 	if (early_cpu_has_feature(CPU_FTR_DEXCR_SBHE))
 		update_userspace_system_dexcr(DEXCR_PRO_SBHE, spec_branch_hint_enable);
 
+	if (early_cpu_has_feature(CPU_FTR_DEXCR_NPHIE) &&
+	    IS_ENABLED(CONFIG_PPC_USER_ROP_PROTECT)) {
+		update_userspace_system_dexcr(DEXCR_PRO_NPHIE, 1);
+		dexcr_prctl_editable &= ~DEXCR_PRO_NPHIE;
+	}
+
 	return 0;
 }
 early_initcall(dexcr_init);
@@ -131,7 +138,7 @@ static int dexcr_aspect_get(struct task_struct *task, unsigned int aspect)
 {
 	int ret = 0;
 
-	if (aspect & DEXCR_PRCTL_EDITABLE)
+	if (aspect & dexcr_prctl_editable)
 		ret |= PR_PPC_DEXCR_PRCTL;
 
 	if (aspect & task->thread.dexcr_mask) {
@@ -174,7 +181,7 @@ int dexcr_prctl_get(struct task_struct *task, unsigned long which)
 
 static int dexcr_aspect_set(struct task_struct *task, unsigned int aspect, unsigned long ctrl)
 {
-	if (!(aspect & DEXCR_PRCTL_EDITABLE))
+	if (!(aspect & dexcr_prctl_editable))
 		return -ENXIO;  /* Aspect is not allowed to be changed by prctl */
 
 	if (aspect & task->thread.dexcr_forced)
-- 
2.38.1


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

* [RFC PATCH 09/13] selftests/powerpc: Add more utility macros
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (7 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 08/13] powerpc/dexcr: Add enforced userspace ROP protection config Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2022-11-28  2:44 ` [RFC PATCH 10/13] selftests/powerpc: Add hashst/hashchk test Benjamin Gray
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

Adds more assertion variants to provide more context behind why a
failure occurred.

The SIGSAFE_FAIL_* variants are to allow safely asserting conditions
in a signal handler (though we are about to exit, so it's unlikely to
run into an issue with regular FAIL_IF_EXIT).

Also adds an ARRAY_SIZE macro.

These will be used by the following DEXCR selftests.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 .../testing/selftests/powerpc/include/utils.h | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
index 95f3a24a4569..b03d2192c6f6 100644
--- a/tools/testing/selftests/powerpc/include/utils.h
+++ b/tools/testing/selftests/powerpc/include/utils.h
@@ -9,12 +9,19 @@
 #define __cacheline_aligned __attribute__((aligned(128)))
 
 #include <stdint.h>
+#include <stdio.h>
 #include <stdbool.h>
+#include <string.h>
+#include <unistd.h>
 #include <linux/auxvec.h>
 #include <linux/perf_event.h>
 #include <asm/cputable.h>
 #include "reg.h"
 
+#ifndef ARRAY_SIZE
+# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#endif
+
 /* Avoid headaches with PRI?64 - just use %ll? always */
 typedef unsigned long long u64;
 typedef   signed long long s64;
@@ -111,6 +118,16 @@ do {								\
 	}							\
 } while (0)
 
+#define FAIL_IF_MSG(x, msg)					\
+do {								\
+	if ((x)) {						\
+		fprintf(stderr,					\
+		"[FAIL] Test FAILED on line %d: %s\n", 		\
+		__LINE__, msg);					\
+		return 1;					\
+	}							\
+} while (0)
+
 #define FAIL_IF_EXIT(x)						\
 do {								\
 	if ((x)) {						\
@@ -120,6 +137,16 @@ do {								\
 	}							\
 } while (0)
 
+#define FAIL_IF_EXIT_MSG(x, msg)				\
+do {								\
+	if ((x)) {						\
+		fprintf(stderr,					\
+		"[FAIL] Test FAILED on line %d: %s\n", 		\
+		__LINE__, msg);					\
+		_exit(1);					\
+	}							\
+} while (0)
+
 /* The test harness uses this, yes it's gross */
 #define MAGIC_SKIP_RETURN_VALUE	99
 
@@ -149,6 +176,23 @@ do {								\
 		ssize_t nbytes __attribute__((unused)); \
 		nbytes = write(STDERR_FILENO, msg, strlen(msg)); })
 
+#define SIGSAFE_FAIL_IF_EXIT(x)							\
+do {										\
+	if ((x)) {								\
+		sigsafe_err("[FAIL] Test FAILED on line " str(__LINE__) "\n");	\
+		_exit(1);							\
+	}									\
+} while (0)
+
+#define SIGSAFE_FAIL_IF_EXIT_MSG(x, msg)					\
+do {										\
+	if ((x)) {								\
+		sigsafe_err("[FAIL] Test FAILED on line " 			\
+			    str(__LINE__) ": " msg "\n");			\
+		_exit(1);							\
+	}									\
+} while (0)
+
 /* POWER9 feature */
 #ifndef PPC_FEATURE2_ARCH_3_00
 #define PPC_FEATURE2_ARCH_3_00 0x00800000
-- 
2.38.1


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

* [RFC PATCH 10/13] selftests/powerpc: Add hashst/hashchk test
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (8 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 09/13] selftests/powerpc: Add more utility macros Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2022-11-28  2:44 ` [RFC PATCH 11/13] selftests/powerpc: Add DEXCR prctl, sysctl interface test Benjamin Gray
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

Test the kernel DEXCR[NPHIE] interface and hashchk exception handling.

Introduces with it a DEXCR utils library for common DEXCR operations.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 tools/testing/selftests/powerpc/Makefile      |   1 +
 .../selftests/powerpc/dexcr/.gitignore        |   1 +
 .../testing/selftests/powerpc/dexcr/Makefile  |   9 +
 tools/testing/selftests/powerpc/dexcr/dexcr.c | 118 +++++++++
 tools/testing/selftests/powerpc/dexcr/dexcr.h |  52 ++++
 .../selftests/powerpc/dexcr/hashchk_test.c    | 229 ++++++++++++++++++
 tools/testing/selftests/powerpc/include/reg.h |   4 +
 7 files changed, 414 insertions(+)
 create mode 100644 tools/testing/selftests/powerpc/dexcr/.gitignore
 create mode 100644 tools/testing/selftests/powerpc/dexcr/Makefile
 create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr.c
 create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr.h
 create mode 100644 tools/testing/selftests/powerpc/dexcr/hashchk_test.c

diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 6ba95cd19e42..00dbd000ee01 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -17,6 +17,7 @@ SUB_DIRS = alignment		\
 	   benchmarks		\
 	   cache_shape		\
 	   copyloops		\
+	   dexcr		\
 	   dscr			\
 	   mm			\
 	   nx-gzip		\
diff --git a/tools/testing/selftests/powerpc/dexcr/.gitignore b/tools/testing/selftests/powerpc/dexcr/.gitignore
new file mode 100644
index 000000000000..37adb7f47832
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/.gitignore
@@ -0,0 +1 @@
+hashchk_user
diff --git a/tools/testing/selftests/powerpc/dexcr/Makefile b/tools/testing/selftests/powerpc/dexcr/Makefile
new file mode 100644
index 000000000000..4b4380d4d986
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/Makefile
@@ -0,0 +1,9 @@
+TEST_GEN_PROGS := hashchk_test
+
+TEST_FILES := settings
+top_srcdir = ../../../../..
+include ../../lib.mk
+
+HASHCHK_TEST_CFLAGS = -no-pie $(call cc-option,-mno-rop-protect)
+
+$(TEST_GEN_PROGS): ../harness.c ../utils.c ./dexcr.c
diff --git a/tools/testing/selftests/powerpc/dexcr/dexcr.c b/tools/testing/selftests/powerpc/dexcr/dexcr.c
new file mode 100644
index 000000000000..3e7cb581d4a2
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/dexcr.c
@@ -0,0 +1,118 @@
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/capability.h>
+#include <sys/prctl.h>
+#include <sys/wait.h>
+
+#include "dexcr.h"
+#include "reg.h"
+#include "utils.h"
+
+long sysctl_get_sbhe(void)
+{
+	long value;
+
+	FAIL_IF_EXIT_MSG(read_long(SYSCTL_DEXCR_SBHE, &value, 10),
+			 "failed to read " SYSCTL_DEXCR_SBHE);
+
+	return value;
+}
+
+void sysctl_set_sbhe(long value)
+{
+	FAIL_IF_EXIT_MSG(write_long(SYSCTL_DEXCR_SBHE, value, 10),
+			 "failed to write to " SYSCTL_DEXCR_SBHE);
+}
+
+unsigned int pr_aspect_to_dexcr_mask(unsigned long which)
+{
+	switch (which) {
+	case PR_PPC_DEXCR_SBHE:
+		return DEXCR_PRO_SBHE;
+	case PR_PPC_DEXCR_IBRTPD:
+		return DEXCR_PRO_IBRTPD;
+	case PR_PPC_DEXCR_SRAPD:
+		return DEXCR_PRO_SRAPD;
+	case PR_PPC_DEXCR_NPHIE:
+		return DEXCR_PRO_NPHIE;
+	default:
+		FAIL_IF_EXIT_MSG(true, "unknown PR aspect");
+	}
+}
+
+static inline unsigned int get_dexcr_pro(void)
+{
+	return mfspr(SPRN_DEXCR);
+}
+
+static inline unsigned int get_dexcr_enf(void)
+{
+	return mfspr(SPRN_HDEXCR);
+}
+
+static inline unsigned int get_dexcr_eff(void)
+{
+	return get_dexcr_pro() | get_dexcr_enf();
+}
+
+unsigned int get_dexcr(enum DexcrSource source)
+{
+	switch (source) {
+	case UDEXCR:
+		return get_dexcr_pro();
+	case ENFORCED:
+		return get_dexcr_enf();
+	case EFFECTIVE:
+		return get_dexcr_eff();
+	default:
+		FAIL_IF_EXIT_MSG(true, "bad DEXCR source");
+	}
+}
+
+bool pr_aspect_supported(unsigned long which)
+{
+	return prctl(PR_PPC_GET_DEXCR, which, 0, 0, 0) >= 0;
+}
+
+bool pr_aspect_editable(unsigned long which)
+{
+	int ret = prctl(PR_PPC_GET_DEXCR, which, 0, 0, 0);
+	return ret > 0 && (ret & PR_PPC_DEXCR_PRCTL) > 0;
+}
+
+bool pr_aspect_edit(unsigned long which, unsigned long ctrl)
+{
+	return prctl(PR_PPC_SET_DEXCR, which, ctrl, 0, 0) == 0;
+}
+
+bool pr_aspect_check(unsigned long which, enum DexcrSource source)
+{
+	unsigned int dexcr = get_dexcr(source);
+	unsigned int aspect = pr_aspect_to_dexcr_mask(which);
+	return (dexcr & aspect) != 0;
+}
+
+int pr_aspect_get(unsigned long pr_aspect)
+{
+	int ret = prctl(PR_PPC_GET_DEXCR, pr_aspect, 0, 0, 0);
+	FAIL_IF_EXIT_MSG(ret < 0, "prctl failed");
+	return ret;
+}
+
+bool dexcr_pro_check(unsigned int pro, enum DexcrSource source)
+{
+	return (get_dexcr(source) & pro) != 0;
+}
+
+void await_child_success(pid_t pid)
+{
+	int wstatus;
+
+	FAIL_IF_EXIT_MSG(pid == -1, "fork failed");
+	FAIL_IF_EXIT_MSG(waitpid(pid, &wstatus, 0) == -1, "wait failed");
+	FAIL_IF_EXIT_MSG(!WIFEXITED(wstatus), "child did not exit cleanly");
+	FAIL_IF_EXIT_MSG(WEXITSTATUS(wstatus) != 0, "child exit error");
+}
diff --git a/tools/testing/selftests/powerpc/dexcr/dexcr.h b/tools/testing/selftests/powerpc/dexcr/dexcr.h
new file mode 100644
index 000000000000..fb8007bf19f8
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/dexcr.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * POWER Dynamic Execution Control Facility (DEXCR)
+ *
+ * This header file contains helper functions and macros
+ * required for all the DEXCR related test cases.
+ */
+#ifndef _SELFTESTS_POWERPC_DEXCR_DEXCR_H
+#define _SELFTESTS_POWERPC_DEXCR_DEXCR_H
+
+#include <stdbool.h>
+#include <sys/prctl.h>
+#include <unistd.h>
+
+#include "reg.h"
+#include "utils.h"
+
+#define DEXCR_PRO_MASK(aspect)	__MASK(63 - (32 + (aspect)))
+#define DEXCR_PRO_SBHE		DEXCR_PRO_MASK(0)
+#define DEXCR_PRO_IBRTPD	DEXCR_PRO_MASK(3)
+#define DEXCR_PRO_SRAPD		DEXCR_PRO_MASK(4)
+#define DEXCR_PRO_NPHIE		DEXCR_PRO_MASK(5)
+
+enum DexcrSource {
+	UDEXCR,		/* Userspace DEXCR value */
+	ENFORCED,	/* Enforced by hypervisor */
+	EFFECTIVE,	/* Bitwise OR of requested and enforced DEXCR bits */
+};
+
+unsigned int get_dexcr(enum DexcrSource source);
+
+bool pr_aspect_supported(unsigned long which);
+
+bool pr_aspect_editable(unsigned long which);
+
+bool pr_aspect_edit(unsigned long which, unsigned long ctrl);
+
+bool pr_aspect_check(unsigned long which, enum DexcrSource source);
+
+int pr_aspect_get(unsigned long which);
+
+unsigned int pr_aspect_to_dexcr_mask(unsigned long which);
+
+bool dexcr_pro_check(unsigned int pro, enum DexcrSource source);
+
+long sysctl_get_sbhe(void);
+
+void sysctl_set_sbhe(long value);
+
+void await_child_success(pid_t pid);
+
+#endif  /* _SELFTESTS_POWERPC_DEXCR_DEXCR_H */
diff --git a/tools/testing/selftests/powerpc/dexcr/hashchk_test.c b/tools/testing/selftests/powerpc/dexcr/hashchk_test.c
new file mode 100644
index 000000000000..3351bdbdaf13
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/hashchk_test.c
@@ -0,0 +1,229 @@
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <sched.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/prctl.h>
+#include <unistd.h>
+
+#include "dexcr.h"
+#include "utils.h"
+
+static int require_nphie(void)
+{
+	SKIP_IF_MSG(!pr_aspect_supported(PR_PPC_DEXCR_NPHIE),
+		    "DEXCR[NPHIE] not supported");
+
+	if (dexcr_pro_check(DEXCR_PRO_NPHIE, EFFECTIVE))
+		return 0;
+
+	pr_aspect_edit(PR_PPC_DEXCR_NPHIE, PR_PPC_DEXCR_FORCE_SET_ASPECT);
+	FAIL_IF_EXIT_MSG(!dexcr_pro_check(DEXCR_PRO_NPHIE, EFFECTIVE),
+			 "failed to enable DEXCR[NPIHE]");
+
+	return 0;
+}
+
+static void sigill_handler_enabled(int signum, siginfo_t *info, void *context)
+{
+	SIGSAFE_FAIL_IF_EXIT_MSG(signum != SIGILL, "wrong signal received");
+	SIGSAFE_FAIL_IF_EXIT_MSG(info->si_code != ILL_ILLOPN, "wrong signal-code received");
+	exit(0);
+}
+
+static void do_bad_hashchk(void)
+{
+	unsigned long hash = 0;
+	void *hash_p = ((void *)&hash) + 8;	/* hash* offset must be at least -8 */
+
+	asm ("li 3, 0;"			/* set r3 (pretend LR) to known value */
+	     "hashst 3, -8(%1);"	/* compute good hash */
+	     "addi 3, 3, 1;"		/* modify hash */
+	     "hashchk 3, -8(%1);"	/* check bad hash */
+	     : "+m" (hash) : "r" (hash_p) : "r3");
+}
+
+/*
+ * Check that hashchk triggers when DEXCR[NPHIE] is enabled
+ * and is detected as such by the kernel exception handler
+ */
+static int hashchk_enabled_test(void)
+{
+	int err;
+	struct sigaction sa;
+
+	if ((err = require_nphie()))
+		return err;
+
+	sa.sa_sigaction = sigill_handler_enabled;
+	sigemptyset(&sa.sa_mask);
+	sa.sa_flags = SA_SIGINFO;
+	FAIL_IF_MSG(sigaction(SIGILL, &sa, NULL), "cannot install signal handler");
+
+	do_bad_hashchk();
+
+	FAIL_IF_MSG(true, "hashchk failed to trigger");
+}
+
+#define HASH_COUNT 8
+
+static unsigned long hash_values[HASH_COUNT + 1];
+
+static void fill_hash_values(void)
+{
+	for (unsigned long i = 0; i < HASH_COUNT; i++) {
+		void *hash_addr = ((void*)&hash_values[i]) + 8;
+
+		asm volatile ("hashst %2, -8(%1);"
+			      : "+m" (hash_values[i]) : "r" (hash_addr), "r" (i));
+	}
+
+	hash_values[HASH_COUNT] = (unsigned long)&hash_values;
+}
+
+static unsigned int count_hash_values_matches(void)
+{
+	unsigned long matches = 0;
+
+	FAIL_IF_EXIT_MSG(hash_values[HASH_COUNT] != (unsigned long)hash_values,
+			 "bad address check");
+
+	for (unsigned long i = 0; i < HASH_COUNT; i++) {
+		unsigned long orig_hash = hash_values[i];
+		void *hash_addr = ((void*)&hash_values[i]) + 8;
+
+		asm volatile ("hashst %2, -8(%1);"
+			      : "+m" (hash_values[i]) : "r" (hash_addr), "r" (i));
+
+		if (hash_values[i] == orig_hash)
+			matches++;
+	}
+
+	return matches;
+}
+
+static int hashchk_exec_child(void)
+{
+	ssize_t count;
+
+	fill_hash_values();
+
+	count = write(STDOUT_FILENO, hash_values, sizeof(hash_values));
+	return count == sizeof(hash_values) ? 0 : EOVERFLOW;
+}
+
+/*
+ * Check that new programs get different keys so a malicious process
+ * can't recreate a victim's hash values.
+ */
+static int hashchk_exec_random_key_test(void)
+{
+	pid_t pid;
+	int err;
+	int pipefd[2];
+
+	if ((err = require_nphie()))
+		return err;
+
+	FAIL_IF_MSG(pipe(pipefd), "failed to create pipe");
+
+	pid = fork();
+	if (pid == 0) {
+		char *args[] = { "hashchk_exec_child", NULL };
+
+		if (dup2(pipefd[1], STDOUT_FILENO) == -1)
+			_exit(errno);
+
+		execve("/proc/self/exe", args, NULL);
+		_exit(errno);
+	}
+
+	await_child_success(pid);
+	FAIL_IF_MSG(read(pipefd[0], hash_values, sizeof(hash_values)) != sizeof(hash_values),
+		    "missing expected child output");
+
+	/* If all hashes are the same it means (most likely) same key */
+	FAIL_IF_MSG(count_hash_values_matches() == HASH_COUNT, "shared key detected");
+
+	return 0;
+}
+
+/*
+ * Check that forks share the same key so that existing hash values
+ * remain valid.
+ */
+static int hashchk_fork_share_key_test(void)
+{
+	pid_t pid;
+	int err;
+
+	if ((err = require_nphie()))
+		return err;
+
+	fill_hash_values();
+
+	pid = fork();
+	if (pid == 0) {
+		if (count_hash_values_matches() != HASH_COUNT)
+			_exit(1);
+		_exit(0);
+	}
+
+	await_child_success(pid);
+	return 0;
+}
+
+#define STACK_SIZE (1024 * 1024)
+
+static int hashchk_clone_child_fn(void *args)
+{
+	fill_hash_values();
+	return 0;
+}
+
+/*
+ * Check that threads share the same key so that existing hash values
+ * remain valid.
+ */
+static int hashchk_clone_share_key_test(void)
+{
+	void *child_stack;
+	pid_t pid;
+	int err;
+
+	if ((err = require_nphie()))
+		return err;
+
+	child_stack = mmap(NULL, STACK_SIZE, PROT_READ | PROT_WRITE,
+			   MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+
+	FAIL_IF_MSG(child_stack == MAP_FAILED, "failed to map child stack");
+
+	pid = clone(hashchk_clone_child_fn, child_stack + STACK_SIZE, CLONE_VM | SIGCHLD, NULL);
+
+	await_child_success(pid);
+	FAIL_IF_MSG(count_hash_values_matches() != HASH_COUNT, "different key detected");
+
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	int err = 0;
+
+	if (argc >= 1 && !strcmp(argv[0], "hashchk_exec_child"))
+		return hashchk_exec_child();
+
+	err |= test_harness(hashchk_enabled_test, "hashchk_enabled");
+	err |= test_harness(hashchk_exec_random_key_test, "hashchk_exec_random_key");
+	err |= test_harness(hashchk_fork_share_key_test, "hashchk_fork_share_key");
+	err |= test_harness(hashchk_clone_share_key_test, "hashchk_clone_share_key");
+
+	return err;
+}
diff --git a/tools/testing/selftests/powerpc/include/reg.h b/tools/testing/selftests/powerpc/include/reg.h
index d5a547f72669..cbb5979cb3e2 100644
--- a/tools/testing/selftests/powerpc/include/reg.h
+++ b/tools/testing/selftests/powerpc/include/reg.h
@@ -19,6 +19,8 @@
 #define mb()		asm volatile("sync" : : : "memory");
 #define barrier()	asm volatile("" : : : "memory");
 
+#define SPRN_HDEXCR    455
+
 #define SPRN_MMCR2     769
 #define SPRN_MMCRA     770
 #define SPRN_MMCR0     779
@@ -47,6 +49,8 @@
 #define SPRN_SDAR      781
 #define SPRN_SIER      768
 
+#define SPRN_DEXCR     812
+
 #define SPRN_TEXASR     0x82    /* Transaction Exception and Status Register */
 #define SPRN_TFIAR      0x81    /* Transaction Failure Inst Addr    */
 #define SPRN_TFHAR      0x80    /* Transaction Failure Handler Addr */
-- 
2.38.1


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

* [RFC PATCH 11/13] selftests/powerpc: Add DEXCR prctl, sysctl interface test
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (9 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 10/13] selftests/powerpc: Add hashst/hashchk test Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2022-11-28  2:44 ` [RFC PATCH 12/13] selftests/powerpc: Add DEXCR status utility lsdexcr Benjamin Gray
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

Test the prctl and sysctl interfaces of the DEXCR.

This adds a new capabilities util for getting and setting CAP_SYS_ADMIN.
Adding this avoids depending on an external libcap package. There is a
similar implementation (and reason) in the tools/testing/selftests/bpf
subtree but there's no obvious place to move it for sharing.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 .../selftests/powerpc/dexcr/.gitignore        |   1 +
 .../testing/selftests/powerpc/dexcr/Makefile  |   4 +-
 tools/testing/selftests/powerpc/dexcr/cap.c   |  72 ++++++
 tools/testing/selftests/powerpc/dexcr/cap.h   |  18 ++
 tools/testing/selftests/powerpc/dexcr/dexcr.h |   2 +
 .../selftests/powerpc/dexcr/dexcr_test.c      | 241 ++++++++++++++++++
 6 files changed, 336 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.c
 create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.h
 create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr_test.c

diff --git a/tools/testing/selftests/powerpc/dexcr/.gitignore b/tools/testing/selftests/powerpc/dexcr/.gitignore
index 37adb7f47832..035a1fcd8fb3 100644
--- a/tools/testing/selftests/powerpc/dexcr/.gitignore
+++ b/tools/testing/selftests/powerpc/dexcr/.gitignore
@@ -1 +1,2 @@
+dexcr_test
 hashchk_user
diff --git a/tools/testing/selftests/powerpc/dexcr/Makefile b/tools/testing/selftests/powerpc/dexcr/Makefile
index 4b4380d4d986..9814e72a4afa 100644
--- a/tools/testing/selftests/powerpc/dexcr/Makefile
+++ b/tools/testing/selftests/powerpc/dexcr/Makefile
@@ -1,4 +1,4 @@
-TEST_GEN_PROGS := hashchk_test
+TEST_GEN_PROGS := dexcr_test hashchk_test
 
 TEST_FILES := settings
 top_srcdir = ../../../../..
@@ -6,4 +6,4 @@ include ../../lib.mk
 
 HASHCHK_TEST_CFLAGS = -no-pie $(call cc-option,-mno-rop-protect)
 
-$(TEST_GEN_PROGS): ../harness.c ../utils.c ./dexcr.c
+$(TEST_GEN_PROGS): ../harness.c ../utils.c ./dexcr.c ./cap.c
diff --git a/tools/testing/selftests/powerpc/dexcr/cap.c b/tools/testing/selftests/powerpc/dexcr/cap.c
new file mode 100644
index 000000000000..3c9b1f27345d
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/cap.c
@@ -0,0 +1,72 @@
+#include <linux/capability.h>
+#include <string.h>
+#include <sys/syscall.h>
+
+#include "cap.h"
+#include "utils.h"
+
+struct kernel_capabilities {
+	struct __user_cap_header_struct header;
+
+	struct __user_cap_data_struct data[_LINUX_CAPABILITY_U32S_3];
+};
+
+static void get_caps(struct kernel_capabilities *caps)
+{
+	FAIL_IF_EXIT_MSG(syscall(SYS_capget, &caps->header, &caps->data),
+			 "cannot get capabilities");
+}
+
+static void set_caps(struct kernel_capabilities *caps)
+{
+	FAIL_IF_EXIT_MSG(syscall(SYS_capset, &caps->header, &caps->data),
+			 "cannot set capabilities");
+}
+
+static void init_caps(struct kernel_capabilities *caps, pid_t pid)
+{
+	memset(caps, 0, sizeof(*caps));
+
+	caps->header.version = _LINUX_CAPABILITY_VERSION_3;
+	caps->header.pid = pid;
+
+	get_caps(caps);
+}
+
+static bool has_cap(struct kernel_capabilities *caps, size_t cap)
+{
+	size_t data_index = cap / 32;
+	size_t offset = cap % 32;
+
+	FAIL_IF_EXIT_MSG(data_index >= ARRAY_SIZE(caps->data), "cap out of range");
+
+	return caps->data[data_index].effective & (1 << offset);
+}
+
+static void drop_cap(struct kernel_capabilities *caps, size_t cap)
+{
+	size_t data_index = cap / 32;
+	size_t offset = cap % 32;
+
+	FAIL_IF_EXIT_MSG(data_index >= ARRAY_SIZE(caps->data), "cap out of range");
+
+	caps->data[data_index].effective &= ~(1 << offset);
+}
+
+bool check_cap_sysadmin(void)
+{
+	struct kernel_capabilities caps;
+
+	init_caps(&caps, 0);
+
+	return has_cap(&caps, CAP_SYS_ADMIN);
+}
+
+void drop_cap_sysadmin(void)
+{
+	struct kernel_capabilities caps;
+
+	init_caps(&caps, 0);
+	drop_cap(&caps, CAP_SYS_ADMIN);
+	set_caps(&caps);
+}
diff --git a/tools/testing/selftests/powerpc/dexcr/cap.h b/tools/testing/selftests/powerpc/dexcr/cap.h
new file mode 100644
index 000000000000..41f41dda9862
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/cap.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Simple capabilities getter/setter
+ *
+ * This header file contains helper functions and macros
+ * required to get and set capabilities(7). Introduced so
+ * we aren't the first to rely on libcap.
+ */
+#ifndef _SELFTESTS_POWERPC_DEXCR_CAP_H
+#define _SELFTESTS_POWERPC_DEXCR_CAP_H
+
+#include <stdbool.h>
+
+bool check_cap_sysadmin(void);
+
+void drop_cap_sysadmin(void);
+
+#endif  /* _SELFTESTS_POWERPC_DEXCR_CAP_H */
diff --git a/tools/testing/selftests/powerpc/dexcr/dexcr.h b/tools/testing/selftests/powerpc/dexcr/dexcr.h
index fb8007bf19f8..b90633ae49e9 100644
--- a/tools/testing/selftests/powerpc/dexcr/dexcr.h
+++ b/tools/testing/selftests/powerpc/dexcr/dexcr.h
@@ -21,6 +21,8 @@
 #define DEXCR_PRO_SRAPD		DEXCR_PRO_MASK(4)
 #define DEXCR_PRO_NPHIE		DEXCR_PRO_MASK(5)
 
+#define SYSCTL_DEXCR_SBHE	"/proc/sys/kernel/speculative_branch_hint_enable"
+
 enum DexcrSource {
 	UDEXCR,		/* Userspace DEXCR value */
 	ENFORCED,	/* Enforced by hypervisor */
diff --git a/tools/testing/selftests/powerpc/dexcr/dexcr_test.c b/tools/testing/selftests/powerpc/dexcr/dexcr_test.c
new file mode 100644
index 000000000000..5446cb350c84
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/dexcr_test.c
@@ -0,0 +1,241 @@
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/prctl.h>
+#include <unistd.h>
+
+#include "cap.h"
+#include "dexcr.h"
+#include "utils.h"
+
+/*
+ * Test that an editable aspect
+ * - Current prctl state reported by the getter
+ * - Can be toggled on and off when process has CAP_SYS_ADMIN
+ * - Can't be edited if CAP_SYS_ADMIN not present
+ * - Can't be modified after force set
+ */
+static int dexcr_prctl_editable_aspect_test(unsigned long which)
+{
+	pid_t pid;
+
+	SKIP_IF_MSG(!check_cap_sysadmin(), "must have capability CAP_SYS_ADMIN");
+	SKIP_IF_MSG(!pr_aspect_supported(which), "aspect not supported");
+
+	FAIL_IF_MSG(!(pr_aspect_get(which) & PR_PPC_DEXCR_PRCTL), "aspect not editable");
+
+	FAIL_IF_MSG(!pr_aspect_edit(which, PR_PPC_DEXCR_CLEAR_ASPECT), "prctl failed");
+	FAIL_IF_MSG(pr_aspect_check(which, UDEXCR),
+		    "resetting aspect did not take effect");
+
+	FAIL_IF_MSG(pr_aspect_get(which) != (PR_PPC_DEXCR_CLEAR_ASPECT | PR_PPC_DEXCR_PRCTL),
+		    "prctl getter not reporting aspect state");
+
+	FAIL_IF_MSG(!pr_aspect_edit(which, PR_PPC_DEXCR_SET_ASPECT), "prctl failed");
+	FAIL_IF_MSG(!pr_aspect_check(which, UDEXCR),
+		    "setting aspect did not take effect");
+
+	FAIL_IF_MSG(pr_aspect_get(which) != (PR_PPC_DEXCR_SET_ASPECT | PR_PPC_DEXCR_PRCTL),
+		    "prctl getter not reporting aspect state");
+
+	FAIL_IF_MSG(!pr_aspect_edit(which, PR_PPC_DEXCR_CLEAR_ASPECT), "prctl failed");
+	FAIL_IF_MSG(pr_aspect_check(which, UDEXCR),
+		    "clearing aspect did not take effect");
+
+	FAIL_IF_MSG(pr_aspect_get(which) != (PR_PPC_DEXCR_CLEAR_ASPECT | PR_PPC_DEXCR_PRCTL),
+		    "prctl getter not reporting aspect state");
+
+	pid = fork();
+	if (pid == 0) {
+		drop_cap_sysadmin();
+		FAIL_IF_EXIT_MSG(pr_aspect_edit(which, PR_PPC_DEXCR_SET_ASPECT),
+				 "prctl success when nonprivileged");
+		FAIL_IF_EXIT_MSG(pr_aspect_check(which, UDEXCR),
+				 "edited aspect when nonprivileged");
+		_exit(0);
+	}
+	await_child_success(pid);
+
+	FAIL_IF_MSG(!pr_aspect_edit(which, PR_PPC_DEXCR_FORCE_SET_ASPECT), "prctl force set failed");
+	FAIL_IF_MSG(!pr_aspect_check(which, UDEXCR),
+		    "force setting aspect did not take effect");
+
+	FAIL_IF_MSG(pr_aspect_get(which) != (PR_PPC_DEXCR_FORCE_SET_ASPECT | PR_PPC_DEXCR_PRCTL),
+		    "prctl getter not reporting aspect state");
+
+	FAIL_IF_MSG(pr_aspect_edit(which, PR_PPC_DEXCR_CLEAR_ASPECT), "prctl success when forced");
+	FAIL_IF_MSG(!pr_aspect_check(which, UDEXCR),
+		    "edited aspect when forced");
+
+	return 0;
+}
+
+static int dexcr_prctl_sbhe_test(void)
+{
+	sysctl_set_sbhe(-1);
+	return dexcr_prctl_editable_aspect_test(PR_PPC_DEXCR_SBHE);
+}
+
+static int dexcr_prctl_ibrtpd_test(void)
+{
+	return dexcr_prctl_editable_aspect_test(PR_PPC_DEXCR_IBRTPD);
+}
+
+static int dexcr_prctl_srapd_test(void)
+{
+	return dexcr_prctl_editable_aspect_test(PR_PPC_DEXCR_SRAPD);
+}
+
+static int dexcr_sysctl_sbhe_test(void)
+{
+	SKIP_IF_MSG(!check_cap_sysadmin(), "must have capability CAP_SYS_ADMIN");
+	SKIP_IF_MSG(!pr_aspect_supported(PR_PPC_DEXCR_SBHE), "aspect not supported");
+
+	sysctl_set_sbhe(0);
+	FAIL_IF_MSG(sysctl_get_sbhe() != 0, "failed to clear sysctl SBHE");
+	FAIL_IF_MSG(pr_aspect_check(PR_PPC_DEXCR_SBHE, UDEXCR),
+		    "SBHE failed to clear");
+
+	sysctl_set_sbhe(1);
+	FAIL_IF_MSG(sysctl_get_sbhe() != 1, "failed to set sysctl SBHE");
+	FAIL_IF_MSG(!pr_aspect_check(PR_PPC_DEXCR_SBHE, UDEXCR),
+		    "SBHE failed to set");
+
+	sysctl_set_sbhe(-1);
+	FAIL_IF_MSG(sysctl_get_sbhe() != -1, "failed to default sysctl SBHE");
+	FAIL_IF_MSG(!pr_aspect_edit(PR_PPC_DEXCR_SBHE, PR_PPC_DEXCR_CLEAR_ASPECT), "prctl failed");
+	FAIL_IF_MSG(pr_aspect_check(PR_PPC_DEXCR_SBHE, UDEXCR),
+		    "SBHE failed to default to prctl clear setting");
+
+	FAIL_IF_MSG(!pr_aspect_edit(PR_PPC_DEXCR_SBHE, PR_PPC_DEXCR_SET_ASPECT), "prctl failed");
+	FAIL_IF_MSG(!pr_aspect_check(PR_PPC_DEXCR_SBHE, UDEXCR),
+		    "SBHE failed to default to prctl set setting");
+
+	sysctl_set_sbhe(0);
+	FAIL_IF_MSG(sysctl_get_sbhe() != 0, "failed to clear sysctl SBHE");
+	FAIL_IF_MSG(pr_aspect_check(PR_PPC_DEXCR_SBHE, UDEXCR),
+		    "SBHE failed to override prctl setting");
+
+	return 0;
+}
+
+static int dexcr_test_inherit_execve(char expected_dexcr)
+{
+	switch (expected_dexcr) {
+	case '0':
+		FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD) !=
+				 (PR_PPC_DEXCR_CLEAR_ASPECT | PR_PPC_DEXCR_PRCTL),
+				 "clearing IBRTPD across exec not inherited");
+
+		FAIL_IF_EXIT_MSG(pr_aspect_check(PR_PPC_DEXCR_IBRTPD, UDEXCR),
+				 "clearing IBRTPD across exec not applied");
+		break;
+	case '1':
+		FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD) !=
+				 (PR_PPC_DEXCR_SET_ASPECT | PR_PPC_DEXCR_PRCTL),
+				 "setting IBRTPD across exec not inherited");
+
+		FAIL_IF_EXIT_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD, UDEXCR),
+				 "setting IBRTPD across exec not applied");
+		break;
+	case '2':
+		FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD) !=
+				 (PR_PPC_DEXCR_FORCE_SET_ASPECT | PR_PPC_DEXCR_PRCTL),
+				 "force setting IBRTPD across exec not inherited");
+
+		FAIL_IF_EXIT_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD, UDEXCR),
+				 "force setting IBRTPD across exec not applied");
+		break;
+	}
+
+	return 0;
+}
+
+/*
+ * Check that a child process inherits the DEXCR over fork and execve
+ */
+static int dexcr_inherit_test(void)
+{
+	pid_t pid;
+
+	SKIP_IF_MSG(!check_cap_sysadmin(), "must have capability CAP_SYS_ADMIN");
+	SKIP_IF_MSG(!pr_aspect_supported(PR_PPC_DEXCR_IBRTPD), "IBRTPD not supported");
+
+	pr_aspect_edit(PR_PPC_DEXCR_IBRTPD, PR_PPC_DEXCR_CLEAR_ASPECT);
+	FAIL_IF_MSG(pr_aspect_check(PR_PPC_DEXCR_IBRTPD, UDEXCR),
+		    "IBRTPD failed to clear");
+
+	pid = fork();
+	if (pid == 0) {
+		char *args[] = { "dexcr_test_inherit_execve", "0", NULL };
+
+		FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD) !=
+				 (PR_PPC_DEXCR_CLEAR_ASPECT | PR_PPC_DEXCR_PRCTL),
+				 "clearing IBRTPD not inherited");
+
+		FAIL_IF_EXIT_MSG(pr_aspect_check(PR_PPC_DEXCR_IBRTPD, UDEXCR),
+				 "clearing IBRTPD not applied");
+
+		execve("/proc/self/exe", args, NULL);
+		_exit(errno);
+	}
+	await_child_success(pid);
+
+	pr_aspect_edit(PR_PPC_DEXCR_IBRTPD, PR_PPC_DEXCR_SET_ASPECT);
+	FAIL_IF_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD, UDEXCR),
+		    "IBRTPD failed to set");
+
+	pid = fork();
+	if (pid == 0) {
+		char *args[] = { "dexcr_test_inherit_execve", "1", NULL };
+
+		FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD) !=
+				 (PR_PPC_DEXCR_SET_ASPECT | PR_PPC_DEXCR_PRCTL),
+				 "setting IBRTPD not inherited");
+
+		FAIL_IF_EXIT_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD, UDEXCR),
+				 "setting IBRTPD not applied");
+
+		execve("/proc/self/exe", args, NULL);
+		_exit(errno);
+	}
+	await_child_success(pid);
+
+	pr_aspect_edit(PR_PPC_DEXCR_IBRTPD, PR_PPC_DEXCR_FORCE_SET_ASPECT);
+	FAIL_IF_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD, UDEXCR),
+		    "IBRTPD failed to force set");
+
+	pid = fork();
+	if (pid == 0) {
+		char *args[] = { "dexcr_test_inherit_execve", "2", NULL };
+
+		FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD) !=
+				 (PR_PPC_DEXCR_FORCE_SET_ASPECT | PR_PPC_DEXCR_PRCTL),
+				 "force setting IBRTPD not inherited");
+
+		FAIL_IF_EXIT_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD, UDEXCR),
+				 "force setting IBRTPD not applied");
+
+		execve("/proc/self/exe", args, NULL);
+		_exit(errno);
+	}
+	await_child_success(pid);
+
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	int err = 0;
+
+	if (argc >= 2 && strcmp(argv[0], "dexcr_test_inherit_execve") == 0)
+		return dexcr_test_inherit_execve(argv[1][0]);
+
+	err |= test_harness(dexcr_prctl_sbhe_test, "dexcr_prctl_sbhe");
+	err |= test_harness(dexcr_prctl_ibrtpd_test, "dexcr_prctl_ibrtpd");
+	err |= test_harness(dexcr_prctl_srapd_test, "dexcr_prctl_srapd");
+	err |= test_harness(dexcr_sysctl_sbhe_test, "dexcr_sysctl_sbhe");
+	err |= test_harness(dexcr_inherit_test, "dexcr_inherit");
+
+	return err;
+}
-- 
2.38.1


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

* [RFC PATCH 12/13] selftests/powerpc: Add DEXCR status utility lsdexcr
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (10 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 11/13] selftests/powerpc: Add DEXCR prctl, sysctl interface test Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2022-11-28  2:44 ` [RFC PATCH 13/13] Documentation: Document PowerPC kernel DEXCR interface Benjamin Gray
  2022-11-28  4:05 ` [RFC PATCH 00/13] Add DEXCR support Russell Currey
  13 siblings, 0 replies; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

Add a utility 'lsdexcr' to print the current DEXCR status. Useful for
quickly checking the status when debugging test failures, using the
sysctl interfaces manually, or just wanting to check it.

Example output:

          Requested: 84000000 (SBHE, NPHIE)
Hypervisor enforced: 00000000
          Effective: 84000000 (SBHE, NPHIE)

        SBHE * (0): set, prctl editable  	(Speculative branch hint enable)
      IBRTPD   (3): clear, prctl editable  	(Indirect branch recurrent target prediction disable)
       SRAPD   (4): clear, prctl editable  	(Subroutine return address prediction disable)
       NPHIE * (5): set  	(Non-privileged hash instruction enable)

Global SBHE override: 1 (set)

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 .../selftests/powerpc/dexcr/.gitignore        |   1 +
 .../testing/selftests/powerpc/dexcr/Makefile  |   2 +
 .../testing/selftests/powerpc/dexcr/lsdexcr.c | 178 ++++++++++++++++++
 3 files changed, 181 insertions(+)
 create mode 100644 tools/testing/selftests/powerpc/dexcr/lsdexcr.c

diff --git a/tools/testing/selftests/powerpc/dexcr/.gitignore b/tools/testing/selftests/powerpc/dexcr/.gitignore
index 035a1fcd8fb3..7dd2fad93732 100644
--- a/tools/testing/selftests/powerpc/dexcr/.gitignore
+++ b/tools/testing/selftests/powerpc/dexcr/.gitignore
@@ -1,2 +1,3 @@
 dexcr_test
 hashchk_user
+lsdexcr
diff --git a/tools/testing/selftests/powerpc/dexcr/Makefile b/tools/testing/selftests/powerpc/dexcr/Makefile
index 9814e72a4afa..8cb732cda7e7 100644
--- a/tools/testing/selftests/powerpc/dexcr/Makefile
+++ b/tools/testing/selftests/powerpc/dexcr/Makefile
@@ -1,4 +1,5 @@
 TEST_GEN_PROGS := dexcr_test hashchk_test
+TEST_GEN_FILES := lsdexcr
 
 TEST_FILES := settings
 top_srcdir = ../../../../..
@@ -7,3 +8,4 @@ include ../../lib.mk
 HASHCHK_TEST_CFLAGS = -no-pie $(call cc-option,-mno-rop-protect)
 
 $(TEST_GEN_PROGS): ../harness.c ../utils.c ./dexcr.c ./cap.c
+$(TEST_GEN_FILES): ../utils.c ./dexcr.c
diff --git a/tools/testing/selftests/powerpc/dexcr/lsdexcr.c b/tools/testing/selftests/powerpc/dexcr/lsdexcr.c
new file mode 100644
index 000000000000..c9f0035f8e2e
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/lsdexcr.c
@@ -0,0 +1,178 @@
+#include <errno.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/prctl.h>
+
+#include "dexcr.h"
+#include "utils.h"
+
+static unsigned int requested;
+static unsigned int enforced;
+static unsigned int effective;
+
+struct dexcr_aspect {
+	const char *name;
+	const char *desc;
+	unsigned int index;
+	unsigned long pr_val;
+};
+
+static const struct dexcr_aspect aspects[] = {
+	{
+		.name = "SBHE",
+		.desc = "Speculative branch hint enable",
+		.index = 0,
+		.pr_val = PR_PPC_DEXCR_SBHE,
+	},
+	{
+		.name = "IBRTPD",
+		.desc = "Indirect branch recurrent target prediction disable",
+		.index = 3,
+		.pr_val = PR_PPC_DEXCR_IBRTPD,
+	},
+	{
+		.name = "SRAPD",
+		.desc = "Subroutine return address prediction disable",
+		.index = 4,
+		.pr_val = PR_PPC_DEXCR_SRAPD,
+	},
+	{
+		.name = "NPHIE",
+		.desc = "Non-privileged hash instruction enable",
+		.index = 5,
+		.pr_val = PR_PPC_DEXCR_NPHIE,
+	},
+};
+
+#define NUM_ASPECTS (sizeof(aspects) / sizeof(struct dexcr_aspect))
+
+static void print_list(const char *list[], size_t len)
+{
+	for (size_t i = 0; i < len; i++) {
+		printf("%s", list[i]);
+		if (i + 1 < len)
+			printf(", ");
+	}
+}
+
+static void print_dexcr(char *name, unsigned int bits)
+{
+	const char *enabled_aspects[32] = {NULL};
+	size_t j = 0;
+
+	printf("%s: %08x", name, bits);
+
+	if (bits == 0) {
+		printf("\n");
+		return;
+	}
+
+	for (size_t i = 0; i < NUM_ASPECTS; i++) {
+		unsigned int mask = pr_aspect_to_dexcr_mask(aspects[i].pr_val);
+		if (bits & mask) {
+			enabled_aspects[j++] = aspects[i].name;
+			bits &= ~mask;
+		}
+	}
+
+	if (bits)
+		enabled_aspects[j++] = "unknown";
+
+	printf(" (");
+	print_list(enabled_aspects, j);
+	printf(")\n");
+}
+
+static void print_aspect(const struct dexcr_aspect *aspect)
+{
+	const char *attributes[32] = {NULL};
+	size_t j = 0;
+	unsigned long mask;
+	int pr_status;
+
+	/* Kernel-independent info about aspect */
+	mask = pr_aspect_to_dexcr_mask(aspect->pr_val);
+	if (requested & mask)
+		attributes[j++] = "set";
+	if (enforced & mask)
+		attributes[j++] = "hypervisor enforced";
+	if (!(effective & mask))
+		attributes[j++] = "clear";
+
+	/* Kernel understanding of the aspect */
+	pr_status = prctl(PR_PPC_GET_DEXCR, aspect->pr_val, 0, 0, 0);
+	if (pr_status == -1) {
+		switch (errno) {
+		case ENODEV:
+			attributes[j++] = "aspect not present";
+			break;
+		case EINVAL:
+			attributes[j++] = "unrecognised aspect";
+			break;
+		default:
+			attributes[j++] = "unknown kernel error";
+			break;
+		}
+	} else {
+		if (pr_status & PR_PPC_DEXCR_SET_ASPECT)
+			attributes[j++] = "prctl set";
+		if (pr_status & PR_PPC_DEXCR_FORCE_SET_ASPECT)
+			attributes[j++] = "prctl force set";
+		if (pr_status & PR_PPC_DEXCR_CLEAR_ASPECT)
+			attributes[j++] = "prctl clear";
+		if (pr_status & PR_PPC_DEXCR_PRCTL)
+			attributes[j++] = "prctl editable";
+	}
+
+	printf("%12s %c (%d): ", aspect->name, effective & mask ? '*' : ' ', aspect->index);
+	print_list(attributes, j);
+	printf("  \t(%s)\n", aspect->desc);
+}
+
+static void print_overrides(void) {
+	long sbhe;
+	int err;
+
+	printf("Global SBHE override: ");
+	if ((err = read_long(SYSCTL_DEXCR_SBHE, &sbhe, 10))) {
+		printf("error reading " SYSCTL_DEXCR_SBHE ": %d (%s)\n", err, strerror(err));
+	} else {
+		const char *meaning;
+		switch (sbhe) {
+		case -1:
+			meaning = "default";
+			break;
+		case 0:
+			meaning = "clear";
+			break;
+		case 1:
+			meaning = "set";
+			break;
+		default:
+			meaning = "unknown";
+		}
+
+		printf("%ld (%s)\n", sbhe, meaning);
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	requested = get_dexcr(UDEXCR);
+	enforced = get_dexcr(ENFORCED);
+	effective = requested | enforced;
+
+	print_dexcr("          Requested", requested);
+	print_dexcr("Hypervisor enforced", enforced);
+	print_dexcr("          Effective", effective);
+	printf("\n");
+
+	for (size_t i = 0; i < NUM_ASPECTS; i++)
+		print_aspect(&aspects[i]);
+	printf("\n");
+
+	print_overrides();
+
+	return 0;
+}
-- 
2.38.1


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

* [RFC PATCH 13/13] Documentation: Document PowerPC kernel DEXCR interface
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (11 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 12/13] selftests/powerpc: Add DEXCR status utility lsdexcr Benjamin Gray
@ 2022-11-28  2:44 ` Benjamin Gray
  2023-03-07  5:40   ` Nicholas Piggin
  2022-11-28  4:05 ` [RFC PATCH 00/13] Add DEXCR support Russell Currey
  13 siblings, 1 reply; 29+ messages in thread
From: Benjamin Gray @ 2022-11-28  2:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: ajd, linux-kernel, linux-hardening, cmr, Benjamin Gray

Describe the DEXCR and document how to interact with it via the
prctl and sysctl interfaces.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 Documentation/powerpc/dexcr.rst | 183 ++++++++++++++++++++++++++++++++
 Documentation/powerpc/index.rst |   1 +
 2 files changed, 184 insertions(+)
 create mode 100644 Documentation/powerpc/dexcr.rst

diff --git a/Documentation/powerpc/dexcr.rst b/Documentation/powerpc/dexcr.rst
new file mode 100644
index 000000000000..3c995f4b9fe0
--- /dev/null
+++ b/Documentation/powerpc/dexcr.rst
@@ -0,0 +1,183 @@
+==========================================
+DEXCR (Dynamic Execution Control Register)
+==========================================
+
+Overview
+========
+
+The DEXCR is a privileged special purpose register (SPR) introduced in
+PowerPC ISA 3.1B (Power10) that allows per-cpu control over several dynamic
+execution behaviours. These behaviours include speculation (e.g., indirect
+branch target prediction) and enabling return-oriented programming (ROP)
+protection instructions.
+
+The execution control is exposed in hardware as up to 32 bits ('aspects') in
+the DEXCR. Each aspect controls a certain behaviour, and can be set or cleared
+to enable/disable the aspect. There are several variants of the DEXCR for
+different purposes:
+
+DEXCR
+    A priviliged SPR that can control aspects for userspace and kernel space
+HDEXCR
+    A hypervisor-privileged SPR that can control aspects for the hypervisor and
+    enforce aspects for the kernel and userspace.
+UDEXCR
+    An optional ultravisor-privileged SPR that can control aspects for the ultravisor.
+
+Userspace can examine the current DEXCR state using a dedicated SPR that
+provides a non-privileged read-only view of the userspace DEXCR aspects.
+There is also an SPR that provides a read-only view of the hypervisor enforced
+aspects, which ORed with the userspace DEXCR view gives the effective DEXCR
+state for a process.
+
+
+User API
+========
+
+prctl()
+-------
+
+A process can control its own userspace DEXCR value using the
+``PR_PPC_GET_DEXCR`` and ``PR_PPC_SET_DEXCR`` pair of
+:manpage:`prctl(2)` commands. These calls have the form::
+
+    prctl(PR_PPC_GET_DEXCR, unsigned long aspect, 0, 0, 0);
+    prctl(PR_PPC_SET_DEXCR, unsigned long aspect, unsigned long flags, 0, 0);
+
+Where ``aspect`` (``arg1``) is a constant and ``flags`` (``arg2``) is a bifield.
+The possible aspect and flag values are as follows. Note there is no relation
+between aspect value and ``prctl()`` constant value.
+
+.. flat-table::
+   :header-rows: 1
+   :widths: 2 7 1
+
+   * - ``prctl()`` constant
+     - Aspect name
+     - Aspect bit
+
+   * - ``PR_PPC_DEXCR_SBHE``
+     - Speculative Branch Hint Enable (SBHE)
+     - 0
+
+   * - ``PR_PPC_DEXCR_IBRTPD``
+     - Indirect Branch Recurrent Target Prediction Disable (IBRTPD)
+     - 3
+
+   * - ``PR_PPC_DEXCR_SRAPD``
+     - Subroutine Return Address Prediction Disable (SRAPD)
+     - 4
+
+   * - ``PR_PPC_DEXCR_NPHIE``
+     - Non-Privileged Hash Instruction Enable (NPHIE)
+     - 5
+
+.. flat-table::
+   :header-rows: 1
+   :widths: 2 8
+
+   * - ``prctl()`` flag
+     - Meaning
+
+   * - ``PR_PPC_DEXCR_PRCTL``
+     - This aspect can be configured with ``prctl(PR_PPC_SET_DEXCR, ...)``
+
+   * - ``PR_PPC_DEXCR_SET_ASPECT``
+     - This aspect is set
+
+   * - ``PR_PPC_DEXCR_FORCE_SET_ASPECT``
+     - This aspect is set and cannot be undone. A subsequent
+       ``prctl(..., PR_PPC_DEXCR_CLEAR_ASPECT)`` will fail.
+
+   * - ``PR_PPC_DEXCR_CLEAR_ASPECT``
+     - This aspect is clear
+
+Note that
+
+* The ``*_SET_ASPECT`` / ``*_CLEAR_ASPECT`` refers to setting/clearing the bit in the DEXCR.
+  For example::
+
+      prctl(PR_PPC_SET_DEXCR, PR_PPC_DEXCR_IBRTPD, PR_PPC_DEXCR_SET_ASPECT, 0, 0);
+
+  will set the IBRTPD aspect bit in the DEXCR, causing indirect branch prediction
+  to be disabled.
+
+* The status returned by ``PR_PPC_GET_DEXCR`` does not include any alternative
+  config overrides. To see the true DEXCR state software should read the appropriate
+  SPRs directly.
+
+* A forced aspect will still report ``PR_PPC_DEXCR_PRCTL`` if it would
+  otherwise be editable.
+
+* The aspect state when starting a process is copied from the parent's
+  state on :manpage:`fork(2)` and :manpage:`execve(2)`. Aspects may also be set
+  or cleared by the kernel on process creation.
+
+Use ``PR_PPC_SET_DEXCR`` with one of ``PR_PPC_DEXCR_SET_ASPECT``,
+``PR_PPC_DEXCR_FORCE_SET_ASPECT``, or ``PR_PPC_DEXCR_CLEAR_ASPECT`` to edit a
+ given aspect.
+
+Common error codes for both getting and setting the DEXCR are as follows:
+
+.. flat-table::
+   :header-rows: 1
+   :widths: 2 8
+
+   * - Error
+     - Meaning
+
+   * - ``EINVAL``
+     - The DEXCR is not supported by the kernel.
+
+   * - ``ENODEV``
+     - The aspect is not recognised by the kernel or not supported by the hardware.
+
+``PR_PPC_SET_DEXCR`` may also report the following error codes:
+
+.. flat-table::
+   :header-rows: 1
+   :widths: 2 8
+
+   * - Error
+     - Meaning
+
+   * - ``ERANGE``
+     - ``arg2`` is incorrect. E.g., it does not select an action (set/clear),
+       or the flags are not recognised by the kernel.
+
+   * - ``ENXIO``
+     - The aspect is not editable via ``prctl()``.
+
+   * - ``EPERM``
+     - The process does not have sufficient privilege to modify this aspect,
+       or the aspect has been force set and cannot be modified.
+
+
+sysctl
+------
+
+Some aspects can be modified globally via :manpage:`sysctl(8)` entries. Such global
+modifications are applied after any process modifications. Any ``prctl()`` call to
+an overridden aspect this aspect may still report it as editable. The prctl setting
+will take effect again if the global override is restored to its default state.
+
+A global SBHE config is exposed in ``/proc/sys/kernel/speculative_branch_hint_enable``.
+Any process can read the current config value from it. Privileged processes can
+write to it to change the config. The new config is applied to all current and future
+processes (though note the kernel cannot override any hypervisor enforced aspects).
+
+.. flat-table::
+   :header-rows: 1
+   :widths: 2 8
+
+   * - Value
+     - Meaning
+
+   * - ``-1``
+     - Do not change from default or ``prctl()`` config.
+
+   * - ``0``
+     - Force clear aspect.
+
+   * - ``1``
+     - Force set aspect.
diff --git a/Documentation/powerpc/index.rst b/Documentation/powerpc/index.rst
index 85e80e30160b..d33b554ca7ba 100644
--- a/Documentation/powerpc/index.rst
+++ b/Documentation/powerpc/index.rst
@@ -15,6 +15,7 @@ powerpc
     cxl
     cxlflash
     dawr-power9
+    dexcr
     dscr
     eeh-pci-error-recovery
     elf_hwcaps
-- 
2.38.1


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

* Re: [RFC PATCH 00/13] Add DEXCR support
  2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
                   ` (12 preceding siblings ...)
  2022-11-28  2:44 ` [RFC PATCH 13/13] Documentation: Document PowerPC kernel DEXCR interface Benjamin Gray
@ 2022-11-28  4:05 ` Russell Currey
  13 siblings, 0 replies; 29+ messages in thread
From: Russell Currey @ 2022-11-28  4:05 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Mon, 2022-11-28 at 13:44 +1100, Benjamin Gray wrote:
> This series is based on initial work by Chris Riedl that was not sent
> to the list.
> 
> Adds a kernel interface for userspace to interact with the DEXCR.
> The DEXCR is a SPR that allows control over various execution
> 'aspects', such as indirect branch prediction and enabling the
> hashst/hashchk instructions. Further details are in ISA 3.1B
> Book 3 chapter 12.
> 
> This RFC proposes an interface for users to interact with the DEXCR.
> It aims to support
> 
> * Querying supported aspects
> * Getting/setting aspects on a per-process level
> * Allowing global overrides across all processes
> 
> There are some parts that I'm not sure on the best way to approach
> (hence RFC):
> 
> * The feature names in arch/powerpc/kernel/dt_cpu_ftrs.c appear to be
> unimplemented
>   in skiboot, so are being defined by this series. Is being so
> verbose fine?

These are going to need to be added to skiboot before they can be
referenced in the kernel.  Inclusion in skiboot makes them ABI, the
kernel is just a consumer.

> * What aspects should be editable by a process? E.g., SBHE has
>   effects that potentially bleed into other processes. Should
>   it only be system wide configurable?

For context, ISA 3.1B p1358 says: 

   In some micro-architectures, the execution behav-
   ior controlled by aspect 0 is difficult to change with
   any degree of timing precision. The change may
   also bleed over into other threads on the same pro-
   cessor. Any environment that has a dependence on
   the more secure setting of aspect 0 should not
   change the value, and ideally should share a pro-
   cessor only with similar threads. For other environ-
   ments, changes to the effective value of aspect 0
   represent a relative risk tolerance for its aspect of
   execution behavior, with the understanding that
   there will be significant hysteresis in the execution
   behavior.
   
If a process sets SBHE for itself and all it takes is context switching
from a process with SBHE unset to cause exposure, then yeah I think it
should just be global.  I doubt branch hints have enough impact for
process granularity to be especially desirable anyway.

> * Should configuring certain aspects for the process be non-
> privileged? E.g.,
>   Is there harm in always allowing configuration of IBRTPD, SRAPD?
> The *FORCE_SET*
>   action prevents further process local changes regardless of
> privilege.

I'm not aware of a reason why it would be a problem to allow
unprivileged configuration as long as there's a way to prevent further
changes.  The concerning case is if a mitigation is set by a trusted
process context, and then untrusted code is executed that manages to
turn the mitigation off again.

> * The tests fail Patchwork CI because of the new prctl macros, and
> the CI
>   doesn't run headers_install and add -isystem
> <buildpath>/usr/include to
>   the make command.

The CI runs on x86 and cross compiles the kernel and selftests, and
boots are done in qemu tcg.  Maybe we can skip the build if the symbols
are undefined or do something like

	#ifndef PR_PPC_DEXCR_...
		return KSFT_SKIP;
	#endif

in the test itself?

> * On handling an exception, I don't check if the NPHIE bit is enabled
> in the DEXCR.
>   To do so would require reading both the DEXCR and HDEXCR, for
> little gain (it
>   should only matter that the current instruction was a hashchk. If
> so, the only
>   reason it would cause an exception is the failed check. If the
> instruction is
>   rewritten between exception and check we'd be wrong anyway).

For context, the hashst and hashchk instructions are implemented using
previously reserved nops.  I'm not aware of any reason a nop could trap
(i.e. we could check for a trap that came from hashchk even if NPHIE is
not set), but afaik that'd be the only reason we would have to check.

> 
> The series is based on the earlier selftest utils series[1], so the
> tests won't build
> at all without applying that first. The kernel side should build fine
> on ppc/next
> 247f34f7b80357943234f93f247a1ae6b6c3a740 though.
> 
> [1]:
> https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20221122231103.15829-1-bgray@linux.ibm.com/
> 
> Benjamin Gray (13):
>   powerpc/book3s: Add missing <linux/sched.h> include
>   powerpc: Add initial Dynamic Execution Control Register (DEXCR)
>     support
>   powerpc/dexcr: Handle hashchk exception
>   powerpc/dexcr: Support userspace ROP protection
>   prctl: Define PowerPC DEXCR interface
>   powerpc/dexcr: Add prctl implementation
>   powerpc/dexcr: Add sysctl entry for SBHE system override
>   powerpc/dexcr: Add enforced userspace ROP protection config
>   selftests/powerpc: Add more utility macros
>   selftests/powerpc: Add hashst/hashchk test
>   selftests/powerpc: Add DEXCR prctl, sysctl interface test
>   selftests/powerpc: Add DEXCR status utility lsdexcr
>   Documentation: Document PowerPC kernel DEXCR interface
> 
>  Documentation/powerpc/dexcr.rst               | 183 +++++++++++
>  Documentation/powerpc/index.rst               |   1 +
>  arch/powerpc/Kconfig                          |   5 +
>  arch/powerpc/include/asm/book3s/64/kexec.h    |   6 +
>  arch/powerpc/include/asm/book3s/64/kup.h      |   1 +
>  arch/powerpc/include/asm/cputable.h           |   8 +-
>  arch/powerpc/include/asm/ppc-opcode.h         |   1 +
>  arch/powerpc/include/asm/processor.h          |  33 ++
>  arch/powerpc/include/asm/reg.h                |   7 +
>  arch/powerpc/kernel/Makefile                  |   1 +
>  arch/powerpc/kernel/dexcr.c                   | 310
> ++++++++++++++++++
>  arch/powerpc/kernel/dt_cpu_ftrs.c             |   4 +
>  arch/powerpc/kernel/process.c                 |  31 +-
>  arch/powerpc/kernel/prom.c                    |   4 +
>  arch/powerpc/kernel/traps.c                   |   6 +
>  include/uapi/linux/prctl.h                    |  14 +
>  kernel/sys.c                                  |  16 +
>  tools/testing/selftests/powerpc/Makefile      |   1 +
>  .../selftests/powerpc/dexcr/.gitignore        |   3 +
>  .../testing/selftests/powerpc/dexcr/Makefile  |  11 +
>  tools/testing/selftests/powerpc/dexcr/cap.c   |  72 ++++
>  tools/testing/selftests/powerpc/dexcr/cap.h   |  18 +
>  tools/testing/selftests/powerpc/dexcr/dexcr.c | 118 +++++++
>  tools/testing/selftests/powerpc/dexcr/dexcr.h |  54 +++
>  .../selftests/powerpc/dexcr/dexcr_test.c      | 241 ++++++++++++++
>  .../selftests/powerpc/dexcr/hashchk_test.c    | 229 +++++++++++++
>  .../testing/selftests/powerpc/dexcr/lsdexcr.c | 178 ++++++++++
>  tools/testing/selftests/powerpc/include/reg.h |   4 +
>  .../testing/selftests/powerpc/include/utils.h |  44 +++
>  29 files changed, 1602 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/powerpc/dexcr.rst
>  create mode 100644 arch/powerpc/kernel/dexcr.c
>  create mode 100644 tools/testing/selftests/powerpc/dexcr/.gitignore
>  create mode 100644 tools/testing/selftests/powerpc/dexcr/Makefile
>  create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.c
>  create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.h
>  create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr.c
>  create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr.h
>  create mode 100644
> tools/testing/selftests/powerpc/dexcr/dexcr_test.c
>  create mode 100644
> tools/testing/selftests/powerpc/dexcr/hashchk_test.c
>  create mode 100644 tools/testing/selftests/powerpc/dexcr/lsdexcr.c
> 
> 
> base-commit: 9dc58a6040662faaf24c8932861f485670fce7ff
> --
> 2.38.1


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

* Re: [RFC PATCH 03/13] powerpc/dexcr: Handle hashchk exception
  2022-11-28  2:44 ` [RFC PATCH 03/13] powerpc/dexcr: Handle hashchk exception Benjamin Gray
@ 2022-11-29 10:39   ` Nicholas Piggin
  2022-11-29 22:04     ` Benjamin Gray
  0 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-11-29 10:39 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> Recognise and pass the appropriate signal to the user program when a
> hashchk instruction triggers. This is independent of allowing
> configuration of DEXCR[NPHIE], as a hypervisor can enforce this aspect
> regardless of the kernel.
>
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/ppc-opcode.h |  1 +
>  arch/powerpc/include/asm/processor.h  |  6 ++++++
>  arch/powerpc/kernel/dexcr.c           | 22 ++++++++++++++++++++++
>  arch/powerpc/kernel/traps.c           |  6 ++++++
>  4 files changed, 35 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 21e33e46f4b8..89b316466ed1 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -215,6 +215,7 @@
>  #define OP_31_XOP_STFSX	    663
>  #define OP_31_XOP_STFSUX    695
>  #define OP_31_XOP_STFDX     727
> +#define OP_31_XOP_HASHCHK   754
>  #define OP_31_XOP_STFDUX    759
>  #define OP_31_XOP_LHBRX     790
>  #define OP_31_XOP_LFIWAX    855
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 0a8a793b8b8b..c17ec1e44c86 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -448,10 +448,16 @@ void *exit_vmx_ops(void *dest);
>  
>  #ifdef CONFIG_PPC_BOOK3S_64
>  
> +bool is_hashchk_trap(struct pt_regs const *regs);
>  unsigned long get_thread_dexcr(struct thread_struct const *t);
>  
>  #else
>  
> +static inline bool is_hashchk_trap(struct pt_regs const *regs)
> +{
> +	return false;
> +}
> +
>  static inline unsigned long get_thread_dexcr(struct thread_struct const *t)
>  {
>  	return 0;
> diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
> index 32a0a69ff638..11515e67afac 100644
> --- a/arch/powerpc/kernel/dexcr.c
> +++ b/arch/powerpc/kernel/dexcr.c
> @@ -3,6 +3,9 @@
>  
>  #include <asm/cpu_has_feature.h>
>  #include <asm/cputable.h>
> +#include <asm/disassemble.h>
> +#include <asm/inst.h>
> +#include <asm/ppc-opcode.h>
>  #include <asm/processor.h>
>  #include <asm/reg.h>
>  
> @@ -19,6 +22,25 @@ static int __init dexcr_init(void)
>  }
>  early_initcall(dexcr_init);
>  
> +bool is_hashchk_trap(struct pt_regs const *regs)
> +{
> +	ppc_inst_t insn;
> +
> +	if (!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
> +		return false;
> +
> +	if (get_user_instr(insn, (void __user *)regs->nip)) {
> +		WARN_ON(1);
> +		return false;
> +	}

Nice series, just starting to have a look at it.

You probably don't want a WARN_ON() here because it's user triggerable
and isn't necessarily even indiciating a problem or attack if the app
is doing code unmapping in order to get faults.

Check some of the other instruction emulation for what to do in case of
an EFAULT.

> +
> +	if (ppc_inst_primary_opcode(insn) == 31 &&
> +	    get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK)
> +		return true;
> +
> +	return false;
> +}
> +
>  unsigned long get_thread_dexcr(struct thread_struct const *t)
>  {
>  	return DEFAULT_DEXCR;
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 9bdd79aa51cf..b83f5b382f24 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1516,6 +1516,12 @@ static void do_program_check(struct pt_regs *regs)
>  				return;
>  			}
>  		}
> +
> +		if (user_mode(regs) && is_hashchk_trap(regs)) {
> +			_exception(SIGILL, regs, ILL_ILLOPN, regs->nip);
> +			return;
> +		}

I guess ILLOPN makes sense. Do you know if any other archs do similar?

Thanks,
Nick

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

* Re: [RFC PATCH 03/13] powerpc/dexcr: Handle hashchk exception
  2022-11-29 10:39   ` Nicholas Piggin
@ 2022-11-29 22:04     ` Benjamin Gray
  0 siblings, 0 replies; 29+ messages in thread
From: Benjamin Gray @ 2022-11-29 22:04 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Tue, 2022-11-29 at 20:39 +1000, Nicholas Piggin wrote:
> On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> > Recognise and pass the appropriate signal to the user program when
> > a
> > hashchk instruction triggers. This is independent of allowing
> > configuration of DEXCR[NPHIE], as a hypervisor can enforce this
> > aspect
> > regardless of the kernel.
> > 
> > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> > ---
> >  arch/powerpc/include/asm/ppc-opcode.h |  1 +
> >  arch/powerpc/include/asm/processor.h  |  6 ++++++
> >  arch/powerpc/kernel/dexcr.c           | 22 ++++++++++++++++++++++
> >  arch/powerpc/kernel/traps.c           |  6 ++++++
> >  4 files changed, 35 insertions(+)
> > 
> > diff --git a/arch/powerpc/include/asm/ppc-opcode.h
> > b/arch/powerpc/include/asm/ppc-opcode.h
> > index 21e33e46f4b8..89b316466ed1 100644
> > --- a/arch/powerpc/include/asm/ppc-opcode.h
> > +++ b/arch/powerpc/include/asm/ppc-opcode.h
> > @@ -215,6 +215,7 @@
> >  #define OP_31_XOP_STFSX            663
> >  #define OP_31_XOP_STFSUX    695
> >  #define OP_31_XOP_STFDX     727
> > +#define OP_31_XOP_HASHCHK   754
> >  #define OP_31_XOP_STFDUX    759
> >  #define OP_31_XOP_LHBRX     790
> >  #define OP_31_XOP_LFIWAX    855
> > diff --git a/arch/powerpc/include/asm/processor.h
> > b/arch/powerpc/include/asm/processor.h
> > index 0a8a793b8b8b..c17ec1e44c86 100644
> > --- a/arch/powerpc/include/asm/processor.h
> > +++ b/arch/powerpc/include/asm/processor.h
> > @@ -448,10 +448,16 @@ void *exit_vmx_ops(void *dest);
> >  
> >  #ifdef CONFIG_PPC_BOOK3S_64
> >  
> > +bool is_hashchk_trap(struct pt_regs const *regs);
> >  unsigned long get_thread_dexcr(struct thread_struct const *t);
> >  
> >  #else
> >  
> > +static inline bool is_hashchk_trap(struct pt_regs const *regs)
> > +{
> > +       return false;
> > +}
> > +
> >  static inline unsigned long get_thread_dexcr(struct thread_struct
> > const *t)
> >  {
> >         return 0;
> > diff --git a/arch/powerpc/kernel/dexcr.c
> > b/arch/powerpc/kernel/dexcr.c
> > index 32a0a69ff638..11515e67afac 100644
> > --- a/arch/powerpc/kernel/dexcr.c
> > +++ b/arch/powerpc/kernel/dexcr.c
> > @@ -3,6 +3,9 @@
> >  
> >  #include <asm/cpu_has_feature.h>
> >  #include <asm/cputable.h>
> > +#include <asm/disassemble.h>
> > +#include <asm/inst.h>
> > +#include <asm/ppc-opcode.h>
> >  #include <asm/processor.h>
> >  #include <asm/reg.h>
> >  
> > @@ -19,6 +22,25 @@ static int __init dexcr_init(void)
> >  }
> >  early_initcall(dexcr_init);
> >  
> > +bool is_hashchk_trap(struct pt_regs const *regs)
> > +{
> > +       ppc_inst_t insn;
> > +
> > +       if (!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
> > +               return false;
> > +
> > +       if (get_user_instr(insn, (void __user *)regs->nip)) {
> > +               WARN_ON(1);
> > +               return false;
> > +       }
> 
> Nice series, just starting to have a look at it.
> 
> You probably don't want a WARN_ON() here because it's user
> triggerable
> and isn't necessarily even indiciating a problem or attack if the app
> is doing code unmapping in order to get faults.
> 
> Check some of the other instruction emulation for what to do in case
> of
> an EFAULT.

Alright, I'll take a look

> > +
> > +       if (ppc_inst_primary_opcode(insn) == 31 &&
> > +           get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK)
> > +               return true;
> > +
> > +       return false;
> > +}
> > +
> >  unsigned long get_thread_dexcr(struct thread_struct const *t)
> >  {
> >         return DEFAULT_DEXCR;
> > diff --git a/arch/powerpc/kernel/traps.c
> > b/arch/powerpc/kernel/traps.c
> > index 9bdd79aa51cf..b83f5b382f24 100644
> > --- a/arch/powerpc/kernel/traps.c
> > +++ b/arch/powerpc/kernel/traps.c
> > @@ -1516,6 +1516,12 @@ static void do_program_check(struct pt_regs
> > *regs)
> >                                 return;
> >                         }
> >                 }
> > +
> > +               if (user_mode(regs) && is_hashchk_trap(regs)) {
> > +                       _exception(SIGILL, regs, ILL_ILLOPN, regs-
> > >nip);
> > +                       return;
> > +               }
> 
> I guess ILLOPN makes sense. Do you know if any other archs do
> similar?

Ah sorry, when refactoring Chris' patches I forgot to put back in the
commit message that this is how ARM reports their similar check
failure. For example, their FPAC handler in
arch/arm64/kernel/traps.c:518 does this.

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

* Re: [RFC PATCH 01/13] powerpc/book3s: Add missing <linux/sched.h> include
  2022-11-28  2:44 ` [RFC PATCH 01/13] powerpc/book3s: Add missing <linux/sched.h> include Benjamin Gray
@ 2023-03-07  4:28   ` Nicholas Piggin
  0 siblings, 0 replies; 29+ messages in thread
From: Nicholas Piggin @ 2023-03-07  4:28 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> The functions here use struct thread_struct fields, so need to import
> the full definition from <linux/sched.h>. The <asm/current.h> header
> that defines current only forward declares struct thread_struct.
>
> Failing to include this <linux/sched.h> header leads to a compilation
> error when a translation unit does not also include <linux/sched.h>
> indirectly.
>
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>

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

> ---
>  arch/powerpc/include/asm/book3s/64/kup.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
> index 54cf46808157..84c09e546115 100644
> --- a/arch/powerpc/include/asm/book3s/64/kup.h
> +++ b/arch/powerpc/include/asm/book3s/64/kup.h
> @@ -194,6 +194,7 @@
>  #else /* !__ASSEMBLY__ */
>  
>  #include <linux/jump_label.h>
> +#include <linux/sched.h>
>  
>  DECLARE_STATIC_KEY_FALSE(uaccess_flush_key);
>  
> -- 
> 2.38.1


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

* Re: [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support
  2022-11-28  2:44 ` [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support Benjamin Gray
@ 2023-03-07  4:45   ` Nicholas Piggin
  2023-03-09 23:46     ` Benjamin Gray
  0 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2023-03-07  4:45 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> ISA 3.1B introduces the Dynamic Execution Control Register (DEXCR). It
> is a per-cpu register that allows control over various CPU behaviours
> including branch hint usage, indirect branch speculation, and
> hashst/hashchk support.
>
> Though introduced in 3.1B, no CPUs using 3.1 were released, so
> CPU_FTR_ARCH_31 is used to determine support for the register itself.
> Support for each DEXCR bit (aspect) is reported separately by the
> firmware.
>
> Add various definitions and basic support for the DEXCR in the kernel.
> Right now it just initialises and maintains the DEXCR on process
> creation/swap, and clears it in reset_sprs().
>

A couple of comments below, but it looks good:

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

> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/book3s/64/kexec.h |  3 +++
>  arch/powerpc/include/asm/cputable.h        |  8 ++++++-
>  arch/powerpc/include/asm/processor.h       | 13 +++++++++++
>  arch/powerpc/include/asm/reg.h             |  6 ++++++
>  arch/powerpc/kernel/Makefile               |  1 +
>  arch/powerpc/kernel/dexcr.c                | 25 ++++++++++++++++++++++
>  arch/powerpc/kernel/dt_cpu_ftrs.c          |  4 ++++
>  arch/powerpc/kernel/process.c              | 13 ++++++++++-
>  arch/powerpc/kernel/prom.c                 |  4 ++++
>  9 files changed, 75 insertions(+), 2 deletions(-)
>  create mode 100644 arch/powerpc/kernel/dexcr.c
>
> diff --git a/arch/powerpc/include/asm/book3s/64/kexec.h b/arch/powerpc/include/asm/book3s/64/kexec.h
> index d4b9d476ecba..563baf94a962 100644
> --- a/arch/powerpc/include/asm/book3s/64/kexec.h
> +++ b/arch/powerpc/include/asm/book3s/64/kexec.h
> @@ -21,6 +21,9 @@ static inline void reset_sprs(void)
>  			plpar_set_ciabr(0);
>  	}
>  
> +	if (cpu_has_feature(CPU_FTR_ARCH_31))
> +		mtspr(SPRN_DEXCR, 0);
> +
>  	/*  Do we need isync()? We are going via a kexec reset */
>  	isync();
>  }
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index 757dbded11dc..03bc192f2d8b 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -192,6 +192,10 @@ static inline void cpu_feature_keys_init(void) { }
>  #define CPU_FTR_P9_RADIX_PREFETCH_BUG	LONG_ASM_CONST(0x0002000000000000)
>  #define CPU_FTR_ARCH_31			LONG_ASM_CONST(0x0004000000000000)
>  #define CPU_FTR_DAWR1			LONG_ASM_CONST(0x0008000000000000)
> +#define CPU_FTR_DEXCR_SBHE		LONG_ASM_CONST(0x0010000000000000)
> +#define CPU_FTR_DEXCR_IBRTPD		LONG_ASM_CONST(0x0020000000000000)
> +#define CPU_FTR_DEXCR_SRAPD		LONG_ASM_CONST(0x0040000000000000)
> +#define CPU_FTR_DEXCR_NPHIE		LONG_ASM_CONST(0x0080000000000000)

We potentially don't need to use CPU_FTR bits for each of these. We
only really want them to use instruction patching and make feature
tests fast. But we have been a bit liberal with using them and they
are kind of tied into cpu feature parsing code so maybe it's easier
to go with them for now.

>  
>  #ifndef __ASSEMBLY__
>  
> @@ -451,7 +455,9 @@ static inline void cpu_feature_keys_init(void) { }
>  	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
>  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
>  	    CPU_FTR_ARCH_300 | CPU_FTR_ARCH_31 | \
> -	    CPU_FTR_DAWR | CPU_FTR_DAWR1)
> +	    CPU_FTR_DAWR | CPU_FTR_DAWR1 | \
> +	    CPU_FTR_DEXCR_SBHE | CPU_FTR_DEXCR_IBRTPD | CPU_FTR_DEXCR_SRAPD | \
> +	    CPU_FTR_DEXCR_NPHIE)
>  #define CPU_FTRS_CELL	(CPU_FTR_LWSYNC | \
>  	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
>  	    CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 631802999d59..0a8a793b8b8b 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -446,6 +446,19 @@ int exit_vmx_usercopy(void);
>  int enter_vmx_ops(void);
>  void *exit_vmx_ops(void *dest);
>  
> +#ifdef CONFIG_PPC_BOOK3S_64
> +
> +unsigned long get_thread_dexcr(struct thread_struct const *t);
> +
> +#else
> +
> +static inline unsigned long get_thread_dexcr(struct thread_struct const *t)
> +{
> +	return 0;
> +}
> +
> +#endif /* CONFIG_PPC_BOOK3S_64 */
> +
>  #endif /* __KERNEL__ */
>  #endif /* __ASSEMBLY__ */
>  #endif /* _ASM_POWERPC_PROCESSOR_H */
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 1e8b2e04e626..cdd1f174c399 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -385,6 +385,12 @@
>  #define SPRN_HSRR0	0x13A	/* Hypervisor Save/Restore 0 */
>  #define SPRN_HSRR1	0x13B	/* Hypervisor Save/Restore 1 */
>  #define SPRN_ASDR	0x330	/* Access segment descriptor register */
> +#define SPRN_DEXCR	0x33C	/* Dynamic execution control register */
> +#define   DEXCR_PRO_MASK(aspect)	__MASK(63 - (32 + (aspect)))	/* Aspect number to problem state aspect mask */

I think PR is a better shorthand for problem state than PRO. It's just
more commonly used.

We also have PPC_BIT and PPC_BITMASK, _BIT being used for single-bit
mask. So this could be -

#define DEXCR_PR_BIT(aspect) PPC_BIT(32 + (aspect))

Or maybe DEXCR_PR_ASPECT_BIT.

> +#define   DEXCR_PRO_SBHE		DEXCR_PRO_MASK(0)	/* Speculative Branch Hint Enable */
> +#define   DEXCR_PRO_IBRTPD		DEXCR_PRO_MASK(3)	/* Indirect Branch Recurrent Target Prediction Disable */
> +#define   DEXCR_PRO_SRAPD		DEXCR_PRO_MASK(4)	/* Subroutine Return Address Prediction Disable */
> +#define   DEXCR_PRO_NPHIE		DEXCR_PRO_MASK(5)	/* Non-Privileged Hash Instruction Enable */
>  #define SPRN_IC		0x350	/* Virtual Instruction Count */
>  #define SPRN_VTB	0x351	/* Virtual Time Base */
>  #define SPRN_LDBAR	0x352	/* LD Base Address Register */
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 9b6146056e48..b112315cfdc2 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -79,6 +79,7 @@ obj-$(CONFIG_VDSO32)		+= vdso32_wrapper.o
>  obj-$(CONFIG_PPC_WATCHDOG)	+= watchdog.o
>  obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
>  obj-$(CONFIG_PPC_DAWR)		+= dawr.o
> +obj-$(CONFIG_PPC_BOOK3S_64)	+= dexcr.o
>  obj-$(CONFIG_PPC_BOOK3S_64)	+= cpu_setup_ppc970.o cpu_setup_pa6t.o
>  obj-$(CONFIG_PPC_BOOK3S_64)	+= cpu_setup_power.o
>  obj-$(CONFIG_PPC_BOOK3S_64)	+= mce.o mce_power.o
> diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
> new file mode 100644
> index 000000000000..32a0a69ff638
> --- /dev/null
> +++ b/arch/powerpc/kernel/dexcr.c
> @@ -0,0 +1,25 @@
> +#include <linux/cache.h>
> +#include <linux/init.h>
> +
> +#include <asm/cpu_has_feature.h>
> +#include <asm/cputable.h>
> +#include <asm/processor.h>
> +#include <asm/reg.h>
> +
> +#define DEFAULT_DEXCR	0
> +
> +static int __init dexcr_init(void)
> +{
> +	if (!early_cpu_has_feature(CPU_FTR_ARCH_31))
> +		return 0;
> +
> +	mtspr(SPRN_DEXCR, DEFAULT_DEXCR);
> +
> +	return 0;
> +}
> +early_initcall(dexcr_init);
> +
> +unsigned long get_thread_dexcr(struct thread_struct const *t)
> +{
> +	return DEFAULT_DEXCR;
> +}
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index c3fb9fdf5bd7..896a48211a37 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -661,6 +661,10 @@ static struct dt_cpu_feature_match __initdata
>  	{"prefix-instructions", feat_enable, 0},
>  	{"matrix-multiply-assist", feat_enable_mma, 0},
>  	{"debug-facilities-v31", feat_enable, CPU_FTR_DAWR1},
> +	{"dexcr-speculative-branch-hint-enable", feat_enable, CPU_FTR_DEXCR_SBHE},
> +	{"dexcr-indirect-branch-recurrent-target-prediction-disable", feat_enable, CPU_FTR_DEXCR_IBRTPD},
> +	{"dexcr-subroutine-return-address-prediction-disable", feat_enable, CPU_FTR_DEXCR_SRAPD},
> +	{"dexcr-non-privileged-hash-instruction-enable", feat_enable, CPU_FTR_DEXCR_NPHIE},
>  };
>  
>  static bool __initdata using_dt_cpu_ftrs;
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 67da147fe34d..17d26f652b80 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1228,6 +1228,13 @@ static inline void restore_sprs(struct thread_struct *old_thread,
>  	if (cpu_has_feature(CPU_FTR_P9_TIDR) &&
>  	    old_thread->tidr != new_thread->tidr)
>  		mtspr(SPRN_TIDR, new_thread->tidr);
> +
> +	if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> +		unsigned long new_dexcr = get_thread_dexcr(new_thread);
> +
> +		if (new_dexcr != get_thread_dexcr(old_thread))
> +			mtspr(SPRN_DEXCR, new_dexcr);
> +	}
>  #endif
>  
>  }
> @@ -1802,7 +1809,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
>  
>  	setup_ksp_vsid(p, sp);
>  
> -#ifdef CONFIG_PPC64 
> +#ifdef CONFIG_PPC64
>  	if (cpu_has_feature(CPU_FTR_DSCR)) {
>  		p->thread.dscr_inherit = current->thread.dscr_inherit;
>  		p->thread.dscr = mfspr(SPRN_DSCR);
> @@ -1939,6 +1946,10 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
>  	current->thread.tm_tfiar = 0;
>  	current->thread.load_tm = 0;
>  #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	if (cpu_has_feature(CPU_FTR_ARCH_31))
> +		mtspr(SPRN_DEXCR, get_thread_dexcr(&current->thread));
> +#endif /* CONFIG_PPC_BOOK3S_64 */

You possibly don't need the ifdef here because CPU_FTR_ARCH_31 should
fold away. Some of the others do because they're using open-coded
access to struct members, but if you're using accessor functions to
get and set such things, there may be no need to.

I think my preference is for your style.

Thanks,
Nick

>  }
>  EXPORT_SYMBOL(start_thread);
>  
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 1eed87d954ba..eff250e1ae9a 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -180,6 +180,10 @@ static struct ibm_feature ibm_pa_features[] __initdata = {
>  	  .cpu_user_ftrs2 = PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_HTM_NOSC_COMP },
>  
>  	{ .pabyte = 64, .pabit = 0, .cpu_features = CPU_FTR_DAWR1 },
> +	{ .pabyte = 68, .pabit = 0, .cpu_features = CPU_FTR_DEXCR_SBHE },
> +	{ .pabyte = 68, .pabit = 3, .cpu_features = CPU_FTR_DEXCR_IBRTPD },
> +	{ .pabyte = 68, .pabit = 4, .cpu_features = CPU_FTR_DEXCR_SRAPD },
> +	{ .pabyte = 68, .pabit = 5, .cpu_features = CPU_FTR_DEXCR_NPHIE },
>  };
>  
>  /*
> -- 
> 2.38.1


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

* Re: [RFC PATCH 04/13] powerpc/dexcr: Support userspace ROP protection
  2022-11-28  2:44 ` [RFC PATCH 04/13] powerpc/dexcr: Support userspace ROP protection Benjamin Gray
@ 2023-03-07  5:05   ` Nicholas Piggin
  2023-03-07  5:37     ` Benjamin Gray
  0 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2023-03-07  5:05 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> The ISA 3.1B hashst and hashchk instructions use a per-cpu SPR HASHKEYR
> to hold a key used in the hash calculation. This key should be different
> for each process to make it harder for a malicious process to recreate
> valid hash values for a victim process.
>
> Add support for storing a per-thread hash key, and setting/clearing
> HASHKEYR appropriately.
>
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/book3s/64/kexec.h |  3 +++
>  arch/powerpc/include/asm/processor.h       |  1 +
>  arch/powerpc/include/asm/reg.h             |  1 +
>  arch/powerpc/kernel/process.c              | 12 ++++++++++++
>  4 files changed, 17 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/kexec.h b/arch/powerpc/include/asm/book3s/64/kexec.h
> index 563baf94a962..163de935df28 100644
> --- a/arch/powerpc/include/asm/book3s/64/kexec.h
> +++ b/arch/powerpc/include/asm/book3s/64/kexec.h
> @@ -24,6 +24,9 @@ static inline void reset_sprs(void)
>  	if (cpu_has_feature(CPU_FTR_ARCH_31))
>  		mtspr(SPRN_DEXCR, 0);
>  
> +	if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
> +		mtspr(SPRN_HASHKEYR, 0);
> +
>  	/*  Do we need isync()? We are going via a kexec reset */
>  	isync();
>  }
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index c17ec1e44c86..2381217c95dc 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -264,6 +264,7 @@ struct thread_struct {
>  	unsigned long   mmcr3;
>  	unsigned long   sier2;
>  	unsigned long   sier3;
> +	unsigned long	hashkeyr;
>  
>  #endif
>  };
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index cdd1f174c399..854664cf844f 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -384,6 +384,7 @@
>  #define SPRN_HRMOR	0x139	/* Real mode offset register */
>  #define SPRN_HSRR0	0x13A	/* Hypervisor Save/Restore 0 */
>  #define SPRN_HSRR1	0x13B	/* Hypervisor Save/Restore 1 */
> +#define SPRN_HASHKEYR	0x1D4	/* Non-privileged hashst/hashchk key register */
>  #define SPRN_ASDR	0x330	/* Access segment descriptor register */
>  #define SPRN_DEXCR	0x33C	/* Dynamic execution control register */
>  #define   DEXCR_PRO_MASK(aspect)	__MASK(63 - (32 + (aspect)))	/* Aspect number to problem state aspect mask */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 17d26f652b80..4d7b0c7641d0 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1229,6 +1229,9 @@ static inline void restore_sprs(struct thread_struct *old_thread,
>  	    old_thread->tidr != new_thread->tidr)
>  		mtspr(SPRN_TIDR, new_thread->tidr);
>  
> +	if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
> +		mtspr(SPRN_HASHKEYR, new_thread->hashkeyr);

I wonder if we'd want to avoid switching it when switching to kernel
threads, and from kernel thread back to the same user thread. Might
want to optimise it to do that in future but for an initial enablement
patch this is okay.

> +
>  	if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>  		unsigned long new_dexcr = get_thread_dexcr(new_thread);
>  
> @@ -1818,6 +1821,10 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
>  		childregs->ppr = DEFAULT_PPR;
>  
>  	p->thread.tidr = 0;
> +#endif
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
> +		p->thread.hashkeyr = current->thread.hashkeyr;
>  #endif

Similar comment about your accessor style, if we had get/set_thread_hashkeyr()
functions then no ifdef required.

I think it is not quite per-process? I don't actually know how the user
toolchain side is put together, but I'm thinking we can not give it a new
salt on fork(), but we could on exec(). I think we could actually give
each thread their own salt within a process too, right?

I don't know off the top of my head whether that can be translated into
a simple test at the copy_thread level. For now you're giving out a new
salt on exec I think, which should be fine at least to start with.

Thanks,
Nick

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

>  	/*
>  	 * Run with the current AMR value of the kernel
> @@ -1947,6 +1954,11 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
>  	current->thread.load_tm = 0;
>  #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
>  #ifdef CONFIG_PPC_BOOK3S_64
> +	if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE)) {
> +		current->thread.hashkeyr = get_random_long();
> +		mtspr(SPRN_HASHKEYR, current->thread.hashkeyr);
> +	}
> +
>  	if (cpu_has_feature(CPU_FTR_ARCH_31))
>  		mtspr(SPRN_DEXCR, get_thread_dexcr(&current->thread));
>  #endif /* CONFIG_PPC_BOOK3S_64 */
> -- 
> 2.38.1


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

* Re: [RFC PATCH 05/13] prctl: Define PowerPC DEXCR interface
  2022-11-28  2:44 ` [RFC PATCH 05/13] prctl: Define PowerPC DEXCR interface Benjamin Gray
@ 2023-03-07  5:07   ` Nicholas Piggin
  0 siblings, 0 replies; 29+ messages in thread
From: Nicholas Piggin @ 2023-03-07  5:07 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> Adds the definitions and generic handler for prctl control of the
> PowerPC Dynamic Execution Control Register (DEXCR).

Assuming we'd go with the later prctl patches, this prep patch
is nice way to split out some of the mechanism.

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

>
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> ---
>  include/uapi/linux/prctl.h | 14 ++++++++++++++
>  kernel/sys.c               | 16 ++++++++++++++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index a5e06dcbba13..b4720e8de6f3 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -281,6 +281,20 @@ struct prctl_mm_map {
>  # define PR_SME_VL_LEN_MASK		0xffff
>  # define PR_SME_VL_INHERIT		(1 << 17) /* inherit across exec */
>  
> +/* PowerPC Dynamic Execution Control Register (DEXCR) controls */
> +#define PR_PPC_GET_DEXCR		65
> +#define PR_PPC_SET_DEXCR		66
> +/* DEXCR aspect to act on */
> +# define PR_PPC_DEXCR_SBHE		0 /* Speculative branch hint enable */
> +# define PR_PPC_DEXCR_IBRTPD		1 /* Indirect branch recurrent target prediction disable */
> +# define PR_PPC_DEXCR_SRAPD		2 /* Subroutine return address prediction disable */
> +# define PR_PPC_DEXCR_NPHIE		3 /* Non-privileged hash instruction enable */
> +/* Action to apply / return */
> +# define PR_PPC_DEXCR_PRCTL		(1 << 0)
> +# define PR_PPC_DEXCR_SET_ASPECT	(1 << 1)
> +# define PR_PPC_DEXCR_FORCE_SET_ASPECT	(1 << 2)
> +# define PR_PPC_DEXCR_CLEAR_ASPECT	(1 << 3)
> +
>  #define PR_SET_VMA		0x53564d41
>  # define PR_SET_VMA_ANON_NAME		0
>  
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 5fd54bf0e886..55b8f7369059 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -139,6 +139,12 @@
>  #ifndef GET_TAGGED_ADDR_CTRL
>  # define GET_TAGGED_ADDR_CTRL()		(-EINVAL)
>  #endif
> +#ifndef PPC_GET_DEXCR_ASPECT
> +# define PPC_GET_DEXCR_ASPECT(a, b)	(-EINVAL)
> +#endif
> +#ifndef PPC_SET_DEXCR_ASPECT
> +# define PPC_SET_DEXCR_ASPECT(a, b, c)	(-EINVAL)
> +#endif
>  
>  /*
>   * this is where the system-wide overflow UID and GID are defined, for
> @@ -2623,6 +2629,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>  		error = sched_core_share_pid(arg2, arg3, arg4, arg5);
>  		break;
>  #endif
> +	case PR_PPC_GET_DEXCR:
> +		if (arg3 || arg4 || arg5)
> +			return -EINVAL;
> +		error = PPC_GET_DEXCR_ASPECT(me, arg2);
> +		break;
> +	case PR_PPC_SET_DEXCR:
> +		if (arg4 || arg5)
> +			return -EINVAL;
> +		error = PPC_SET_DEXCR_ASPECT(me, arg2, arg3);
> +		break;
>  	case PR_SET_VMA:
>  		error = prctl_set_vma(arg2, arg3, arg4, arg5);
>  		break;
> -- 
> 2.38.1


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

* Re: [RFC PATCH 06/13] powerpc/dexcr: Add prctl implementation
  2022-11-28  2:44 ` [RFC PATCH 06/13] powerpc/dexcr: Add prctl implementation Benjamin Gray
@ 2023-03-07  5:12   ` Nicholas Piggin
  0 siblings, 0 replies; 29+ messages in thread
From: Nicholas Piggin @ 2023-03-07  5:12 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> Adds an initial prctl interface implementation. Unprivileged processes
> can query the current prctl setting, including whether an aspect is
> implemented by the hardware or is permitted to be modified by a setter
> prctl. Editable aspects can be changed by a CAP_SYS_ADMIN privileged
> process.
>
> The prctl setting represents what the process itself has requested, and
> does not account for any overrides. Either the kernel or a hypervisor
> may enforce a different setting for an aspect.
>
> Userspace can access a readonly view of the current DEXCR via SPR 812,
> and a readonly view of the aspects enforced by the hypervisor via
> SPR 455. A bitwise OR of these two SPRs will give the effective
> DEXCR aspect state of the process.

You said (offline) that you were looking at the PR_SPEC_* speculation
control APIs but that this was different enough that you needed a
different one.

It would be good to know what some of those issues were in the
changelog, would be nice to have some docs (could we add something
to spec_ctrl.rst maybe?). I assume at least one difference is that
some of our bits are not speculative but architectural (e.g., the
stack hash check).

I also wonder if we could implement some of the PR_SPEC controls
APIs by mapping relevant DEXCR aspects to them instead of (or as well
as) the DEXCR controls? Or would the PR_SPEC users be amenable to
extensions that make our usage fit a bit better?

I'm just thinking if we can reduce reliance on arch specific APIs a
bit would be nice.

>
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/processor.h |  13 +++
>  arch/powerpc/kernel/dexcr.c          | 133 ++++++++++++++++++++++++++-
>  arch/powerpc/kernel/process.c        |   6 ++
>  3 files changed, 151 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 2381217c95dc..4c995258f668 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -265,6 +265,9 @@ struct thread_struct {
>  	unsigned long   sier2;
>  	unsigned long   sier3;
>  	unsigned long	hashkeyr;
> +	unsigned int	dexcr_override;
> +	unsigned int	dexcr_mask;

Hmm, what's the mask doing here? It only gets bits set and never
cleared AFAIKS. What is different between an initial state and a
SET then CLEAR state?

Thanks,
Nick

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

* Re: [RFC PATCH 07/13] powerpc/dexcr: Add sysctl entry for SBHE system override
  2022-11-28  2:44 ` [RFC PATCH 07/13] powerpc/dexcr: Add sysctl entry for SBHE system override Benjamin Gray
@ 2023-03-07  5:30   ` Nicholas Piggin
  2023-03-07  5:58     ` Benjamin Gray
  0 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2023-03-07  5:30 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> The DEXCR Speculative Branch Hint Enable (SBHE) aspect controls whether
> the hints provided by BO field of Branch instructions are obeyed during
> speculative execution.
>
> SBHE behaviour per ISA 3.1B:
>
> 0:	The hints provided by BO field of Branch instructions may be
> 	ignored during speculative execution
>
> 1:	The hints provided by BO field of Branch instructions are obeyed
> 	during speculative execution
>
> Add a sysctl entry to allow changing this aspect globally in the system
> at runtime:
>
> 	/proc/sys/kernel/speculative_branch_hint_enable
>
> Three values are supported:
>
> -1:	Disable DEXCR SBHE sysctl override
>  0:	Override and set DEXCR[SBHE] aspect to 0
>  1:	Override and set DEXCR[SBHE] aspect to 1
>
> Internally, introduces a mechanism to apply arbitrary system wide
> overrides on top of the prctl() config.

Why have an override for this, and not others?

Thanks,
Nick


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

* Re: [RFC PATCH 04/13] powerpc/dexcr: Support userspace ROP protection
  2023-03-07  5:05   ` Nicholas Piggin
@ 2023-03-07  5:37     ` Benjamin Gray
  2023-03-21  4:51       ` Nicholas Piggin
  0 siblings, 1 reply; 29+ messages in thread
From: Benjamin Gray @ 2023-03-07  5:37 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Tue, 2023-03-07 at 15:05 +1000, Nicholas Piggin wrote:
> I think it is not quite per-process? I don't actually know how the
> user
> toolchain side is put together, but I'm thinking we can not give it a
> new
> salt on fork(), but we could on exec(). I think we could actually
> give
> each thread their own salt within a process too, right?

Yeah, the error case is we return further than we called in a given
execution context. A forked child may return after the fork, meaning it
needs the same key as the parent for the hashchk to work. Exec can get
a new key because we can't return with any existing hashes. I haven't
seen enough of kernel thread support to know if/how we can give threads
their own key. I believe they go through the fork() call that copies
the parent key currently.

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

* Re: [RFC PATCH 13/13] Documentation: Document PowerPC kernel DEXCR interface
  2022-11-28  2:44 ` [RFC PATCH 13/13] Documentation: Document PowerPC kernel DEXCR interface Benjamin Gray
@ 2023-03-07  5:40   ` Nicholas Piggin
  2023-03-07  5:52     ` Benjamin Gray
  0 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2023-03-07  5:40 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> Describe the DEXCR and document how to interact with it via the
> prctl and sysctl interfaces.

Oh you've got the docs here, sorry. Thanks for that. I don't know enough
yet to give much useful feedback on the API. I think at least all the
mechanism stuff up to the prctl API looks pretty straightfoward so would
like to get that merged if we can.

Might need a bit more time and discussion on the API. Interestingly
because the hashchk aspect is architectural, we may not be able to
necessarily sanely enable that, because if it was disabled to start
out with, our callchain up to the prctl call I think would have no
return hashes set so we'd immediately fail on our first return.

Thanks,
Nick


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

* Re: [RFC PATCH 13/13] Documentation: Document PowerPC kernel DEXCR interface
  2023-03-07  5:40   ` Nicholas Piggin
@ 2023-03-07  5:52     ` Benjamin Gray
  0 siblings, 0 replies; 29+ messages in thread
From: Benjamin Gray @ 2023-03-07  5:52 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Tue, 2023-03-07 at 15:40 +1000, Nicholas Piggin wrote:
> Might need a bit more time and discussion on the API. Interestingly
> because the hashchk aspect is architectural, we may not be able to
> necessarily sanely enable that, because if it was disabled to start
> out with, our callchain up to the prctl call I think would have no
> return hashes set so we'd immediately fail on our first return.

I assumed it could eventually be supported in whatever startup wrapper
programs are built with, so either as one of the first things before
any calls, or the compiler could skip putting hash instructions in the
wrapper altogether. The ELF file itself might even be able to request
bits be enabled, so the kernel would start the process correctly.

As it is inherited, it's also possible for a wrapper program to set a
specific DEXCR before a child runs. E.g.,

	fork();
	prctl(...NPHIE...);
	exec();

I hadn't thought of the prctl call itself causing an unbalanced hashchk
when it returns, but that should be solvable with an inline syscall.

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

* Re: [RFC PATCH 07/13] powerpc/dexcr: Add sysctl entry for SBHE system override
  2023-03-07  5:30   ` Nicholas Piggin
@ 2023-03-07  5:58     ` Benjamin Gray
  0 siblings, 0 replies; 29+ messages in thread
From: Benjamin Gray @ 2023-03-07  5:58 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Tue, 2023-03-07 at 15:30 +1000, Nicholas Piggin wrote:
> On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> > The DEXCR Speculative Branch Hint Enable (SBHE) aspect controls
> > whether
> > the hints provided by BO field of Branch instructions are obeyed
> > during
> > speculative execution.
> > 
> > SBHE behaviour per ISA 3.1B:
> > 
> > 0:      The hints provided by BO field of Branch instructions may
> > be
> >         ignored during speculative execution
> > 
> > 1:      The hints provided by BO field of Branch instructions are
> > obeyed
> >         during speculative execution
> > 
> > Add a sysctl entry to allow changing this aspect globally in the
> > system
> > at runtime:
> > 
> >         /proc/sys/kernel/speculative_branch_hint_enable
> > 
> > Three values are supported:
> > 
> > -1:     Disable DEXCR SBHE sysctl override
> >  0:     Override and set DEXCR[SBHE] aspect to 0
> >  1:     Override and set DEXCR[SBHE] aspect to 1
> > 
> > Internally, introduces a mechanism to apply arbitrary system wide
> > overrides on top of the prctl() config.
> 
> Why have an override for this, and not others?
> 

Should be in the commit message of course, but this aspect bleeds over
to other processes, so a user may wish to prevent all processes from
changing the value. The other aspects are probably only relevant to
their own process, though the implementation here should support
arbitrary system wide overrides.


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

* Re: [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support
  2023-03-07  4:45   ` Nicholas Piggin
@ 2023-03-09 23:46     ` Benjamin Gray
  0 siblings, 0 replies; 29+ messages in thread
From: Benjamin Gray @ 2023-03-09 23:46 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: linux-kernel, linux-hardening, cmr, ajd

On Tue, 2023-03-07 at 14:45 +1000, Nicholas Piggin wrote:
> On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> > diff --git a/arch/powerpc/include/asm/cputable.h
> > b/arch/powerpc/include/asm/cputable.h
> > index 757dbded11dc..03bc192f2d8b 100644
> > --- a/arch/powerpc/include/asm/cputable.h
> > +++ b/arch/powerpc/include/asm/cputable.h
> > @@ -192,6 +192,10 @@ static inline void cpu_feature_keys_init(void)
> > { }
> >  #define
> > CPU_FTR_P9_RADIX_PREFETCH_BUG  LONG_ASM_CONST(0x0002000000000000)
> >  #define
> > CPU_FTR_ARCH_31                        LONG_ASM_CONST(0x00040000000
> > 00000)
> >  #define
> > CPU_FTR_DAWR1                  LONG_ASM_CONST(0x0008000000000000)
> > +#define
> > CPU_FTR_DEXCR_SBHE             LONG_ASM_CONST(0x0010000000000000)
> > +#define
> > CPU_FTR_DEXCR_IBRTPD           LONG_ASM_CONST(0x0020000000000000)
> > +#define
> > CPU_FTR_DEXCR_SRAPD            LONG_ASM_CONST(0x0040000000000000)
> > +#define
> > CPU_FTR_DEXCR_NPHIE            LONG_ASM_CONST(0x0080000000000000)
> 
> We potentially don't need to use CPU_FTR bits for each of these. We
> only really want them to use instruction patching and make feature
> tests fast. But we have been a bit liberal with using them and they
> are kind of tied into cpu feature parsing code so maybe it's easier
> to go with them for now.

For the static only DEXCR series I've only got CPU_FTR_DEXCR_NPHIE
because that's needed for hashkey updates. The others don't really
matter; they are only interesting for masking out unsupported bits.
Masking itself seems to be unnecessary; the DEXCR will just ignore
unsupported bits. Attempting to set all bits on a P10 showed the first
8 were set and the remainder stayed 0'd, and the kernel worked fine.

It's definitely easier to use CPU_FTR_* for feature detection from the
PAPR specified blob though. Maybe it would be possible to support a
callback on a match instead of setting a feature flag.
@@ -1802,7 +1809,7 @@ int copy_thread(struct task_struct *p, const
> 
> 

> > @@ -1802,7 +1809,7 @@ int copy_thread(struct task_struct *p, const
> > struct kernel_clone_args *args)
> >  
> >         setup_ksp_vsid(p, sp);
> >  
> > -#ifdef CONFIG_PPC64 
> > +#ifdef CONFIG_PPC64
> >         if (cpu_has_feature(CPU_FTR_DSCR)) {
> >                 p->thread.dscr_inherit = current-
> > >thread.dscr_inherit;
> >                 p->thread.dscr = mfspr(SPRN_DSCR);
> > @@ -1939,6 +1946,10 @@ void start_thread(struct pt_regs *regs,
> > unsigned long start, unsigned long sp)
> >         current->thread.tm_tfiar = 0;
> >         current->thread.load_tm = 0;
> >  #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
> > +#ifdef CONFIG_PPC_BOOK3S_64
> > +       if (cpu_has_feature(CPU_FTR_ARCH_31))
> > +               mtspr(SPRN_DEXCR, get_thread_dexcr(&current-
> > >thread));
> > +#endif /* CONFIG_PPC_BOOK3S_64 */
> 
> You possibly don't need the ifdef here because CPU_FTR_ARCH_31 should
> fold away. Some of the others do because they're using open-coded
> access to struct members, but if you're using accessor functions to
> get and set such things, there may be no need to.
> 
> I think my preference is for your style.

I've been revisiting where the DEXCR is initialised and updated. With
the static DEXCR, the thread value is just a field on the task struct
like the others.

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

* Re: [RFC PATCH 04/13] powerpc/dexcr: Support userspace ROP protection
  2023-03-07  5:37     ` Benjamin Gray
@ 2023-03-21  4:51       ` Nicholas Piggin
  0 siblings, 0 replies; 29+ messages in thread
From: Nicholas Piggin @ 2023-03-21  4:51 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: linux-hardening, ajd, cmr, linux-kernel

On Tue Mar 7, 2023 at 3:37 PM AEST, Benjamin Gray wrote:
> On Tue, 2023-03-07 at 15:05 +1000, Nicholas Piggin wrote:
> > I think it is not quite per-process? I don't actually know how the
> > user
> > toolchain side is put together, but I'm thinking we can not give it a
> > new
> > salt on fork(), but we could on exec(). I think we could actually
> > give
> > each thread their own salt within a process too, right?
>
> Yeah, the error case is we return further than we called in a given
> execution context. A forked child may return after the fork, meaning it
> needs the same key as the parent for the hashchk to work. Exec can get
> a new key because we can't return with any existing hashes. I haven't
> seen enough of kernel thread support to know if/how we can give threads
> their own key. I believe they go through the fork() call that copies
> the parent key currently.

Could look at possibly doing per-thread keys afterward but what you're
doing makes sense so no problem.

Thanks,
Nick

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

end of thread, other threads:[~2023-03-21  4:52 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-28  2:44 [RFC PATCH 00/13] Add DEXCR support Benjamin Gray
2022-11-28  2:44 ` [RFC PATCH 01/13] powerpc/book3s: Add missing <linux/sched.h> include Benjamin Gray
2023-03-07  4:28   ` Nicholas Piggin
2022-11-28  2:44 ` [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support Benjamin Gray
2023-03-07  4:45   ` Nicholas Piggin
2023-03-09 23:46     ` Benjamin Gray
2022-11-28  2:44 ` [RFC PATCH 03/13] powerpc/dexcr: Handle hashchk exception Benjamin Gray
2022-11-29 10:39   ` Nicholas Piggin
2022-11-29 22:04     ` Benjamin Gray
2022-11-28  2:44 ` [RFC PATCH 04/13] powerpc/dexcr: Support userspace ROP protection Benjamin Gray
2023-03-07  5:05   ` Nicholas Piggin
2023-03-07  5:37     ` Benjamin Gray
2023-03-21  4:51       ` Nicholas Piggin
2022-11-28  2:44 ` [RFC PATCH 05/13] prctl: Define PowerPC DEXCR interface Benjamin Gray
2023-03-07  5:07   ` Nicholas Piggin
2022-11-28  2:44 ` [RFC PATCH 06/13] powerpc/dexcr: Add prctl implementation Benjamin Gray
2023-03-07  5:12   ` Nicholas Piggin
2022-11-28  2:44 ` [RFC PATCH 07/13] powerpc/dexcr: Add sysctl entry for SBHE system override Benjamin Gray
2023-03-07  5:30   ` Nicholas Piggin
2023-03-07  5:58     ` Benjamin Gray
2022-11-28  2:44 ` [RFC PATCH 08/13] powerpc/dexcr: Add enforced userspace ROP protection config Benjamin Gray
2022-11-28  2:44 ` [RFC PATCH 09/13] selftests/powerpc: Add more utility macros Benjamin Gray
2022-11-28  2:44 ` [RFC PATCH 10/13] selftests/powerpc: Add hashst/hashchk test Benjamin Gray
2022-11-28  2:44 ` [RFC PATCH 11/13] selftests/powerpc: Add DEXCR prctl, sysctl interface test Benjamin Gray
2022-11-28  2:44 ` [RFC PATCH 12/13] selftests/powerpc: Add DEXCR status utility lsdexcr Benjamin Gray
2022-11-28  2:44 ` [RFC PATCH 13/13] Documentation: Document PowerPC kernel DEXCR interface Benjamin Gray
2023-03-07  5:40   ` Nicholas Piggin
2023-03-07  5:52     ` Benjamin Gray
2022-11-28  4:05 ` [RFC PATCH 00/13] Add DEXCR support Russell Currey

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).