[PATCH 0/4] arm64: correctly and consistently handle Xt == XZR

[PATCH 0/4] arm64: correctly and consistently handle Xt == XZR

[Mark Rutland]

Hi,

I spotted that we inconsistently handle the use of Xt when we trap
instructions, and in some cases we may erroneously access SP.

These patches add new helpers to handle this for us, and moves code over to
using them, fixing the erroneous SP usage in the process.

Patch 2 (correcting the trapped CTR_EL0 handling) will need backporting to
stable, and is dependent on patch 1.

Patch 3 is also a fix, but shouldn't need backporting as the patch hasn't made
it into a release yet. Patch 4 is a cleanup for consistency.

Thanks,
Mark.

Mark Rutland (4):
  arm64: ptrace: add XZR-safe regs accessors
  arm64: traps: correctly handle MRS/MSR with XZR
  arm64: cpufeature: correctly handle MRS to XZR
  arm64/kprobes: consistently handle MRS/MSR with XZR

 arch/arm64/include/asm/ptrace.h          | 20 ++++++++++++++++++++
 arch/arm64/kernel/cpufeature.c           |  2 +-
 arch/arm64/kernel/probes/simulate-insn.c | 18 ++++++------------
 arch/arm64/kernel/traps.c                |  6 ++++--
 4 files changed, 31 insertions(+), 15 deletions(-)

-- 
1.9.1

[PATCH 1/4] arm64: ptrace: add XZR-safe regs accessors

[Mark Rutland]

In A64, XZR and the SP share the same encoding (31), and whether an
instruction accesses XZR or SP for a particular register parameter
depends on the definition of the instruction.

We store the SP in pt_regs::regs[31], and thus when emulating
instructions, we must be careful to not erroneously read from or write
back to the saved SP. Unfortunately, we often fail to be this careful.

In all cases, instructions using a transfer register parameter Xt use
this to refer to XZR rather than SP. This patch adds helpers so that we
can more easily and consistently handle these cases.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/ptrace.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 513daf0..11403fd 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -194,6 +194,26 @@ static inline u64 regs_get_register(struct pt_regs *regs, unsigned int offset)
 	return val;
 }
 
+/*
+ * Read a register given an architectural register index r.
+ * This handles the common case where 31 means XZR, not SP.
+ */
+static inline unsigned long pt_regs_read_reg(const struct pt_regs *regs, int r)
+{
+	return (r == 31) ? 0 : regs->regs[r];
+}
+
+/*
+ * Write a register given an architectural register index r.
+ * This handles the common case where 31 means XZR, not SP.
+ */
+static inline void pt_regs_write_reg(struct pt_regs *regs, int r,
+				     unsigned long val)
+{
+	if (r != 31)
+		regs->regs[r] = val;
+}
+
 /* Valid only for Kernel mode traps. */
 static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
 {
-- 
1.9.1

[PATCH 2/4] arm64: traps: correctly handle MRS/MSR with XZR

[Mark Rutland]

Currently we hand-roll XZR-safe register handling in
user_cache_maint_handler(), though we forget to do the same in
ctr_read_handler(), and may erroneously write back to the user SP rather
than XZR.

Use the new helpers to handle these cases correctly and consistently.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Fixes: 116c81f427ff6c53 ("arm64: Work around systems with mismatched cache line sizes")
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/traps.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Note: this is dependent on patch 1, but I couldn't fill in a Cc line for stable
as patch 1 doesn't (yet) have a stable commit id.

Mark.

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 7c3fc06..350179be 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -466,7 +466,7 @@ static void user_cache_maint_handler(unsigned int esr, struct pt_regs *regs)
 	int crm = (esr & ESR_ELx_SYS64_ISS_CRM_MASK) >> ESR_ELx_SYS64_ISS_CRM_SHIFT;
 	int ret = 0;
 
-	address = (rt == 31) ? 0 : regs->regs[rt];
+	address = pt_regs_read_reg(regs, rt);
 
 	switch (crm) {
 	case ESR_ELx_SYS64_ISS_CRM_DC_CVAU:	/* DC CVAU, gets promoted */
@@ -495,8 +495,10 @@ static void user_cache_maint_handler(unsigned int esr, struct pt_regs *regs)
 static void ctr_read_handler(unsigned int esr, struct pt_regs *regs)
 {
 	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+	unsigned long val = arm64_ftr_reg_user_value(&arm64_ftr_reg_ctrel0);
+
+	pt_regs_write_reg(regs, rt, val);
 
-	regs->regs[rt] = arm64_ftr_reg_user_value(&arm64_ftr_reg_ctrel0);
 	regs->pc += 4;
 }
 
-- 
1.9.1

[PATCH 3/4] arm64: cpufeature: correctly handle MRS to XZR

[Mark Rutland]

In emulate_mrs() we may erroneously write back to the user SP rather
than XZR if we trap an MRS instruction where Xt == 31.

Use the new pt_regs_write_reg() helper to handle this correctly.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Fixes: 77c97b4ee21290f5 ("arm64: cpufeature: Expose CPUID registers by emulation")
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 1ee5357..abda8e8 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1214,7 +1214,7 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
 	rc = emulate_sys_reg(sys_reg, &val);
 	if (!rc) {
 		dst = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn);
-		regs->user_regs.regs[dst] = val;
+		pt_regs_write_reg(regs, dst, val);
 		regs->pc += 4;
 	}
 
-- 
1.9.1

[PATCH 4/4] arm64/kprobes: consistently handle MRS/MSR with XZR

[Mark Rutland]

Now that we have XZR-safe helpers for fiddling with registers, use these
in the arm64 kprobes code rather than open-coding the logic.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/probes/simulate-insn.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kernel/probes/simulate-insn.c b/arch/arm64/kernel/probes/simulate-insn.c
index 357d3ef..be05868 100644
--- a/arch/arm64/kernel/probes/simulate-insn.c
+++ b/arch/arm64/kernel/probes/simulate-insn.c
@@ -17,6 +17,8 @@
 #include <linux/kernel.h>
 #include <linux/kprobes.h>
 
+#include <asm/ptrace.h>
+
 #include "simulate-insn.h"
 
 #define bbl_displacement(insn)		\
@@ -36,30 +38,22 @@
 
 static inline void set_x_reg(struct pt_regs *regs, int reg, u64 val)
 {
-	if (reg < 31)
-		regs->regs[reg] = val;
+	pt_regs_write_reg(regs, reg, val);
 }
 
 static inline void set_w_reg(struct pt_regs *regs, int reg, u64 val)
 {
-	if (reg < 31)
-		regs->regs[reg] = lower_32_bits(val);
+	pt_regs_write_reg(regs, reg, lower_32_bits(val));
 }
 
 static inline u64 get_x_reg(struct pt_regs *regs, int reg)
 {
-	if (reg < 31)
-		return regs->regs[reg];
-	else
-		return 0;
+	return pt_regs_read_reg(regs, reg);
 }
 
 static inline u32 get_w_reg(struct pt_regs *regs, int reg)
 {
-	if (reg < 31)
-		return lower_32_bits(regs->regs[reg]);
-	else
-		return 0;
+	return lower_32_bits(pt_regs_read_reg(regs, reg));
 }
 
 static bool __kprobes check_cbz(u32 opcode, struct pt_regs *regs)
-- 
1.9.1