[PATCH] arm64: Expose original FAR_EL1 value in sigcontext

Subsystems: arm pmu profiling and debugging, arm64 port (aarch64 architecture), the rest

STALE2300d

36 messages, 5 authors, 2020-05-21 · open the first message on its own page

[PATCH] arm64: Expose original FAR_EL1 value in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-03-12 17:18:27

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h |  9 +++++
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/hw_breakpoint.c        |  3 +-
 arch/arm64/kernel/signal.c               | 20 ++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8e..90e772d9b2cd8 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }
 
 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 5ba63204d078a..77d916c075319 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -142,7 +142,7 @@ struct thread_struct {
 	void			*sve_state;	/* SVE registers, if any */
 	unsigned int		sve_vl;		/* SVE vector length */
 	unsigned int		sve_vl_onexec;	/* SVE vl after next exec */
-	unsigned long		fault_address;	/* fault info */
+	unsigned long		fault_address;	/* FAR_EL1 value */
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427b..f532a2505d5e8 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,6 +44,7 @@ struct sigcontext {
  *
  *	0x210		fpsimd_context
  *	 0x10		esr_context
+ *	 0x10		far_context
  *	0x8a0		sve_context (vl <= 64) (optional)
  *	 0x20		extra_context (optional)
  *	 0x10		terminator (null _aarch64_ctx)

@@ -94,6 +95,14 @@ struct esr_context {
 	__u64 esr;
 };
 
+/* FAR_EL1 context */
+#define FAR_MAGIC	0x46415201
+
+struct far_context {
+	struct _aarch64_ctx head;
+	__u64 far;
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index fde59981445ca..290ea59c68b85 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	local_daif_inherit(regs);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)
 
 	user_exit_irqoff();
 	local_daif_restore(DAIF_PROCCTX);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 0b727edf41046..985cd44decf62 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c

@@ -730,7 +730,7 @@ static u64 get_distance_from_watchpoint(unsigned long addr, u64 val,
 		return 0;
 }
 
-static int watchpoint_handler(unsigned long addr, unsigned int esr,
+static int watchpoint_handler(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
 	int i, step = 0, *kernel_step, access, closest_match = 0;

@@ -741,6 +741,7 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
 	struct debug_info *debug_info;
 	struct arch_hw_breakpoint *info;
 	struct arch_hw_breakpoint_ctrl ctrl;
+	unsigned long addr = untagged_addr(far);
 
 	slots = this_cpu_ptr(wp_on_reg);
 	debug_info = &current->thread.debug;

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a915..48e8b6c7b5369 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long far_offset;
 	unsigned long sve_offset;
 	unsigned long extra_offset;
 	unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			break;
 
 		case ESR_MAGIC:
+		case FAR_MAGIC:
 			/* ignore */
 			break;

@@ -581,6 +583,11 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 				     sizeof(struct esr_context));
 		if (err)
 			return err;
+
+		err = sigframe_alloc(user, &user->far_offset,
+				     sizeof(struct far_context));
+		if (err)
+			return err;
 	}
 
 	if (system_supports_sve()) {

@@ -621,7 +628,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	__put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
 	__put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);
 
-	__put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+	__put_user_error(untagged_addr(current->thread.fault_address),
+			 &sf->uc.uc_mcontext.fault_address, err);
 
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));

@@ -641,6 +649,16 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->far_offset) {
+		struct far_context __user *far_ctx =
+			apply_user_offset(user, user->far_offset);
+
+		__put_user_error(FAR_MAGIC, &far_ctx->head.magic, err);
+		__put_user_error(sizeof(*far_ctx), &far_ctx->head.size, err);
+		__put_user_error(current->thread.fault_address, &far_ctx->far,
+				 err);
+	}
+
 	/* Scalable Vector Extension state, if present */
 	if (system_supports_sve() && err == 0 && user->sve_offset) {
 		struct sve_context __user *sve_ctx =

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f5..2ca2de1ff43be 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>
 
 struct fault_info {
-	int	(*fn)(unsigned long addr, unsigned int esr,
+	int	(*fn)(unsigned long far, unsigned int esr,
 		      struct pt_regs *regs);
 	int	sig;
 	int	code;

@@ -320,9 +320,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	die_kernel_fault(msg, addr, esr, regs);
 }
 
-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void set_thread_esr(unsigned long far, unsigned int esr)
 {
-	current->thread.fault_address = address;
+	unsigned long addr = untagged_addr(far);
+
+	current->thread.fault_address = far;
 
 	/*
 	 * If the faulting address is in the kernel, we must sanitize the ESR.

@@ -336,7 +338,7 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	 * type", so we ignore this wrinkle and just return the translation
 	 * fault.)
 	 */
-	if (!is_ttbr0_addr(current->thread.fault_address)) {
+	if (!is_ttbr0_addr(addr)) {
 		switch (ESR_ELx_EC(esr)) {
 		case ESR_ELx_EC_DABT_LOW:
 			/*

@@ -377,8 +379,11 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	current->thread.fault_code = esr;
 }
 
-static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static void do_bad_area(unsigned long far, unsigned int esr,
+			struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	/*
 	 * If we are in kernel mode at this point, we have no context to
 	 * handle this fault with.

@@ -386,7 +391,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
 	if (user_mode(regs)) {
 		const struct fault_info *inf = esr_to_fault_info(esr);
 
-		set_thread_esr(addr, esr);
+		set_thread_esr(far, esr);
 		arm64_force_sig_fault(inf->sig, inf->code, (void __user *)addr,
 				      inf->name);
 	} else {

@@ -439,7 +444,7 @@ static bool is_write_abort(unsigned int esr)
 	return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM);
 }
 
-static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
+static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 				   struct pt_regs *regs)
 {
 	const struct fault_info *inf;

@@ -447,6 +452,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	vm_fault_t fault, major = 0;
 	unsigned long vm_flags = VM_READ | VM_WRITE | VM_EXEC;
 	unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
+	unsigned long addr = untagged_addr(far);
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;

@@ -580,7 +586,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	}
 
 	inf = esr_to_fault_info(esr);
-	set_thread_esr(addr, esr);
+	set_thread_esr(far, esr);
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to successfully fix up

@@ -615,30 +621,32 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	return 0;
 }
 
-static int __kprobes do_translation_fault(unsigned long addr,
+static int __kprobes do_translation_fault(unsigned long far,
 					  unsigned int esr,
 					  struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	if (is_ttbr0_addr(addr))
-		return do_page_fault(addr, esr, regs);
+		return do_page_fault(far, esr, regs);
 
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_alignment_fault(unsigned long addr, unsigned int esr,
+static int do_alignment_fault(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_bad(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	return 1; /* "fault" */
 }
 
-static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_sea(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf;
 	void __user *siaddr;

@@ -654,7 +662,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	if (esr & ESR_ELx_FnV)
 		siaddr = NULL;
 	else
-		siaddr  = (void __user *)addr;
+		siaddr  = (void __user *)untagged_addr(far);
 	arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
 
 	return 0;

@@ -727,11 +735,12 @@ static const struct fault_info fault_info[] = {
 	{ do_bad,		SIGKILL, SI_KERNEL,	"unknown 63"			},
 };
 
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf = esr_to_fault_info(esr);
+	unsigned long addr = untagged_addr(far);
 
-	if (!inf->fn(addr, esr, regs))
+	if (!inf->fn(far, esr, regs))
 		return;
 
 	if (!user_mode(regs)) {

-- 
2.25.1.481.gfbce0eb801-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Expose original FAR_EL1 value in sigcontext

From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2020-03-25 13:10:34

Hi Peter,

On Thu, Mar 12, 2020 at 10:17:55AM -0700, Peter Collingbourne wrote:

quoted hunk

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index fde59981445ca..290ea59c68b85 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	local_daif_inherit(regs);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

Would we get a signal on faults triggered by the kernel? Anyway, I'm
fine with this change for consistency and may help with the fault
information printed by the kernel with khwasan or (later) MTE.

quoted hunk

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)
 
 	user_exit_irqoff();
 	local_daif_restore(DAIF_PROCCTX);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 0b727edf41046..985cd44decf62 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c

@@ -730,7 +730,7 @@ static u64 get_distance_from_watchpoint(unsigned long addr, u64 val,
 		return 0;
 }
 
-static int watchpoint_handler(unsigned long addr, unsigned int esr,
+static int watchpoint_handler(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
 	int i, step = 0, *kernel_step, access, closest_match = 0;

@@ -741,6 +741,7 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
 	struct debug_info *debug_info;
 	struct arch_hw_breakpoint *info;
 	struct arch_hw_breakpoint_ctrl ctrl;
+	unsigned long addr = untagged_addr(far);
 
 	slots = this_cpu_ptr(wp_on_reg);
 	debug_info = &current->thread.debug;

Why do we need to untag this here? Have you hit any bug? This function
gets the original FAR_EL1 value, untagged (via elX_dbg()), and we clear
the tag further down in get_distance_from_watchpoint().

quoted hunk

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a915..48e8b6c7b5369 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long far_offset;
 	unsigned long sve_offset;
 	unsigned long extra_offset;
 	unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			break;
 
 		case ESR_MAGIC:
+		case FAR_MAGIC:
 			/* ignore */
 			break;

@@ -581,6 +583,11 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 				     sizeof(struct esr_context));
 		if (err)
 			return err;
+
+		err = sigframe_alloc(user, &user->far_offset,
+				     sizeof(struct far_context));
+		if (err)
+			return err;

It looks fine, I think it makes sense to only expose the raw FAR_EL1
when we also expose the ESR_EL1 (via set_thread_esr()).

quoted hunk

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f5..2ca2de1ff43be 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>
 
 struct fault_info {
-	int	(*fn)(unsigned long addr, unsigned int esr,
+	int	(*fn)(unsigned long far, unsigned int esr,
 		      struct pt_regs *regs);
 	int	sig;
 	int	code;

@@ -320,9 +320,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	die_kernel_fault(msg, addr, esr, regs);
 }
 
-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void set_thread_esr(unsigned long far, unsigned int esr)

We might as well rename this to set_thread_far_esr().

Thanks.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2] arm64: Expose original FAR_EL1 value in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-03-25 17:41:05

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h |  9 +++++
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 20 ++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 6 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }
 
 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 5ba63204d078..77d916c07531 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -142,7 +142,7 @@ struct thread_struct {
 	void			*sve_state;	/* SVE registers, if any */
 	unsigned int		sve_vl;		/* SVE vector length */
 	unsigned int		sve_vl_onexec;	/* SVE vl after next exec */
-	unsigned long		fault_address;	/* fault info */
+	unsigned long		fault_address;	/* FAR_EL1 value */
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..f532a2505d5e 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,6 +44,7 @@ struct sigcontext {
  *
  *	0x210		fpsimd_context
  *	 0x10		esr_context
+ *	 0x10		far_context
  *	0x8a0		sve_context (vl <= 64) (optional)
  *	 0x20		extra_context (optional)
  *	 0x10		terminator (null _aarch64_ctx)

@@ -94,6 +95,14 @@ struct esr_context {
 	__u64 esr;
 };
 
+/* FAR_EL1 context */
+#define FAR_MAGIC	0x46415201
+
+struct far_context {
+	struct _aarch64_ctx head;
+	__u64 far;
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index fde59981445c..290ea59c68b8 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	local_daif_inherit(regs);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)
 
 	user_exit_irqoff();
 	local_daif_restore(DAIF_PROCCTX);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..48e8b6c7b536 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long far_offset;
 	unsigned long sve_offset;
 	unsigned long extra_offset;
 	unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			break;
 
 		case ESR_MAGIC:
+		case FAR_MAGIC:
 			/* ignore */
 			break;

@@ -581,6 +583,11 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 				     sizeof(struct esr_context));
 		if (err)
 			return err;
+
+		err = sigframe_alloc(user, &user->far_offset,
+				     sizeof(struct far_context));
+		if (err)
+			return err;
 	}
 
 	if (system_supports_sve()) {

@@ -621,7 +628,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	__put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
 	__put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);
 
-	__put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+	__put_user_error(untagged_addr(current->thread.fault_address),
+			 &sf->uc.uc_mcontext.fault_address, err);
 
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));

@@ -641,6 +649,16 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->far_offset) {
+		struct far_context __user *far_ctx =
+			apply_user_offset(user, user->far_offset);
+
+		__put_user_error(FAR_MAGIC, &far_ctx->head.magic, err);
+		__put_user_error(sizeof(*far_ctx), &far_ctx->head.size, err);
+		__put_user_error(current->thread.fault_address, &far_ctx->far,
+				 err);
+	}
+
 	/* Scalable Vector Extension state, if present */
 	if (system_supports_sve() && err == 0 && user->sve_offset) {
 		struct sve_context __user *sve_ctx =

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f..738adc950012 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>
 
 struct fault_info {
-	int	(*fn)(unsigned long addr, unsigned int esr,
+	int	(*fn)(unsigned long far, unsigned int esr,
 		      struct pt_regs *regs);
 	int	sig;
 	int	code;

@@ -320,9 +320,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	die_kernel_fault(msg, addr, esr, regs);
 }
 
-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void set_thread_far_esr(unsigned long far, unsigned int esr)
 {
-	current->thread.fault_address = address;
+	unsigned long addr = untagged_addr(far);
+
+	current->thread.fault_address = far;
 
 	/*
 	 * If the faulting address is in the kernel, we must sanitize the ESR.

@@ -336,7 +338,7 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	 * type", so we ignore this wrinkle and just return the translation
 	 * fault.)
 	 */
-	if (!is_ttbr0_addr(current->thread.fault_address)) {
+	if (!is_ttbr0_addr(addr)) {
 		switch (ESR_ELx_EC(esr)) {
 		case ESR_ELx_EC_DABT_LOW:
 			/*

@@ -377,8 +379,11 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	current->thread.fault_code = esr;
 }
 
-static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static void do_bad_area(unsigned long far, unsigned int esr,
+			struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	/*
 	 * If we are in kernel mode at this point, we have no context to
 	 * handle this fault with.

@@ -386,7 +391,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
 	if (user_mode(regs)) {
 		const struct fault_info *inf = esr_to_fault_info(esr);
 
-		set_thread_esr(addr, esr);
+		set_thread_far_esr(far, esr);
 		arm64_force_sig_fault(inf->sig, inf->code, (void __user *)addr,
 				      inf->name);
 	} else {

@@ -439,7 +444,7 @@ static bool is_write_abort(unsigned int esr)
 	return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM);
 }
 
-static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
+static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 				   struct pt_regs *regs)
 {
 	const struct fault_info *inf;

@@ -447,6 +452,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	vm_fault_t fault, major = 0;
 	unsigned long vm_flags = VM_READ | VM_WRITE | VM_EXEC;
 	unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
+	unsigned long addr = untagged_addr(far);
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;

@@ -580,7 +586,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	}
 
 	inf = esr_to_fault_info(esr);
-	set_thread_esr(addr, esr);
+	set_thread_far_esr(far, esr);
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to successfully fix up

@@ -615,30 +621,32 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	return 0;
 }
 
-static int __kprobes do_translation_fault(unsigned long addr,
+static int __kprobes do_translation_fault(unsigned long far,
 					  unsigned int esr,
 					  struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	if (is_ttbr0_addr(addr))
-		return do_page_fault(addr, esr, regs);
+		return do_page_fault(far, esr, regs);
 
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_alignment_fault(unsigned long addr, unsigned int esr,
+static int do_alignment_fault(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_bad(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	return 1; /* "fault" */
 }
 
-static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_sea(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf;
 	void __user *siaddr;

@@ -654,7 +662,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	if (esr & ESR_ELx_FnV)
 		siaddr = NULL;
 	else
-		siaddr  = (void __user *)addr;
+		siaddr  = (void __user *)untagged_addr(far);
 	arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
 
 	return 0;

@@ -727,11 +735,12 @@ static const struct fault_info fault_info[] = {
 	{ do_bad,		SIGKILL, SI_KERNEL,	"unknown 63"			},
 };
 
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf = esr_to_fault_info(esr);
+	unsigned long addr = untagged_addr(far);
 
-	if (!inf->fn(addr, esr, regs))
+	if (!inf->fn(far, esr, regs))
 		return;
 
 	if (!user_mode(regs)) {

-- 
2.25.1.696.g5e7596f4ac-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Expose original FAR_EL1 value in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-03-25 17:41:54

On Wed, Mar 25, 2020 at 6:10 AM Catalin Marinas [off-list ref] wrote:

Hi Peter,

On Thu, Mar 12, 2020 at 10:17:55AM -0700, Peter Collingbourne wrote:

quoted

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index fde59981445ca..290ea59c68b85 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
      unsigned long far = read_sysreg(far_el1);

      local_daif_inherit(regs);
-     far = untagged_addr(far);
      do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

Would we get a signal on faults triggered by the kernel? Anyway, I'm
fine with this change for consistency and may help with the fault
information printed by the kernel with khwasan or (later) MTE.

It doesn't look like we would. As far as I can tell all of the signal
injection paths are guarded with if (user_mode(regs)) and such. Agreed
with the consistency argument.

quoted

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)

      user_exit_irqoff();
      local_daif_restore(DAIF_PROCCTX);
-     far = untagged_addr(far);
      do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 0b727edf41046..985cd44decf62 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c

@@ -730,7 +730,7 @@ static u64 get_distance_from_watchpoint(unsigned long addr, u64 val,
              return 0;
 }

-static int watchpoint_handler(unsigned long addr, unsigned int esr,
+static int watchpoint_handler(unsigned long far, unsigned int esr,
                            struct pt_regs *regs)
 {
      int i, step = 0, *kernel_step, access, closest_match = 0;

@@ -741,6 +741,7 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
      struct debug_info *debug_info;
      struct arch_hw_breakpoint *info;
      struct arch_hw_breakpoint_ctrl ctrl;
+     unsigned long addr = untagged_addr(far);

      slots = this_cpu_ptr(wp_on_reg);
      debug_info = &current->thread.debug;

Why do we need to untag this here? Have you hit any bug? This function
gets the original FAR_EL1 value, untagged (via elX_dbg()), and we clear
the tag further down in get_distance_from_watchpoint().

You're right, I missed that this was going via elX_dbg() rather than
an abort handler. In fact, this would seem to be a potential userspace
break because the now-untagged address is also stored in
counter_arch_bp(wp)->trigger, which is exposed to userspace via
ptrace_hbptriggered in arch/arm64/kernel/ptrace.c. I've reverted this
part in v2.

quoted

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a915..48e8b6c7b5369 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {

      unsigned long fpsimd_offset;
      unsigned long esr_offset;
+     unsigned long far_offset;
      unsigned long sve_offset;
      unsigned long extra_offset;
      unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
                      break;

              case ESR_MAGIC:
+             case FAR_MAGIC:
                      /* ignore */
                      break;

@@ -581,6 +583,11 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
                                   sizeof(struct esr_context));
              if (err)
                      return err;
+
+             err = sigframe_alloc(user, &user->far_offset,
+                                  sizeof(struct far_context));
+             if (err)
+                     return err;

It looks fine, I think it makes sense to only expose the raw FAR_EL1
when we also expose the ESR_EL1 (via set_thread_esr()).

quoted

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f5..2ca2de1ff43be 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>

 struct fault_info {
-     int     (*fn)(unsigned long addr, unsigned int esr,
+     int     (*fn)(unsigned long far, unsigned int esr,
                    struct pt_regs *regs);
      int     sig;
      int     code;

@@ -320,9 +320,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
      die_kernel_fault(msg, addr, esr, regs);
 }

-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void set_thread_esr(unsigned long far, unsigned int esr)

We might as well rename this to set_thread_far_esr().

Done in v2.


Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] arm64: Expose original FAR_EL1 value in sigcontext

From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2020-03-26 16:45:51

On Wed, Mar 25, 2020 at 10:40:01AM -0700, Peter Collingbourne wrote:

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>

The patch looks fine. However, I wouldn't queue it for 5.7, it's too
close to the merging window and I'd like it to sit in linux-next for a
bit. Unless there are other comments, it looks fine to me for -rc8.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] arm64: Expose original FAR_EL1 value in sigcontext

From: Will Deacon <will@kernel.org>
Date: 2020-03-27 07:57:07

On Thu, Mar 26, 2020 at 04:45:39PM +0000, Catalin Marinas wrote:

On Wed, Mar 25, 2020 at 10:40:01AM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>

The patch looks fine. However, I wouldn't queue it for 5.7, it's too
close to the merging window and I'd like it to sit in linux-next for a
bit. Unless there are other comments, it looks fine to me for -rc8.

You mean 5.8? I'm also a bit surprised not to see a docs update, given that
we talk about the general lack of tags in siginfo_t towards the end of
Documentation/arm64/tagged-pointers.rst

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] arm64: Expose original FAR_EL1 value in sigcontext

From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2020-03-27 11:39:13

On Fri, Mar 27, 2020 at 07:56:56AM +0000, Will Deacon wrote:

On Thu, Mar 26, 2020 at 04:45:39PM +0000, Catalin Marinas wrote:

quoted

On Wed, Mar 25, 2020 at 10:40:01AM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>

The patch looks fine. However, I wouldn't queue it for 5.7, it's too
close to the merging window and I'd like it to sit in linux-next for a
bit. Unless there are other comments, it looks fine to me for -rc8.

You mean 5.8?

Yes.

I'm also a bit surprised not to see a docs update, given that
we talk about the general lack of tags in siginfo_t towards the end of
Documentation/arm64/tagged-pointers.rst

Good point. It's worth adding this to the tagged-pointers.rst document
since the only use of the raw FAR_EL1 is for tagged pointers.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-03-27 19:20:01

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 21 +++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 20 ++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 74 insertions(+), 35 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index eab4323609b9..9da7f6262fad 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst

@@ -53,12 +53,17 @@ visibility.
 Preserving tags
 ---------------
 
-Non-zero tags are not preserved when delivering signals. This means that
-signal handlers in applications making use of tags cannot rely on the
-tag information for user virtual addresses being maintained for fields
-inside siginfo_t. One exception to this rule is for signals raised in
-response to watchpoint debug exceptions, where the tag information will
-be preserved.
+Non-zero tags are not preserved in the fault address fields
+siginfo.si_addr or sigcontext.fault_address when delivering
+signals. This means that signal handlers in applications making use
+of tags cannot rely on the tag information for user virtual addresses
+being maintained in these fields. One exception to this rule is for
+signals raised in response to watchpoint debug exceptions, where the
+tag information will be preserved.
+
+The fault address tag is preserved in the far field of the signal
+frame record far_context, which is present for signals raised in
+response to data aborts and instruction aborts.
 
 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }
 
 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 5ba63204d078..77d916c07531 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -142,7 +142,7 @@ struct thread_struct {
 	void			*sve_state;	/* SVE registers, if any */
 	unsigned int		sve_vl;		/* SVE vector length */
 	unsigned int		sve_vl_onexec;	/* SVE vl after next exec */
-	unsigned long		fault_address;	/* fault info */
+	unsigned long		fault_address;	/* FAR_EL1 value */
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..6782394633cb 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *	0x210		fpsimd_context
  *	 0x10		esr_context
+ *	 0x10		far_context
  *	0x8a0		sve_context (vl <= 64) (optional)
  *	 0x20		extra_context (optional)
  *	 0x10		terminator (null _aarch64_ctx)
  *
- *	0x510		(reserved for future allocation)
+ *	0x500		(reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,25 @@ struct esr_context {
 	__u64 esr;
 };
 
+/* FAR_EL1 context */
+#define FAR_MAGIC	0x46415201
+
+struct far_context {
+	struct _aarch64_ctx head;
+	__u64 far;
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, far_context and extra_context must be
+ * placed in sigcontext.__reserved[] if present.  They cannot be placed
+ * in the extra space.  Any other record can be placed either in the
+ * extra space or in sigcontext.__reserved[], unless otherwise specified
+ * in this file.
  *
  * 2) There must not be more than one extra_context.
  *

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index fde59981445c..290ea59c68b8 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	local_daif_inherit(regs);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)
 
 	user_exit_irqoff();
 	local_daif_restore(DAIF_PROCCTX);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..48e8b6c7b536 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long far_offset;
 	unsigned long sve_offset;
 	unsigned long extra_offset;
 	unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			break;
 
 		case ESR_MAGIC:
+		case FAR_MAGIC:
 			/* ignore */
 			break;

@@ -581,6 +583,11 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 				     sizeof(struct esr_context));
 		if (err)
 			return err;
+
+		err = sigframe_alloc(user, &user->far_offset,
+				     sizeof(struct far_context));
+		if (err)
+			return err;
 	}
 
 	if (system_supports_sve()) {

@@ -621,7 +628,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	__put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
 	__put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);
 
-	__put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+	__put_user_error(untagged_addr(current->thread.fault_address),
+			 &sf->uc.uc_mcontext.fault_address, err);
 
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));

@@ -641,6 +649,16 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->far_offset) {
+		struct far_context __user *far_ctx =
+			apply_user_offset(user, user->far_offset);
+
+		__put_user_error(FAR_MAGIC, &far_ctx->head.magic, err);
+		__put_user_error(sizeof(*far_ctx), &far_ctx->head.size, err);
+		__put_user_error(current->thread.fault_address, &far_ctx->far,
+				 err);
+	}
+
 	/* Scalable Vector Extension state, if present */
 	if (system_supports_sve() && err == 0 && user->sve_offset) {
 		struct sve_context __user *sve_ctx =

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f..738adc950012 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>
 
 struct fault_info {
-	int	(*fn)(unsigned long addr, unsigned int esr,
+	int	(*fn)(unsigned long far, unsigned int esr,
 		      struct pt_regs *regs);
 	int	sig;
 	int	code;

@@ -320,9 +320,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	die_kernel_fault(msg, addr, esr, regs);
 }
 
-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void set_thread_far_esr(unsigned long far, unsigned int esr)
 {
-	current->thread.fault_address = address;
+	unsigned long addr = untagged_addr(far);
+
+	current->thread.fault_address = far;
 
 	/*
 	 * If the faulting address is in the kernel, we must sanitize the ESR.

@@ -336,7 +338,7 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	 * type", so we ignore this wrinkle and just return the translation
 	 * fault.)
 	 */
-	if (!is_ttbr0_addr(current->thread.fault_address)) {
+	if (!is_ttbr0_addr(addr)) {
 		switch (ESR_ELx_EC(esr)) {
 		case ESR_ELx_EC_DABT_LOW:
 			/*

@@ -377,8 +379,11 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	current->thread.fault_code = esr;
 }
 
-static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static void do_bad_area(unsigned long far, unsigned int esr,
+			struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	/*
 	 * If we are in kernel mode at this point, we have no context to
 	 * handle this fault with.

@@ -386,7 +391,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
 	if (user_mode(regs)) {
 		const struct fault_info *inf = esr_to_fault_info(esr);
 
-		set_thread_esr(addr, esr);
+		set_thread_far_esr(far, esr);
 		arm64_force_sig_fault(inf->sig, inf->code, (void __user *)addr,
 				      inf->name);
 	} else {

@@ -439,7 +444,7 @@ static bool is_write_abort(unsigned int esr)
 	return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM);
 }
 
-static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
+static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 				   struct pt_regs *regs)
 {
 	const struct fault_info *inf;

@@ -447,6 +452,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	vm_fault_t fault, major = 0;
 	unsigned long vm_flags = VM_READ | VM_WRITE | VM_EXEC;
 	unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
+	unsigned long addr = untagged_addr(far);
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;

@@ -580,7 +586,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	}
 
 	inf = esr_to_fault_info(esr);
-	set_thread_esr(addr, esr);
+	set_thread_far_esr(far, esr);
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to successfully fix up

@@ -615,30 +621,32 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	return 0;
 }
 
-static int __kprobes do_translation_fault(unsigned long addr,
+static int __kprobes do_translation_fault(unsigned long far,
 					  unsigned int esr,
 					  struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	if (is_ttbr0_addr(addr))
-		return do_page_fault(addr, esr, regs);
+		return do_page_fault(far, esr, regs);
 
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_alignment_fault(unsigned long addr, unsigned int esr,
+static int do_alignment_fault(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_bad(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	return 1; /* "fault" */
 }
 
-static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_sea(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf;
 	void __user *siaddr;

@@ -654,7 +662,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	if (esr & ESR_ELx_FnV)
 		siaddr = NULL;
 	else
-		siaddr  = (void __user *)addr;
+		siaddr  = (void __user *)untagged_addr(far);
 	arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
 
 	return 0;

@@ -727,11 +735,12 @@ static const struct fault_info fault_info[] = {
 	{ do_bad,		SIGKILL, SI_KERNEL,	"unknown 63"			},
 };
 
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf = esr_to_fault_info(esr);
+	unsigned long addr = untagged_addr(far);
 
-	if (!inf->fn(addr, esr, regs))
+	if (!inf->fn(far, esr, regs))
 		return;
 
 	if (!user_mode(regs)) {

-- 
2.26.0.rc2.310.g2932bb562d-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] arm64: Expose original FAR_EL1 value in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-03-27 19:27:05

On Fri, Mar 27, 2020 at 4:39 AM Catalin Marinas [off-list ref] wrote:

On Fri, Mar 27, 2020 at 07:56:56AM +0000, Will Deacon wrote:

quoted

On Thu, Mar 26, 2020 at 04:45:39PM +0000, Catalin Marinas wrote:

quoted

On Wed, Mar 25, 2020 at 10:40:01AM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>

The patch looks fine. However, I wouldn't queue it for 5.7, it's too
close to the merging window and I'd like it to sit in linux-next for a
bit. Unless there are other comments, it looks fine to me for -rc8.

You mean 5.8?

Yes.

quoted

I'm also a bit surprised not to see a docs update, given that
we talk about the general lack of tags in siginfo_t towards the end of
Documentation/arm64/tagged-pointers.rst

Good point. It's worth adding this to the tagged-pointers.rst document
since the only use of the raw FAR_EL1 is for tagged pointers.

In v3 I've added a paragraph about far_context after the paragraph
that talks about siginfo.

Unless I'm mistaken it looks like that paragraph is only really
talking about the fault address (there aren't any other user address
fields in siginfo on arm64 as far as I can tell), so I reworded it so
that my new paragraph follows on from it (and clarified that
sigcontext.fault_address doesn't have the tag either).

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2020-04-22 14:27:06

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Will Deacon <will@kernel.org>
Date: 2020-04-29 21:08:36

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

Hmm, although the code looks fine, why don't we just expose the tag in the
new field, rather than duplicate the address information? I'm nervous about
exposing privileged registers directly to userspace.

Also, Catalin, could you elaborate on the MTE use-case please? The
architecture says that FAR_EL1[63:60] are UNKNOWN on a synchronous tag
check fault, so we'd have to *avoid* exposing them in that case!

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-04-29 21:42:27

On Wed, Apr 29, 2020 at 2:08 PM Will Deacon [off-list ref] wrote:

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

Hmm, although the code looks fine, why don't we just expose the tag in the
new field, rather than duplicate the address information? I'm nervous about
exposing privileged registers directly to userspace.

I have no strong opinion on whether this should just contain the tag or not.

Also, Catalin, could you elaborate on the MTE use-case please? The
architecture says that FAR_EL1[63:60] are UNKNOWN on a synchronous tag
check fault, so we'd have to *avoid* exposing them in that case!

The basic use case is to allow a signal handler to identify which
allocation was accessed improperly in order to provide better
diagnostics. For example, if you have granules tagged 1,2,3
consecutively and see an access with pointer tag 1 on the granule
tagged 2, you can tell that it was probably a buffer overflow from the
1 granule and you can report that to the user.

It seems unfortunate that bits 63:60 are now UNKNOWN on synchronous
tag check faults. It seems to be a recent change to the specification.
I can think of a number of use cases for bits 63:60 after a
synchronous tag check fault -- for example, an allocator could store
an additional set of random bits there, and later use them to
determine with more accuracy which allocation was used after free or
out of bounds. That being said, we aren't planning to do this in the
initial version of the MTE-aware scudo allocator.

If there is no chance of changing the architecture at this point, I
will send a v4 with the bits masked out when handling a tag check
fault.

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2020-04-30 09:50:16

On Wed, Apr 29, 2020 at 10:08:26PM +0100, Will Deacon wrote:

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

Hmm, although the code looks fine, why don't we just expose the tag in the
new field, rather than duplicate the address information? I'm nervous about
exposing privileged registers directly to userspace.

That's for consistency with ESR_EL1 which we expose in a similar way,
though with bits of it not relevant to user masked out. For FAR_EL1, all
the bits are relevant, even if some of them are duplicated in the
si_addr field.

Also, Catalin, could you elaborate on the MTE use-case please? The
architecture says that FAR_EL1[63:60] are UNKNOWN on a synchronous tag
check fault, so we'd have to *avoid* exposing them in that case!

With MTE, FAR_EL1[63:60] will be cleared on sync tag check faults (not
currently done as I don't have this patch in my MTE series).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Will Deacon <will@kernel.org>
Date: 2020-04-30 10:00:42

On Thu, Apr 30, 2020 at 10:50:01AM +0100, Catalin Marinas wrote:

On Wed, Apr 29, 2020 at 10:08:26PM +0100, Will Deacon wrote:

quoted

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

Hmm, although the code looks fine, why don't we just expose the tag in the
new field, rather than duplicate the address information? I'm nervous about
exposing privileged registers directly to userspace.

That's for consistency with ESR_EL1 which we expose in a similar way,
though with bits of it not relevant to user masked out. For FAR_EL1, all
the bits are relevant, even if some of them are duplicated in the
si_addr field.

It may be consistent, but I would argue that exposing ESR_EL1 was a mistake,
as illustrated by cc19846079a7 ("arm64: fault: Don't leak data in ESR
context for user fault on kernel VA"). We have to live with that, but we
should try to do better for new fields in the sigcontext.

quoted

Also, Catalin, could you elaborate on the MTE use-case please? The
architecture says that FAR_EL1[63:60] are UNKNOWN on a synchronous tag
check fault, so we'd have to *avoid* exposing them in that case!

With MTE, FAR_EL1[63:60] will be cleared on sync tag check faults (not
currently done as I don't have this patch in my MTE series).

Ok, but in [1] you said "I'm fine with this change for consistency and
may help with the fault information printed by the kernel with khwasan
or (later) MTE."

But I don't think consistency is necessarily a good thing here and I don't
see how it helps with MTE if we zap the bits to 0! We'd be better off not
exposing the information at all in this situation.

Will

[1] https://lore.kernel.org/r/20200325131023.GN3901@mbp

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2020-04-30 13:34:59

On Thu, Apr 30, 2020 at 10:59:19AM +0100, Will Deacon wrote:

On Thu, Apr 30, 2020 at 10:50:01AM +0100, Catalin Marinas wrote:

quoted

On Wed, Apr 29, 2020 at 10:08:26PM +0100, Will Deacon wrote:

quoted

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

Hmm, although the code looks fine, why don't we just expose the tag in the
new field, rather than duplicate the address information? I'm nervous about
exposing privileged registers directly to userspace.

That's for consistency with ESR_EL1 which we expose in a similar way,
though with bits of it not relevant to user masked out. For FAR_EL1, all
the bits are relevant, even if some of them are duplicated in the
si_addr field.

It may be consistent, but I would argue that exposing ESR_EL1 was a mistake,
as illustrated by cc19846079a7 ("arm64: fault: Don't leak data in ESR
context for user fault on kernel VA"). We have to live with that, but we
should try to do better for new fields in the sigcontext.

The alternative would be to only expose the tag of the faulting address
if you are worried of leaking some kernel address from FAR_EL1 (and I
agree, there is a risk).

quoted

Also, Catalin, could you elaborate on the MTE use-case please? The
architecture says that FAR_EL1[63:60] are UNKNOWN on a synchronous tag
check fault, so we'd have to *avoid* exposing them in that case!

With MTE, FAR_EL1[63:60] will be cleared on sync tag check faults (not
currently done as I don't have this patch in my MTE series).

Ok, but in [1] you said "I'm fine with this change for consistency and
may help with the fault information printed by the kernel with khwasan
or (later) MTE."

But I don't think consistency is necessarily a good thing here and I don't
see how it helps with MTE if we zap the bits to 0! We'd be better off not
exposing the information at all in this situation.

The plan for MTE is to only zap bits 63:60 which are unknown on tag
check fault. The actual MTE tag in 59:56 would be preserved. There is an
inconsistency with the TBI feature but that only happens for tag check
faults which is a new thing. The behaviour on any other fault will be
consistent with the non-MTE case (page faults etc.).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Dave Martin <Dave.Martin@arm.com>
Date: 2020-05-04 10:19:43

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 21 +++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 20 ++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 74 insertions(+), 35 deletions(-)

[...]

quoted hunk

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..6782394633cb 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *	0x210		fpsimd_context
  *	 0x10		esr_context
+ *	 0x10		far_context
  *	0x8a0		sve_context (vl <= 64) (optional)
  *	 0x20		extra_context (optional)
  *	 0x10		terminator (null _aarch64_ctx)
  *
- *	0x510		(reserved for future allocation)
+ *	0x500		(reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,25 @@ struct esr_context {
 	__u64 esr;
 };
 
+/* FAR_EL1 context */
+#define FAR_MAGIC	0x46415201
+
+struct far_context {
+	struct _aarch64_ctx head;
+	__u64 far;
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, far_context and extra_context must be
+ * placed in sigcontext.__reserved[] if present.  They cannot be placed
+ * in the extra space.  Any other record can be placed either in the
+ * extra space or in sigcontext.__reserved[], unless otherwise specified
+ * in this file.

This is for backwards compatibility only.  We don't need this constraint
for any new field, so you can probably leave the paragraph as-is.

Removing this would mean constraint would mean that userspace must be
prepared to traverse extra_context when looking for far_context.  But
really we want modern userspace to do this anyway, since it reduces
backwards compatibilty worries when adding more new records in the
future.


The nasty loop in parse_user_sigframe() allows some flexibility
regarding the order of records, but prior to this patch there is no
record that can be actually be moved, due to other backwards
compatibility constraints -- so the flexibility isn't used today.  I'd
like to avoid reorderability creeping in, so that we can get rid of the
loop.

So, mandating that records must be in a consistent order to sigcontext.h
could be helpful.  inserting new records in the middle should be fine,
so long as there is no shuffling.

I'm not sure this patch needs to do anything extra for that: perhaps we
can leave this no-shuffling rule implicit for now (?)

People already get shouted at for needslessly noisy diffs, so there is a
strong disincentive to shuffle existing headers in any case...

[...]

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Will Deacon <will@kernel.org>
Date: 2020-05-04 17:03:31

Hi Peter,

On Wed, Apr 29, 2020 at 02:42:01PM -0700, Peter Collingbourne wrote:

On Wed, Apr 29, 2020 at 2:08 PM Will Deacon [off-list ref] wrote:

quoted

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

Hmm, although the code looks fine, why don't we just expose the tag in the
new field, rather than duplicate the address information? I'm nervous about
exposing privileged registers directly to userspace.

I have no strong opinion on whether this should just contain the tag or not.

A few of us chatted about this today. Please could you spin a v4 where only
the top byte is exposed in the new sigcontext record as a __u8? You'll need
to think of a better name than "FAR"; perhaps something like 'si_addr_top_byte',
'si_addr_63_56' or whatever you fancy. Naming is hard.

For MTE we can add a separate record later on, so as not to overload this
(e.g. si_addr_mte_tag).

Ta,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-07 17:55:23

On Mon, May 4, 2020 at 3:19 AM Dave Martin [off-list ref] wrote:

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 21 +++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 20 ++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 74 insertions(+), 35 deletions(-)

[...]

quoted

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..6782394633cb 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *   0x210           fpsimd_context
  *    0x10           esr_context
+ *    0x10           far_context
  *   0x8a0           sve_context (vl <= 64) (optional)
  *    0x20           extra_context (optional)
  *    0x10           terminator (null _aarch64_ctx)
  *
- *   0x510           (reserved for future allocation)
+ *   0x500           (reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,25 @@ struct esr_context {
      __u64 esr;
 };

+/* FAR_EL1 context */
+#define FAR_MAGIC    0x46415201
+
+struct far_context {
+     struct _aarch64_ctx head;
+     __u64 far;
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, far_context and extra_context must be
+ * placed in sigcontext.__reserved[] if present.  They cannot be placed
+ * in the extra space.  Any other record can be placed either in the
+ * extra space or in sigcontext.__reserved[], unless otherwise specified
+ * in this file.

This is for backwards compatibility only.  We don't need this constraint
for any new field, so you can probably leave the paragraph as-is.

Removing this would mean constraint would mean that userspace must be
prepared to traverse extra_context when looking for far_context.  But
really we want modern userspace to do this anyway, since it reduces
backwards compatibilty worries when adding more new records in the
future.

My original reason for updating this comment was that I figured that
this record was small enough that we could just always include it in
__reserved.

But thinking about this a bit more, it doesn't seem that just wanting
userspace to read extra_context will guarantee that it will do so. In
practice, it would be easy to write userspace code that works right
now but doesn't read extra_context correctly (either because
extra_context wasn't considered at all, or because the code purporting
to read the record from extra_context contains a latent bug because it
wasn't exercised). Since we may be practically constrained from moving
the record anyway, we might as well document it and allow the
userspace code to be a little simpler.

I guess one alternative is that we always place this record in
extra_context, which would force userspace to read it correctly. That
has something of the opposite problem (userspace code could be written
to only expect the record in extra_context), but at least we're less
constrained there, and it's more likely that the code would be parsing
__reserved correctly since it would need to do so in order to find
extra_context.

Anyway, I've reverted the comment change for now in v4, but let me
know what you think.


Peter


The nasty loop in parse_user_sigframe() allows some flexibility
regarding the order of records, but prior to this patch there is no
record that can be actually be moved, due to other backwards
compatibility constraints -- so the flexibility isn't used today.  I'd
like to avoid reorderability creeping in, so that we can get rid of the
loop.

So, mandating that records must be in a consistent order to sigcontext.h
could be helpful.  inserting new records in the middle should be fine,
so long as there is no shuffling.

I'm not sure this patch needs to do anything extra for that: perhaps we
can leave this no-shuffling rule implicit for now (?)

People already get shouted at for needslessly noisy diffs, so there is a
strong disincentive to shuffle existing headers in any case...

[...]

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-07 17:57:41

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v4:
- expose only the tag bits in the context instead of the entire FAR_EL1
- remove mention of the new context from the sigcontext.__reserved[] note

v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 11 +++++-
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 22 +++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 71 insertions(+), 30 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index eab4323609b9..c6e9592a9dea 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst

@@ -53,12 +53,17 @@ visibility.
 Preserving tags
 ---------------
 
-Non-zero tags are not preserved when delivering signals. This means that
-signal handlers in applications making use of tags cannot rely on the
-tag information for user virtual addresses being maintained for fields
-inside siginfo_t. One exception to this rule is for signals raised in
-response to watchpoint debug exceptions, where the tag information will
-be preserved.
+Non-zero tags are not preserved in the fault address fields
+siginfo.si_addr or sigcontext.fault_address when delivering
+signals. This means that signal handlers in applications making use
+of tags cannot rely on the tag information for user virtual addresses
+being maintained in these fields. One exception to this rule is for
+signals raised in response to watchpoint debug exceptions, where the
+tag information will be preserved.
+
+The fault address tag is preserved in the fault_addr_top_byte field of
+the signal frame record fault_addr_top_byte_context, which is present
+for signals raised in response to data aborts and instruction aborts.
 
 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }
 
 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 240fe5e5b720..63185be29ff9 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -144,7 +144,7 @@ struct thread_struct {
 	void			*sve_state;	/* SVE registers, if any */
 	unsigned int		sve_vl;		/* SVE vector length */
 	unsigned int		sve_vl_onexec;	/* SVE vl after next exec */
-	unsigned long		fault_address;	/* fault info */
+	unsigned long		fault_address;	/* FAR_EL1 value */
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..736d6e845b66 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *	0x210		fpsimd_context
  *	 0x10		esr_context
+ *	 0x10		fault_addr_top_byte_context
  *	0x8a0		sve_context (vl <= 64) (optional)
  *	 0x20		extra_context (optional)
  *	 0x10		terminator (null _aarch64_ctx)
  *
- *	0x510		(reserved for future allocation)
+ *	0x500		(reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,6 +95,14 @@ struct esr_context {
 	__u64 esr;
 };
 
+/* Top byte of fault address (normally not exposed via si_addr) */
+#define FAULT_ADDR_TOP_BYTE_MAGIC	0x46544201
+
+struct fault_addr_top_byte_context {
+	struct _aarch64_ctx head;
+	__u8 fault_addr_top_byte;
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c839b5bf1904..045b4f518836 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	local_daif_inherit(regs);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)
 
 	user_exit_irqoff();
 	local_daif_restore(DAIF_PROCCTX);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..baa88dc02e5c 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long ftb_offset;
 	unsigned long sve_offset;
 	unsigned long extra_offset;
 	unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			break;
 
 		case ESR_MAGIC:
+		case FAULT_ADDR_TOP_BYTE_MAGIC:
 			/* ignore */
 			break;

@@ -581,6 +583,12 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 				     sizeof(struct esr_context));
 		if (err)
 			return err;
+
+		err = sigframe_alloc(
+			user, &user->ftb_offset,
+			sizeof(struct fault_addr_top_byte_context));
+		if (err)
+			return err;
 	}
 
 	if (system_supports_sve()) {

@@ -621,7 +629,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	__put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
 	__put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);
 
-	__put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+	__put_user_error(untagged_addr(current->thread.fault_address),
+			 &sf->uc.uc_mcontext.fault_address, err);
 
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));

@@ -641,6 +650,17 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->ftb_offset) {
+		struct fault_addr_top_byte_context __user *ftb_ctx =
+			apply_user_offset(user, user->ftb_offset);
+
+		__put_user_error(FAULT_ADDR_TOP_BYTE_MAGIC,
+				 &ftb_ctx->head.magic, err);
+		__put_user_error(sizeof(*ftb_ctx), &ftb_ctx->head.size, err);
+		__put_user_error(current->thread.fault_address >> 56,
+				 &ftb_ctx->fault_addr_top_byte, err);
+	}
+
 	/* Scalable Vector Extension state, if present */
 	if (system_supports_sve() && err == 0 && user->sve_offset) {
 		struct sve_context __user *sve_ctx =

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c9cedc0432d2..39bbaa05f162 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>
 
 struct fault_info {
-	int	(*fn)(unsigned long addr, unsigned int esr,
+	int	(*fn)(unsigned long far, unsigned int esr,
 		      struct pt_regs *regs);
 	int	sig;
 	int	code;

@@ -320,9 +320,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	die_kernel_fault(msg, addr, esr, regs);
 }
 
-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void set_thread_far_esr(unsigned long far, unsigned int esr)
 {
-	current->thread.fault_address = address;
+	unsigned long addr = untagged_addr(far);
+
+	current->thread.fault_address = far;
 
 	/*
 	 * If the faulting address is in the kernel, we must sanitize the ESR.

@@ -336,7 +338,7 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	 * type", so we ignore this wrinkle and just return the translation
 	 * fault.)
 	 */
-	if (!is_ttbr0_addr(current->thread.fault_address)) {
+	if (!is_ttbr0_addr(addr)) {
 		switch (ESR_ELx_EC(esr)) {
 		case ESR_ELx_EC_DABT_LOW:
 			/*

@@ -377,8 +379,11 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	current->thread.fault_code = esr;
 }
 
-static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static void do_bad_area(unsigned long far, unsigned int esr,
+			struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	/*
 	 * If we are in kernel mode at this point, we have no context to
 	 * handle this fault with.

@@ -386,7 +391,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
 	if (user_mode(regs)) {
 		const struct fault_info *inf = esr_to_fault_info(esr);
 
-		set_thread_esr(addr, esr);
+		set_thread_far_esr(far, esr);
 		arm64_force_sig_fault(inf->sig, inf->code, (void __user *)addr,
 				      inf->name);
 	} else {

@@ -439,7 +444,7 @@ static bool is_write_abort(unsigned int esr)
 	return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM);
 }
 
-static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
+static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 				   struct pt_regs *regs)
 {
 	const struct fault_info *inf;

@@ -447,6 +452,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	vm_fault_t fault, major = 0;
 	unsigned long vm_flags = VM_ACCESS_FLAGS;
 	unsigned int mm_flags = FAULT_FLAG_DEFAULT;
+	unsigned long addr = untagged_addr(far);
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;

@@ -570,7 +576,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	}
 
 	inf = esr_to_fault_info(esr);
-	set_thread_esr(addr, esr);
+	set_thread_far_esr(far, esr);
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to successfully fix up

@@ -605,30 +611,32 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	return 0;
 }
 
-static int __kprobes do_translation_fault(unsigned long addr,
+static int __kprobes do_translation_fault(unsigned long far,
 					  unsigned int esr,
 					  struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	if (is_ttbr0_addr(addr))
-		return do_page_fault(addr, esr, regs);
+		return do_page_fault(far, esr, regs);
 
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_alignment_fault(unsigned long addr, unsigned int esr,
+static int do_alignment_fault(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_bad(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	return 1; /* "fault" */
 }
 
-static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_sea(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf;
 	void __user *siaddr;

@@ -644,7 +652,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	if (esr & ESR_ELx_FnV)
 		siaddr = NULL;
 	else
-		siaddr  = (void __user *)addr;
+		siaddr  = (void __user *)untagged_addr(far);
 	arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
 
 	return 0;

@@ -717,11 +725,12 @@ static const struct fault_info fault_info[] = {
 	{ do_bad,		SIGKILL, SI_KERNEL,	"unknown 63"			},
 };
 
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf = esr_to_fault_info(esr);
+	unsigned long addr = untagged_addr(far);
 
-	if (!inf->fn(addr, esr, regs))
+	if (!inf->fn(far, esr, regs))
 		return;
 
 	if (!user_mode(regs)) {

-- 
2.26.2.526.g744177e7f7-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-08 02:01:26

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v5:
- add padding to fault_addr_top_byte_context in order to ensure the correct
  size and preserve sp alignment

v4:
- expose only the tag bits in the context instead of the entire FAR_EL1
- remove mention of the new context from the sigcontext.__reserved[] note

v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 12 ++++++-
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 22 +++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index eab4323609b9..c6e9592a9dea 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst

@@ -53,12 +53,17 @@ visibility.
 Preserving tags
 ---------------
 
-Non-zero tags are not preserved when delivering signals. This means that
-signal handlers in applications making use of tags cannot rely on the
-tag information for user virtual addresses being maintained for fields
-inside siginfo_t. One exception to this rule is for signals raised in
-response to watchpoint debug exceptions, where the tag information will
-be preserved.
+Non-zero tags are not preserved in the fault address fields
+siginfo.si_addr or sigcontext.fault_address when delivering
+signals. This means that signal handlers in applications making use
+of tags cannot rely on the tag information for user virtual addresses
+being maintained in these fields. One exception to this rule is for
+signals raised in response to watchpoint debug exceptions, where the
+tag information will be preserved.
+
+The fault address tag is preserved in the fault_addr_top_byte field of
+the signal frame record fault_addr_top_byte_context, which is present
+for signals raised in response to data aborts and instruction aborts.
 
 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }
 
 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 240fe5e5b720..63185be29ff9 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -144,7 +144,7 @@ struct thread_struct {
 	void			*sve_state;	/* SVE registers, if any */
 	unsigned int		sve_vl;		/* SVE vector length */
 	unsigned int		sve_vl_onexec;	/* SVE vl after next exec */
-	unsigned long		fault_address;	/* fault info */
+	unsigned long		fault_address;	/* FAR_EL1 value */
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..1a2d23092d8f 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *	0x210		fpsimd_context
  *	 0x10		esr_context
+ *	 0x10		fault_addr_top_byte_context
  *	0x8a0		sve_context (vl <= 64) (optional)
  *	 0x20		extra_context (optional)
  *	 0x10		terminator (null _aarch64_ctx)
  *
- *	0x510		(reserved for future allocation)
+ *	0x500		(reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,6 +95,15 @@ struct esr_context {
 	__u64 esr;
 };
 
+/* Top byte of fault address (normally not exposed via si_addr) */
+#define FAULT_ADDR_TOP_BYTE_MAGIC	0x46544201
+
+struct fault_addr_top_byte_context {
+	struct _aarch64_ctx head;
+	__u8 fault_addr_top_byte;
+	__u8 __reserved[7];
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c839b5bf1904..045b4f518836 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	local_daif_inherit(regs);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)
 
 	user_exit_irqoff();
 	local_daif_restore(DAIF_PROCCTX);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..baa88dc02e5c 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long ftb_offset;
 	unsigned long sve_offset;
 	unsigned long extra_offset;
 	unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			break;
 
 		case ESR_MAGIC:
+		case FAULT_ADDR_TOP_BYTE_MAGIC:
 			/* ignore */
 			break;

@@ -581,6 +583,12 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 				     sizeof(struct esr_context));
 		if (err)
 			return err;
+
+		err = sigframe_alloc(
+			user, &user->ftb_offset,
+			sizeof(struct fault_addr_top_byte_context));
+		if (err)
+			return err;
 	}
 
 	if (system_supports_sve()) {

@@ -621,7 +629,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	__put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
 	__put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);
 
-	__put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+	__put_user_error(untagged_addr(current->thread.fault_address),
+			 &sf->uc.uc_mcontext.fault_address, err);
 
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));

@@ -641,6 +650,17 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->ftb_offset) {
+		struct fault_addr_top_byte_context __user *ftb_ctx =
+			apply_user_offset(user, user->ftb_offset);
+
+		__put_user_error(FAULT_ADDR_TOP_BYTE_MAGIC,
+				 &ftb_ctx->head.magic, err);
+		__put_user_error(sizeof(*ftb_ctx), &ftb_ctx->head.size, err);
+		__put_user_error(current->thread.fault_address >> 56,
+				 &ftb_ctx->fault_addr_top_byte, err);
+	}
+
 	/* Scalable Vector Extension state, if present */
 	if (system_supports_sve() && err == 0 && user->sve_offset) {
 		struct sve_context __user *sve_ctx =

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c9cedc0432d2..39bbaa05f162 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>
 
 struct fault_info {
-	int	(*fn)(unsigned long addr, unsigned int esr,
+	int	(*fn)(unsigned long far, unsigned int esr,
 		      struct pt_regs *regs);
 	int	sig;
 	int	code;

@@ -320,9 +320,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	die_kernel_fault(msg, addr, esr, regs);
 }
 
-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void set_thread_far_esr(unsigned long far, unsigned int esr)
 {
-	current->thread.fault_address = address;
+	unsigned long addr = untagged_addr(far);
+
+	current->thread.fault_address = far;
 
 	/*
 	 * If the faulting address is in the kernel, we must sanitize the ESR.

@@ -336,7 +338,7 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	 * type", so we ignore this wrinkle and just return the translation
 	 * fault.)
 	 */
-	if (!is_ttbr0_addr(current->thread.fault_address)) {
+	if (!is_ttbr0_addr(addr)) {
 		switch (ESR_ELx_EC(esr)) {
 		case ESR_ELx_EC_DABT_LOW:
 			/*

@@ -377,8 +379,11 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	current->thread.fault_code = esr;
 }
 
-static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static void do_bad_area(unsigned long far, unsigned int esr,
+			struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	/*
 	 * If we are in kernel mode at this point, we have no context to
 	 * handle this fault with.

@@ -386,7 +391,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
 	if (user_mode(regs)) {
 		const struct fault_info *inf = esr_to_fault_info(esr);
 
-		set_thread_esr(addr, esr);
+		set_thread_far_esr(far, esr);
 		arm64_force_sig_fault(inf->sig, inf->code, (void __user *)addr,
 				      inf->name);
 	} else {

@@ -439,7 +444,7 @@ static bool is_write_abort(unsigned int esr)
 	return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM);
 }
 
-static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
+static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 				   struct pt_regs *regs)
 {
 	const struct fault_info *inf;

@@ -447,6 +452,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	vm_fault_t fault, major = 0;
 	unsigned long vm_flags = VM_ACCESS_FLAGS;
 	unsigned int mm_flags = FAULT_FLAG_DEFAULT;
+	unsigned long addr = untagged_addr(far);
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;

@@ -570,7 +576,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	}
 
 	inf = esr_to_fault_info(esr);
-	set_thread_esr(addr, esr);
+	set_thread_far_esr(far, esr);
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to successfully fix up

@@ -605,30 +611,32 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	return 0;
 }
 
-static int __kprobes do_translation_fault(unsigned long addr,
+static int __kprobes do_translation_fault(unsigned long far,
 					  unsigned int esr,
 					  struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	if (is_ttbr0_addr(addr))
-		return do_page_fault(addr, esr, regs);
+		return do_page_fault(far, esr, regs);
 
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_alignment_fault(unsigned long addr, unsigned int esr,
+static int do_alignment_fault(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_bad(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	return 1; /* "fault" */
 }
 
-static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_sea(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf;
 	void __user *siaddr;

@@ -644,7 +652,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	if (esr & ESR_ELx_FnV)
 		siaddr = NULL;
 	else
-		siaddr  = (void __user *)addr;
+		siaddr  = (void __user *)untagged_addr(far);
 	arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
 
 	return 0;

@@ -717,11 +725,12 @@ static const struct fault_info fault_info[] = {
 	{ do_bad,		SIGKILL, SI_KERNEL,	"unknown 63"			},
 };
 
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf = esr_to_fault_info(esr);
+	unsigned long addr = untagged_addr(far);
 
-	if (!inf->fn(addr, esr, regs))
+	if (!inf->fn(far, esr, regs))
 		return;
 
 	if (!user_mode(regs)) {

-- 
2.26.2.645.ge9eca65c58-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2020-05-12 16:25:35

On Thu, May 07, 2020 at 07:01:06PM -0700, Peter Collingbourne wrote:

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>

It looks alright to me.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Dave Martin <Dave.Martin@arm.com>
Date: 2020-05-13 17:27:55

On Thu, May 07, 2020 at 10:55:02AM -0700, Peter Collingbourne wrote:

On Mon, May 4, 2020 at 3:19 AM Dave Martin [off-list ref] wrote:

quoted

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 21 +++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 20 ++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 74 insertions(+), 35 deletions(-)

[...]

quoted

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..6782394633cb 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *   0x210           fpsimd_context
  *    0x10           esr_context
+ *    0x10           far_context
  *   0x8a0           sve_context (vl <= 64) (optional)
  *    0x20           extra_context (optional)
  *    0x10           terminator (null _aarch64_ctx)
  *
- *   0x510           (reserved for future allocation)
+ *   0x500           (reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,25 @@ struct esr_context {
      __u64 esr;
 };

+/* FAR_EL1 context */
+#define FAR_MAGIC    0x46415201
+
+struct far_context {
+     struct _aarch64_ctx head;
+     __u64 far;
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, far_context and extra_context must be
+ * placed in sigcontext.__reserved[] if present.  They cannot be placed
+ * in the extra space.  Any other record can be placed either in the
+ * extra space or in sigcontext.__reserved[], unless otherwise specified
+ * in this file.

This is for backwards compatibility only.  We don't need this constraint
for any new field, so you can probably leave the paragraph as-is.

Removing this would mean constraint would mean that userspace must be
prepared to traverse extra_context when looking for far_context.  But
really we want modern userspace to do this anyway, since it reduces
backwards compatibilty worries when adding more new records in the
future.

My original reason for updating this comment was that I figured that
this record was small enough that we could just always include it in
__reserved.

But thinking about this a bit more, it doesn't seem that just wanting
userspace to read extra_context will guarantee that it will do so. In
practice, it would be easy to write userspace code that works right
now but doesn't read extra_context correctly (either because
extra_context wasn't considered at all, or because the code purporting
to read the record from extra_context contains a latent bug because it
wasn't exercised). Since we may be practically constrained from moving
the record anyway, we might as well document it and allow the
userspace code to be a little simpler.

I guess one alternative is that we always place this record in
extra_context, which would force userspace to read it correctly. That
has something of the opposite problem (userspace code could be written
to only expect the record in extra_context), but at least we're less
constrained there, and it's more likely that the code would be parsing
__reserved correctly since it would need to do so in order to find
extra_context.

Anyway, I've reverted the comment change for now in v4, but let me
know what you think.

Apologies for the delay in responding -- I think it does make sense to
reserve space in __reserved[] for the new record, the the location you
suggested for it is sensible.

__reserved[] is a scarce resource, and should only be burned on "small"
records, but far_context is small.


here's another reason too, which is that we don't want to needlessly
block new software from using this field without allocating larger
stacks -- not least because they just won't, and the problem won't
bite them until much later.


Hope that helps clarify things.

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-13 18:00:28

On Wed, May 13, 2020 at 10:27 AM Dave Martin [off-list ref] wrote:

On Thu, May 07, 2020 at 10:55:02AM -0700, Peter Collingbourne wrote:

quoted

On Mon, May 4, 2020 at 3:19 AM Dave Martin [off-list ref] wrote:

quoted

On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address
fields, because there may be existing userspace applications that are
expecting the tag bits to be cleared. Instead, create a far_context in
sigcontext (similar to the existing esr_context), and store the original
value of FAR_EL1 (including the tag bits) there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 21 +++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 20 ++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 74 insertions(+), 35 deletions(-)

[...]

quoted

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..6782394633cb 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *   0x210           fpsimd_context
  *    0x10           esr_context
+ *    0x10           far_context
  *   0x8a0           sve_context (vl <= 64) (optional)
  *    0x20           extra_context (optional)
  *    0x10           terminator (null _aarch64_ctx)
  *
- *   0x510           (reserved for future allocation)
+ *   0x500           (reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,25 @@ struct esr_context {
      __u64 esr;
 };

+/* FAR_EL1 context */
+#define FAR_MAGIC    0x46415201
+
+struct far_context {
+     struct _aarch64_ctx head;
+     __u64 far;
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, far_context and extra_context must be
+ * placed in sigcontext.__reserved[] if present.  They cannot be placed
+ * in the extra space.  Any other record can be placed either in the
+ * extra space or in sigcontext.__reserved[], unless otherwise specified
+ * in this file.

This is for backwards compatibility only.  We don't need this constraint
for any new field, so you can probably leave the paragraph as-is.

Removing this would mean constraint would mean that userspace must be
prepared to traverse extra_context when looking for far_context.  But
really we want modern userspace to do this anyway, since it reduces
backwards compatibilty worries when adding more new records in the
future.

My original reason for updating this comment was that I figured that
this record was small enough that we could just always include it in
__reserved.

But thinking about this a bit more, it doesn't seem that just wanting
userspace to read extra_context will guarantee that it will do so. In
practice, it would be easy to write userspace code that works right
now but doesn't read extra_context correctly (either because
extra_context wasn't considered at all, or because the code purporting
to read the record from extra_context contains a latent bug because it
wasn't exercised). Since we may be practically constrained from moving
the record anyway, we might as well document it and allow the
userspace code to be a little simpler.

I guess one alternative is that we always place this record in
extra_context, which would force userspace to read it correctly. That
has something of the opposite problem (userspace code could be written
to only expect the record in extra_context), but at least we're less
constrained there, and it's more likely that the code would be parsing
__reserved correctly since it would need to do so in order to find
extra_context.

Anyway, I've reverted the comment change for now in v4, but let me
know what you think.

Apologies for the delay in responding -- I think it does make sense to
reserve space in __reserved[] for the new record, the the location you
suggested for it is sensible.

__reserved[] is a scarce resource, and should only be burned on "small"
records, but far_context is small.


here's another reason too, which is that we don't want to needlessly
block new software from using this field without allocating larger
stacks -- not least because they just won't, and the problem won't
bite them until much later.


Hope that helps clarify things.

Thanks, that makes sense. I will send a v6 with the comment brought back.

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-13 18:09:31

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v6:
- bring back comment about __reserved[]

v5:
- add padding to fault_addr_top_byte_context in order to ensure the correct
  size and preserve sp alignment

v4:
- expose only the tag bits in the context instead of the entire FAR_EL1
- remove mention of the new context from the sigcontext.__reserved[] note

v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 22 ++++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 22 +++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 77 insertions(+), 35 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index eab4323609b9..c6e9592a9dea 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst

@@ -53,12 +53,17 @@ visibility.
 Preserving tags
 ---------------
 
-Non-zero tags are not preserved when delivering signals. This means that
-signal handlers in applications making use of tags cannot rely on the
-tag information for user virtual addresses being maintained for fields
-inside siginfo_t. One exception to this rule is for signals raised in
-response to watchpoint debug exceptions, where the tag information will
-be preserved.
+Non-zero tags are not preserved in the fault address fields
+siginfo.si_addr or sigcontext.fault_address when delivering
+signals. This means that signal handlers in applications making use
+of tags cannot rely on the tag information for user virtual addresses
+being maintained in these fields. One exception to this rule is for
+signals raised in response to watchpoint debug exceptions, where the
+tag information will be preserved.
+
+The fault address tag is preserved in the fault_addr_top_byte field of
+the signal frame record fault_addr_top_byte_context, which is present
+for signals raised in response to data aborts and instruction aborts.
 
 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }
 
 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 240fe5e5b720..63185be29ff9 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -144,7 +144,7 @@ struct thread_struct {
 	void			*sve_state;	/* SVE registers, if any */
 	unsigned int		sve_vl;		/* SVE vector length */
 	unsigned int		sve_vl_onexec;	/* SVE vl after next exec */
-	unsigned long		fault_address;	/* fault info */
+	unsigned long		fault_address;	/* FAR_EL1 value */
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..2a3fe3de899d 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *	0x210		fpsimd_context
  *	 0x10		esr_context
+ *	 0x10		fault_addr_top_byte_context
  *	0x8a0		sve_context (vl <= 64) (optional)
  *	 0x20		extra_context (optional)
  *	 0x10		terminator (null _aarch64_ctx)
  *
- *	0x510		(reserved for future allocation)
+ *	0x500		(reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,26 @@ struct esr_context {
 	__u64 esr;
 };
 
+/* Top byte of fault address (normally not exposed via si_addr) */
+#define FAULT_ADDR_TOP_BYTE_MAGIC	0x46544201
+
+struct fault_addr_top_byte_context {
+	struct _aarch64_ctx head;
+	__u8 fault_addr_top_byte;
+	__u8 __reserved[7];
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, fault_addr_top_byte_context and
+ * extra_context must be placed in sigcontext.__reserved[] if present.
+ * They cannot be placed in the extra space.  Any other record can be
+ * placed either in the extra space or in sigcontext.__reserved[],
+ * unless otherwise specified in this file.
  *
  * 2) There must not be more than one extra_context.
  *

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c839b5bf1904..045b4f518836 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	local_daif_inherit(regs);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)
 
 	user_exit_irqoff();
 	local_daif_restore(DAIF_PROCCTX);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..baa88dc02e5c 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long ftb_offset;
 	unsigned long sve_offset;
 	unsigned long extra_offset;
 	unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			break;
 
 		case ESR_MAGIC:
+		case FAULT_ADDR_TOP_BYTE_MAGIC:
 			/* ignore */
 			break;

@@ -581,6 +583,12 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 				     sizeof(struct esr_context));
 		if (err)
 			return err;
+
+		err = sigframe_alloc(
+			user, &user->ftb_offset,
+			sizeof(struct fault_addr_top_byte_context));
+		if (err)
+			return err;
 	}
 
 	if (system_supports_sve()) {

@@ -621,7 +629,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	__put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
 	__put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);
 
-	__put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+	__put_user_error(untagged_addr(current->thread.fault_address),
+			 &sf->uc.uc_mcontext.fault_address, err);
 
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));

@@ -641,6 +650,17 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->ftb_offset) {
+		struct fault_addr_top_byte_context __user *ftb_ctx =
+			apply_user_offset(user, user->ftb_offset);
+
+		__put_user_error(FAULT_ADDR_TOP_BYTE_MAGIC,
+				 &ftb_ctx->head.magic, err);
+		__put_user_error(sizeof(*ftb_ctx), &ftb_ctx->head.size, err);
+		__put_user_error(current->thread.fault_address >> 56,
+				 &ftb_ctx->fault_addr_top_byte, err);
+	}
+
 	/* Scalable Vector Extension state, if present */
 	if (system_supports_sve() && err == 0 && user->sve_offset) {
 		struct sve_context __user *sve_ctx =

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c9cedc0432d2..39bbaa05f162 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>
 
 struct fault_info {
-	int	(*fn)(unsigned long addr, unsigned int esr,
+	int	(*fn)(unsigned long far, unsigned int esr,
 		      struct pt_regs *regs);
 	int	sig;
 	int	code;

@@ -320,9 +320,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	die_kernel_fault(msg, addr, esr, regs);
 }
 
-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void set_thread_far_esr(unsigned long far, unsigned int esr)
 {
-	current->thread.fault_address = address;
+	unsigned long addr = untagged_addr(far);
+
+	current->thread.fault_address = far;
 
 	/*
 	 * If the faulting address is in the kernel, we must sanitize the ESR.

@@ -336,7 +338,7 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	 * type", so we ignore this wrinkle and just return the translation
 	 * fault.)
 	 */
-	if (!is_ttbr0_addr(current->thread.fault_address)) {
+	if (!is_ttbr0_addr(addr)) {
 		switch (ESR_ELx_EC(esr)) {
 		case ESR_ELx_EC_DABT_LOW:
 			/*

@@ -377,8 +379,11 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	current->thread.fault_code = esr;
 }
 
-static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static void do_bad_area(unsigned long far, unsigned int esr,
+			struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	/*
 	 * If we are in kernel mode at this point, we have no context to
 	 * handle this fault with.

@@ -386,7 +391,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
 	if (user_mode(regs)) {
 		const struct fault_info *inf = esr_to_fault_info(esr);
 
-		set_thread_esr(addr, esr);
+		set_thread_far_esr(far, esr);
 		arm64_force_sig_fault(inf->sig, inf->code, (void __user *)addr,
 				      inf->name);
 	} else {

@@ -439,7 +444,7 @@ static bool is_write_abort(unsigned int esr)
 	return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM);
 }
 
-static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
+static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 				   struct pt_regs *regs)
 {
 	const struct fault_info *inf;

@@ -447,6 +452,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	vm_fault_t fault, major = 0;
 	unsigned long vm_flags = VM_ACCESS_FLAGS;
 	unsigned int mm_flags = FAULT_FLAG_DEFAULT;
+	unsigned long addr = untagged_addr(far);
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;

@@ -570,7 +576,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	}
 
 	inf = esr_to_fault_info(esr);
-	set_thread_esr(addr, esr);
+	set_thread_far_esr(far, esr);
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to successfully fix up

@@ -605,30 +611,32 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	return 0;
 }
 
-static int __kprobes do_translation_fault(unsigned long addr,
+static int __kprobes do_translation_fault(unsigned long far,
 					  unsigned int esr,
 					  struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	if (is_ttbr0_addr(addr))
-		return do_page_fault(addr, esr, regs);
+		return do_page_fault(far, esr, regs);
 
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_alignment_fault(unsigned long addr, unsigned int esr,
+static int do_alignment_fault(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_bad(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	return 1; /* "fault" */
 }
 
-static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_sea(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf;
 	void __user *siaddr;

@@ -644,7 +652,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	if (esr & ESR_ELx_FnV)
 		siaddr = NULL;
 	else
-		siaddr  = (void __user *)addr;
+		siaddr  = (void __user *)untagged_addr(far);
 	arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
 
 	return 0;

@@ -717,11 +725,12 @@ static const struct fault_info fault_info[] = {
 	{ do_bad,		SIGKILL, SI_KERNEL,	"unknown 63"			},
 };
 
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf = esr_to_fault_info(esr);
+	unsigned long addr = untagged_addr(far);
 
-	if (!inf->fn(addr, esr, regs))
+	if (!inf->fn(far, esr, regs))
 		return;
 
 	if (!user_mode(regs)) {

-- 
2.26.2.645.ge9eca65c58-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Dave Martin <Dave.Martin@arm.com>
Date: 2020-05-13 20:28:28

On Wed, May 13, 2020 at 11:09:14AM -0700, Peter Collingbourne wrote:

quoted hunk

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v6:
- bring back comment about __reserved[]

v5:
- add padding to fault_addr_top_byte_context in order to ensure the correct
  size and preserve sp alignment

v4:
- expose only the tag bits in the context instead of the entire FAR_EL1
- remove mention of the new context from the sigcontext.__reserved[] note

v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 22 ++++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 22 +++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 77 insertions(+), 35 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index eab4323609b9..c6e9592a9dea 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst

@@ -53,12 +53,17 @@ visibility.
 Preserving tags
 ---------------
 
-Non-zero tags are not preserved when delivering signals. This means that
-signal handlers in applications making use of tags cannot rely on the
-tag information for user virtual addresses being maintained for fields
-inside siginfo_t. One exception to this rule is for signals raised in
-response to watchpoint debug exceptions, where the tag information will
-be preserved.
+Non-zero tags are not preserved in the fault address fields
+siginfo.si_addr or sigcontext.fault_address when delivering
+signals. This means that signal handlers in applications making use
+of tags cannot rely on the tag information for user virtual addresses
+being maintained in these fields. One exception to this rule is for
+signals raised in response to watchpoint debug exceptions, where the
+tag information will be preserved.
+
+The fault address tag is preserved in the fault_addr_top_byte field of
+the signal frame record fault_addr_top_byte_context, which is present
+for signals raised in response to data aborts and instruction aborts.
 
 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }
 
 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 240fe5e5b720..63185be29ff9 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -144,7 +144,7 @@ struct thread_struct {
 	void			*sve_state;	/* SVE registers, if any */
 	unsigned int		sve_vl;		/* SVE vector length */
 	unsigned int		sve_vl_onexec;	/* SVE vl after next exec */
-	unsigned long		fault_address;	/* fault info */
+	unsigned long		fault_address;	/* FAR_EL1 value */
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..2a3fe3de899d 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *	0x210		fpsimd_context
  *	 0x10		esr_context
+ *	 0x10		fault_addr_top_byte_context
  *	0x8a0		sve_context (vl <= 64) (optional)
  *	 0x20		extra_context (optional)
  *	 0x10		terminator (null _aarch64_ctx)
  *
- *	0x510		(reserved for future allocation)
+ *	0x500		(reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,26 @@ struct esr_context {
 	__u64 esr;
 };
 
+/* Top byte of fault address (normally not exposed via si_addr) */
+#define FAULT_ADDR_TOP_BYTE_MAGIC	0x46544201
+
+struct fault_addr_top_byte_context {
+	struct _aarch64_ctx head;
+	__u8 fault_addr_top_byte;
+	__u8 __reserved[7];
+};
+

Nit: the name here is a bit cumbersome (obviously bikeshedding...)


For the rest, some of my comments may be bogus -- I haven't dug into
this stuff for a little while!


Anyway:

Do we really get the whole top byte of the address in the FAR?  If so,
fine, but I'm having trouble finding a clear statement in the
architecture one way or the other.  (I didn't attempt to dive into the
pseudocode.)


Also, since we're burning 16 bytes here, I'd prefer if we make this
extensible.  At present the __reserved[7] is unusable because
userspace has no way to know whether it's valid or not.

Options include an additional flag byte (0 for now), or just making
the whole thing a __u64.  In that case we can leave the top byte bits
in their original positions if we want, but it would be a good idea to
include a flag to say that field is valid at all.  (See comments below
on Synchronous external abort.)

So, say, foo_context->fault_info = (esr & (~0ULL << 56)) | TOP_BYTE_VALID.
(with #defines for the bits/fields as appropriate).

quoted hunk

 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, fault_addr_top_byte_context and
+ * extra_context must be placed in sigcontext.__reserved[] if present.
+ * They cannot be placed in the extra space.  Any other record can be
+ * placed either in the extra space or in sigcontext.__reserved[],
+ * unless otherwise specified in this file.
  *
  * 2) There must not be more than one extra_context.
  *

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c839b5bf1904..045b4f518836 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	local_daif_inherit(regs);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)
 
 	user_exit_irqoff();
 	local_daif_restore(DAIF_PROCCTX);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..baa88dc02e5c 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long ftb_offset;
 	unsigned long sve_offset;
 	unsigned long extra_offset;
 	unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			break;
 
 		case ESR_MAGIC:
+		case FAULT_ADDR_TOP_BYTE_MAGIC:
 			/* ignore */
 			break;

@@ -581,6 +583,12 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 				     sizeof(struct esr_context));
 		if (err)
 			return err;
+
+		err = sigframe_alloc(
+			user, &user->ftb_offset,
+			sizeof(struct fault_addr_top_byte_context));

Nit: inconsistent indentation?

(Mostly just because it makes the change look odd against the hunk
context, but not a big deal.)

quoted hunk

+		if (err)
+			return err;
 	}
 
 	if (system_supports_sve()) {

@@ -621,7 +629,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	__put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
 	__put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);
 
-	__put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+	__put_user_error(untagged_addr(current->thread.fault_address),
+			 &sf->uc.uc_mcontext.fault_address, err);
 
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
 
@@ -641,6 +650,17 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->ftb_offset) {
+		struct fault_addr_top_byte_context __user *ftb_ctx =
+			apply_user_offset(user, user->ftb_offset);
+
+		__put_user_error(FAULT_ADDR_TOP_BYTE_MAGIC,
+				 &ftb_ctx->head.magic, err);
+		__put_user_error(sizeof(*ftb_ctx), &ftb_ctx->head.size, err);
+		__put_user_error(current->thread.fault_address >> 56,
+				 &ftb_ctx->fault_addr_top_byte, err);
+	}
+

How do we handle the fact that the top byte of FAR is sometimes UNKNOWN?

For Synchronous external aborts in particular, those bits are documented
as UNKNOWN, but I don't see any special handling,  There may be other
cases I haven't spotted.

For preference we can omit this record entirely if we don't have any
information we can report, but certainly we shouldn't expose UNKNOWN
bits.


[ Aside:

Also, what if we're not reporting a memory abort at all?  Does
thread.fault_address just contain junk from the last fault?  I see
nothing anywhere that cleans this up.  (This is historical and not
your fault, but it would be good to close this down while we're about
it.)


Hmmm, looking at the code I think we probably leak fault_address etc.
across execve() too, so it may even be stale junk from an old process
:/

Maybe I just confused myself. 

End aside. ]


Apart from these issues, the actual code looks reasonable.

Cheers
---Dave

quoted hunk

 	/* Scalable Vector Extension state, if present */
 	if (system_supports_sve() && err == 0 && user->sve_offset) {
 		struct sve_context __user *sve_ctx =

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c9cedc0432d2..39bbaa05f162 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>
 
 struct fault_info {
-	int	(*fn)(unsigned long addr, unsigned int esr,
+	int	(*fn)(unsigned long far, unsigned int esr,
 		      struct pt_regs *regs);
 	int	sig;
 	int	code;

@@ -320,9 +320,11 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	die_kernel_fault(msg, addr, esr, regs);
 }
 
-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void set_thread_far_esr(unsigned long far, unsigned int esr)
 {
-	current->thread.fault_address = address;
+	unsigned long addr = untagged_addr(far);
+
+	current->thread.fault_address = far;
 
 	/*
 	 * If the faulting address is in the kernel, we must sanitize the ESR.

@@ -336,7 +338,7 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	 * type", so we ignore this wrinkle and just return the translation
 	 * fault.)
 	 */
-	if (!is_ttbr0_addr(current->thread.fault_address)) {
+	if (!is_ttbr0_addr(addr)) {
 		switch (ESR_ELx_EC(esr)) {
 		case ESR_ELx_EC_DABT_LOW:
 			/*

@@ -377,8 +379,11 @@ static void set_thread_esr(unsigned long address, unsigned int esr)
 	current->thread.fault_code = esr;
 }
 
-static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static void do_bad_area(unsigned long far, unsigned int esr,
+			struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	/*
 	 * If we are in kernel mode at this point, we have no context to
 	 * handle this fault with.

@@ -386,7 +391,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
 	if (user_mode(regs)) {
 		const struct fault_info *inf = esr_to_fault_info(esr);
 
-		set_thread_esr(addr, esr);
+		set_thread_far_esr(far, esr);
 		arm64_force_sig_fault(inf->sig, inf->code, (void __user *)addr,
 				      inf->name);
 	} else {

@@ -439,7 +444,7 @@ static bool is_write_abort(unsigned int esr)
 	return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM);
 }
 
-static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
+static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 				   struct pt_regs *regs)
 {
 	const struct fault_info *inf;

@@ -447,6 +452,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	vm_fault_t fault, major = 0;
 	unsigned long vm_flags = VM_ACCESS_FLAGS;
 	unsigned int mm_flags = FAULT_FLAG_DEFAULT;
+	unsigned long addr = untagged_addr(far);
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;

@@ -570,7 +576,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	}
 
 	inf = esr_to_fault_info(esr);
-	set_thread_esr(addr, esr);
+	set_thread_far_esr(far, esr);
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to successfully fix up

@@ -605,30 +611,32 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	return 0;
 }
 
-static int __kprobes do_translation_fault(unsigned long addr,
+static int __kprobes do_translation_fault(unsigned long far,
 					  unsigned int esr,
 					  struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	if (is_ttbr0_addr(addr))
-		return do_page_fault(addr, esr, regs);
+		return do_page_fault(far, esr, regs);
 
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_alignment_fault(unsigned long addr, unsigned int esr,
+static int do_alignment_fault(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_bad(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	return 1; /* "fault" */
 }
 
-static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_sea(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf;
 	void __user *siaddr;

@@ -644,7 +652,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	if (esr & ESR_ELx_FnV)
 		siaddr = NULL;
 	else
-		siaddr  = (void __user *)addr;
+		siaddr  = (void __user *)untagged_addr(far);
 	arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
 
 	return 0;

@@ -717,11 +725,12 @@ static const struct fault_info fault_info[] = {
 	{ do_bad,		SIGKILL, SI_KERNEL,	"unknown 63"			},
 };
 
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf = esr_to_fault_info(esr);
+	unsigned long addr = untagged_addr(far);
 
-	if (!inf->fn(addr, esr, regs))
+	if (!inf->fn(far, esr, regs))
 		return;
 
 	if (!user_mode(regs)) {

-- 
2.26.2.645.ge9eca65c58-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-15 00:58:52

On Wed, May 13, 2020 at 1:28 PM Dave Martin [off-list ref] wrote:

On Wed, May 13, 2020 at 11:09:14AM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v6:
- bring back comment about __reserved[]

v5:
- add padding to fault_addr_top_byte_context in order to ensure the correct
  size and preserve sp alignment

v4:
- expose only the tag bits in the context instead of the entire FAR_EL1
- remove mention of the new context from the sigcontext.__reserved[] note

v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 22 ++++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 22 +++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 77 insertions(+), 35 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index eab4323609b9..c6e9592a9dea 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst

@@ -53,12 +53,17 @@ visibility.
 Preserving tags
 ---------------

-Non-zero tags are not preserved when delivering signals. This means that
-signal handlers in applications making use of tags cannot rely on the
-tag information for user virtual addresses being maintained for fields
-inside siginfo_t. One exception to this rule is for signals raised in
-response to watchpoint debug exceptions, where the tag information will
-be preserved.
+Non-zero tags are not preserved in the fault address fields
+siginfo.si_addr or sigcontext.fault_address when delivering
+signals. This means that signal handlers in applications making use
+of tags cannot rely on the tag information for user virtual addresses
+being maintained in these fields. One exception to this rule is for
+signals raised in response to watchpoint debug exceptions, where the
+tag information will be preserved.
+
+The fault address tag is preserved in the fault_addr_top_byte field of
+the signal frame record fault_addr_top_byte_context, which is present
+for signals raised in response to data aborts and instruction aborts.

 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }

 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 240fe5e5b720..63185be29ff9 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -144,7 +144,7 @@ struct thread_struct {
      void                    *sve_state;     /* SVE registers, if any */
      unsigned int            sve_vl;         /* SVE vector length */
      unsigned int            sve_vl_onexec;  /* SVE vl after next exec */
-     unsigned long           fault_address;  /* fault info */
+     unsigned long           fault_address;  /* FAR_EL1 value */
      unsigned long           fault_code;     /* ESR_EL1 value */
      struct debug_info       debug;          /* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..2a3fe3de899d 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *   0x210           fpsimd_context
  *    0x10           esr_context
+ *    0x10           fault_addr_top_byte_context
  *   0x8a0           sve_context (vl <= 64) (optional)
  *    0x20           extra_context (optional)
  *    0x10           terminator (null _aarch64_ctx)
  *
- *   0x510           (reserved for future allocation)
+ *   0x500           (reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,26 @@ struct esr_context {
      __u64 esr;
 };

+/* Top byte of fault address (normally not exposed via si_addr) */
+#define FAULT_ADDR_TOP_BYTE_MAGIC    0x46544201
+
+struct fault_addr_top_byte_context {
+     struct _aarch64_ctx head;
+     __u8 fault_addr_top_byte;
+     __u8 __reserved[7];
+};
+

Nit: the name here is a bit cumbersome (obviously bikeshedding...)


For the rest, some of my comments may be bogus -- I haven't dug into
this stuff for a little while!


Anyway:

Do we really get the whole top byte of the address in the FAR?  If so,
fine, but I'm having trouble finding a clear statement in the
architecture one way or the other.  (I didn't attempt to dive into the
pseudocode.)

I rely on this statement in the ARM:

https://developer.arm.com/docs/ddi0595/b/aarch64-system-registers/far_el1
"For a Data Abort or Watchpoint exception, if address tagging is
enabled for the address accessed by the data access that caused the
exception, then this field includes the tag."

And note that address tagging here essentially means TBI (which is
always enabled on Linux), and not memory tagging.

Also, since we're burning 16 bytes here, I'd prefer if we make this
extensible.  At present the __reserved[7] is unusable because
userspace has no way to know whether it's valid or not.

Options include an additional flag byte (0 for now), or just making
the whole thing a __u64.  In that case we can leave the top byte bits
in their original positions if we want, but it would be a good idea to
include a flag to say that field is valid at all.  (See comments below
on Synchronous external abort.)

So, say, foo_context->fault_info = (esr & (~0ULL << 56)) | TOP_BYTE_VALID.
(with #defines for the bits/fields as appropriate).

The flag bits seem like a good idea. Thinking ahead to the MTE sync
tag fault (which might not provide us with bits 60-63), we may
consider having separate bits to indicate "bits 56-59 valid" and "bits
60-63 valid", set both bits for regular data aborts and only the
former for sync tag faults, which would avoid the need to define a
separate context for these faults. And if a future architecture
revision provides us with bits 60-63 for tag faults, we could start
setting both flag bits even for tag faults.

quoted

 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, fault_addr_top_byte_context and
+ * extra_context must be placed in sigcontext.__reserved[] if present.
+ * They cannot be placed in the extra space.  Any other record can be
+ * placed either in the extra space or in sigcontext.__reserved[],
+ * unless otherwise specified in this file.
  *
  * 2) There must not be more than one extra_context.
  *

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c839b5bf1904..045b4f518836 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
      unsigned long far = read_sysreg(far_el1);

      local_daif_inherit(regs);
-     far = untagged_addr(far);
      do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)

      user_exit_irqoff();
      local_daif_restore(DAIF_PROCCTX);
-     far = untagged_addr(far);
      do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..baa88dc02e5c 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {

      unsigned long fpsimd_offset;
      unsigned long esr_offset;
+     unsigned long ftb_offset;
      unsigned long sve_offset;
      unsigned long extra_offset;
      unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
                      break;

              case ESR_MAGIC:
+             case FAULT_ADDR_TOP_BYTE_MAGIC:
                      /* ignore */
                      break;

@@ -581,6 +583,12 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
                                   sizeof(struct esr_context));
              if (err)
                      return err;
+
+             err = sigframe_alloc(
+                     user, &user->ftb_offset,
+                     sizeof(struct fault_addr_top_byte_context));

Nit: inconsistent indentation?

(Mostly just because it makes the change look odd against the hunk
context, but not a big deal.)

With consistent indentation we violate 80 cols due to the extra long
struct name. The indentation is what clang-format is giving me.

quoted

+             if (err)
+                     return err;
      }

      if (system_supports_sve()) {

@@ -621,7 +629,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
      __put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
      __put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);

-     __put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+     __put_user_error(untagged_addr(current->thread.fault_address),
+                      &sf->uc.uc_mcontext.fault_address, err);

      err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
@@ -641,6 +650,17 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
              __put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
      }

+     if (err == 0 && user->ftb_offset) {
+             struct fault_addr_top_byte_context __user *ftb_ctx =
+                     apply_user_offset(user, user->ftb_offset);
+
+             __put_user_error(FAULT_ADDR_TOP_BYTE_MAGIC,
+                              &ftb_ctx->head.magic, err);
+             __put_user_error(sizeof(*ftb_ctx), &ftb_ctx->head.size, err);
+             __put_user_error(current->thread.fault_address >> 56,
+                              &ftb_ctx->fault_addr_top_byte, err);
+     }
+

How do we handle the fact that the top byte of FAR is sometimes UNKNOWN?

For Synchronous external aborts in particular, those bits are documented
as UNKNOWN, but I don't see any special handling,  There may be other
cases I haven't spotted.

For preference we can omit this record entirely if we don't have any
information we can report, but certainly we shouldn't expose UNKNOWN
bits.

In this case we mask out the top byte in do_sea before passing the
address to arm64_notify_die (which clears fault_address and passes the
address argument on to arm64_force_sig_fault to be exposed via
si_addr). So the record would always contain a 0 byte. It seems
reasonable to omit the record in this case instead.

[ Aside:

Also, what if we're not reporting a memory abort at all?  Does
thread.fault_address just contain junk from the last fault?  I see
nothing anywhere that cleans this up.  (This is historical and not
your fault, but it would be good to close this down while we're about
it.)


Hmmm, looking at the code I think we probably leak fault_address etc.
across execve() too, so it may even be stale junk from an old process
:/

Maybe I just confused myself.

End aside. ]

Yes, it's unclear whether we always manage to not expose a fault
address if we're not reporting a data or instruction abort. The code
would need to arrange for fault_code to be set to 0 in order to avoid
exposing previous fault_address values via future signals. I don't see
anywhere where we're resetting these fields after delivering a signal,
so it seems possible by calling arm64_force_sig_fault without first
setting fault_code (most callers do this, but the calls in
arch/arm64/kernel/debug-monitors.c and arch/arm64/kernel/ptrace.c seem
not to), or simply by calling force_sig_fault (which happens in many
places throughout the kernel).

Maybe something like this would do the trick? (Untested, and forgive
spaces instead of tabs, grumble grumble gmail):

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index baa88dc02e5c..5867f2fdbe64 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -648,6 +648,7 @@ static int setup_sigframe(struct

rt_sigframe_user_layout *user,
                __put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
                __put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
                __put_user_error(current->thread.fault_code,
&esr_ctx->esr, err);
+               current->thread.fault_code = 0;
        }

        if (err == 0 && user->ftb_offset) {

Apart from these issues, the actual code looks reasonable.

Thanks for the review.

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Dave Martin <Dave.Martin@arm.com>
Date: 2020-05-18 09:53:26

On Thu, May 14, 2020 at 05:58:21PM -0700, Peter Collingbourne wrote:

On Wed, May 13, 2020 at 1:28 PM Dave Martin [off-list ref] wrote:

quoted

On Wed, May 13, 2020 at 11:09:14AM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v6:
- bring back comment about __reserved[]

v5:
- add padding to fault_addr_top_byte_context in order to ensure the correct
  size and preserve sp alignment

v4:
- expose only the tag bits in the context instead of the entire FAR_EL1
- remove mention of the new context from the sigcontext.__reserved[] note

v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 22 ++++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 22 +++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 77 insertions(+), 35 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index eab4323609b9..c6e9592a9dea 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst

@@ -53,12 +53,17 @@ visibility.
 Preserving tags
 ---------------

-Non-zero tags are not preserved when delivering signals. This means that
-signal handlers in applications making use of tags cannot rely on the
-tag information for user virtual addresses being maintained for fields
-inside siginfo_t. One exception to this rule is for signals raised in
-response to watchpoint debug exceptions, where the tag information will
-be preserved.
+Non-zero tags are not preserved in the fault address fields
+siginfo.si_addr or sigcontext.fault_address when delivering
+signals. This means that signal handlers in applications making use
+of tags cannot rely on the tag information for user virtual addresses
+being maintained in these fields. One exception to this rule is for
+signals raised in response to watchpoint debug exceptions, where the
+tag information will be preserved.
+
+The fault address tag is preserved in the fault_addr_top_byte field of
+the signal frame record fault_addr_top_byte_context, which is present
+for signals raised in response to data aborts and instruction aborts.

 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }

 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 240fe5e5b720..63185be29ff9 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -144,7 +144,7 @@ struct thread_struct {
      void                    *sve_state;     /* SVE registers, if any */
      unsigned int            sve_vl;         /* SVE vector length */
      unsigned int            sve_vl_onexec;  /* SVE vl after next exec */
-     unsigned long           fault_address;  /* fault info */
+     unsigned long           fault_address;  /* FAR_EL1 value */
      unsigned long           fault_code;     /* ESR_EL1 value */
      struct debug_info       debug;          /* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..2a3fe3de899d 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *   0x210           fpsimd_context
  *    0x10           esr_context
+ *    0x10           fault_addr_top_byte_context
  *   0x8a0           sve_context (vl <= 64) (optional)
  *    0x20           extra_context (optional)
  *    0x10           terminator (null _aarch64_ctx)
  *
- *   0x510           (reserved for future allocation)
+ *   0x500           (reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,26 @@ struct esr_context {
      __u64 esr;
 };

+/* Top byte of fault address (normally not exposed via si_addr) */
+#define FAULT_ADDR_TOP_BYTE_MAGIC    0x46544201
+
+struct fault_addr_top_byte_context {
+     struct _aarch64_ctx head;
+     __u8 fault_addr_top_byte;
+     __u8 __reserved[7];
+};
+

Nit: the name here is a bit cumbersome (obviously bikeshedding...)


For the rest, some of my comments may be bogus -- I haven't dug into
this stuff for a little while!


Anyway:

Do we really get the whole top byte of the address in the FAR?  If so,
fine, but I'm having trouble finding a clear statement in the
architecture one way or the other.  (I didn't attempt to dive into the
pseudocode.)

I rely on this statement in the ARM:

https://developer.arm.com/docs/ddi0595/b/aarch64-system-registers/far_el1
"For a Data Abort or Watchpoint exception, if address tagging is
enabled for the address accessed by the data access that caused the
exception, then this field includes the tag."

Yes, I think that covers it.  I hadn't found a clear definition of
"tag", but I think the TBI mechanism makes it "reasonably obvious" the
non-address (i.e., tag) bits are [63:56].

And note that address tagging here essentially means TBI (which is
always enabled on Linux), and not memory tagging.

quoted

Also, since we're burning 16 bytes here, I'd prefer if we make this
extensible.  At present the __reserved[7] is unusable because
userspace has no way to know whether it's valid or not.

Options include an additional flag byte (0 for now), or just making
the whole thing a __u64.  In that case we can leave the top byte bits
in their original positions if we want, but it would be a good idea to
include a flag to say that field is valid at all.  (See comments below
on Synchronous external abort.)

So, say, foo_context->fault_info = (esr & (~0ULL << 56)) | TOP_BYTE_VALID.
(with #defines for the bits/fields as appropriate).

The flag bits seem like a good idea. Thinking ahead to the MTE sync
tag fault (which might not provide us with bits 60-63), we may
consider having separate bits to indicate "bits 56-59 valid" and "bits
60-63 valid", set both bits for regular data aborts and only the
former for sync tag faults, which would avoid the need to define a
separate context for these faults. And if a future architecture
revision provides us with bits 60-63 for tag faults, we could start
setting both flag bits even for tag faults.

Seems reasonable, but a "tag mask" field of some sort might be
preferable to hard-wiring, just in case a future update to MTE supports
more than 4 bits.

quoted

 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, fault_addr_top_byte_context and
+ * extra_context must be placed in sigcontext.__reserved[] if present.
+ * They cannot be placed in the extra space.  Any other record can be
+ * placed either in the extra space or in sigcontext.__reserved[],
+ * unless otherwise specified in this file.
  *
  * 2) There must not be more than one extra_context.
  *

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c839b5bf1904..045b4f518836 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
      unsigned long far = read_sysreg(far_el1);

      local_daif_inherit(regs);
-     far = untagged_addr(far);
      do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)

      user_exit_irqoff();
      local_daif_restore(DAIF_PROCCTX);
-     far = untagged_addr(far);
      do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..baa88dc02e5c 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {

      unsigned long fpsimd_offset;
      unsigned long esr_offset;
+     unsigned long ftb_offset;
      unsigned long sve_offset;
      unsigned long extra_offset;
      unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
                      break;

              case ESR_MAGIC:
+             case FAULT_ADDR_TOP_BYTE_MAGIC:
                      /* ignore */
                      break;

@@ -581,6 +583,12 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
                                   sizeof(struct esr_context));
              if (err)
                      return err;
+
+             err = sigframe_alloc(
+                     user, &user->ftb_offset,
+                     sizeof(struct fault_addr_top_byte_context));

Nit: inconsistent indentation?

(Mostly just because it makes the change look odd against the hunk
context, but not a big deal.)

With consistent indentation we violate 80 cols due to the extra long
struct name. The indentation is what clang-format is giving me.

I suspected that might be why.  Fair enough (though a shorter name would
be no bad thing, it's not worth changing that just for nicer indentation).

quoted

+             if (err)
+                     return err;
      }

      if (system_supports_sve()) {

@@ -621,7 +629,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
      __put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
      __put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);

-     __put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+     __put_user_error(untagged_addr(current->thread.fault_address),
+                      &sf->uc.uc_mcontext.fault_address, err);

      err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
@@ -641,6 +650,17 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
              __put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
      }

+     if (err == 0 && user->ftb_offset) {
+             struct fault_addr_top_byte_context __user *ftb_ctx =
+                     apply_user_offset(user, user->ftb_offset);
+
+             __put_user_error(FAULT_ADDR_TOP_BYTE_MAGIC,
+                              &ftb_ctx->head.magic, err);
+             __put_user_error(sizeof(*ftb_ctx), &ftb_ctx->head.size, err);
+             __put_user_error(current->thread.fault_address >> 56,
+                              &ftb_ctx->fault_addr_top_byte, err);
+     }
+

How do we handle the fact that the top byte of FAR is sometimes UNKNOWN?

For Synchronous external aborts in particular, those bits are documented
as UNKNOWN, but I don't see any special handling,  There may be other
cases I haven't spotted.

For preference we can omit this record entirely if we don't have any
information we can report, but certainly we shouldn't expose UNKNOWN
bits.

In this case we mask out the top byte in do_sea before passing the
address to arm64_notify_die (which clears fault_address and passes the
address argument on to arm64_force_sig_fault to be exposed via
si_addr). So the record would always contain a 0 byte. It seems
reasonable to omit the record in this case instead.

Ah, right.  Missed that.

The record is already omitted when fault_code == 0 IIUC, so perhaps
we're already doing the right thing for synchronous external aborts.

quoted hunk

quoted

[ Aside:

Also, what if we're not reporting a memory abort at all?  Does
thread.fault_address just contain junk from the last fault?  I see
nothing anywhere that cleans this up.  (This is historical and not
your fault, but it would be good to close this down while we're about
it.)


Hmmm, looking at the code I think we probably leak fault_address etc.
across execve() too, so it may even be stale junk from an old process
:/

Maybe I just confused myself.

End aside. ]

Yes, it's unclear whether we always manage to not expose a fault
address if we're not reporting a data or instruction abort. The code
would need to arrange for fault_code to be set to 0 in order to avoid
exposing previous fault_address values via future signals. I don't see
anywhere where we're resetting these fields after delivering a signal,
so it seems possible by calling arm64_force_sig_fault without first
setting fault_code (most callers do this, but the calls in
arch/arm64/kernel/debug-monitors.c and arch/arm64/kernel/ptrace.c seem
not to), or simply by calling force_sig_fault (which happens in many
places throughout the kernel).

Maybe something like this would do the trick? (Untested, and forgive
spaces instead of tabs, grumble grumble gmail):

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index baa88dc02e5c..5867f2fdbe64 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -648,6 +648,7 @@ static int setup_sigframe(struct

rt_sigframe_user_layout *user,
                __put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
                __put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
                __put_user_error(current->thread.fault_code,
&esr_ctx->esr, err);
+               current->thread.fault_code = 0;

Perhaps, but we'd need to be careful.  For example, can we run out of
user stack before this and deliver a SIGSEGV, but with the old
fault_code still set?  Then we'd emit the old fault code with the
new "can't deliver signal" signal, which doesn't make sense.

Stuff may also go wrong with signal prioritisation.

If a higher-priority signal (say SIGINT) comes in after a data abort
enters the kernel but before the resulting SIGSEGV is dequeued for
delivery, wouldn't we deliver SIGINT first, with the bogus fault code?
With your change we'd then have cleared the fault code by the time we
deliver the SIGSEGV it actually relates to, if I've understood right.

Today, I think we just attach that fault code to every signal that's
delivered until something overwrites or resets it, which means that
a signal that needs fault_code gets it, at the expense of attaching
it to a bunch of other random signals too.


Checking the signal number and si_code might help us to know what we
should be doing with fault_code.  We need to have sure userspace can't
trick us with a non kernel generated signal here.  It would also be
necessary to check how PTRACE_SETSIGINFO interacts with this.


Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-19 22:00:36

On Mon, May 18, 2020 at 2:53 AM Dave Martin [off-list ref] wrote:

On Thu, May 14, 2020 at 05:58:21PM -0700, Peter Collingbourne wrote:

quoted

On Wed, May 13, 2020 at 1:28 PM Dave Martin [off-list ref] wrote:

quoted

On Wed, May 13, 2020 at 11:09:14AM -0700, Peter Collingbourne wrote:

quoted

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v6:
- bring back comment about __reserved[]

v5:
- add padding to fault_addr_top_byte_context in order to ensure the correct
  size and preserve sp alignment

v4:
- expose only the tag bits in the context instead of the entire FAR_EL1
- remove mention of the new context from the sigcontext.__reserved[] note

v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 +++++----
 arch/arm64/include/asm/exception.h       |  2 +-
 arch/arm64/include/asm/processor.h       |  2 +-
 arch/arm64/include/uapi/asm/sigcontext.h | 22 ++++++++----
 arch/arm64/kernel/entry-common.c         |  2 --
 arch/arm64/kernel/signal.c               | 22 +++++++++++-
 arch/arm64/mm/fault.c                    | 45 ++++++++++++++----------
 7 files changed, 77 insertions(+), 35 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index eab4323609b9..c6e9592a9dea 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst

@@ -53,12 +53,17 @@ visibility.
 Preserving tags
 ---------------

-Non-zero tags are not preserved when delivering signals. This means that
-signal handlers in applications making use of tags cannot rely on the
-tag information for user virtual addresses being maintained for fields
-inside siginfo_t. One exception to this rule is for signals raised in
-response to watchpoint debug exceptions, where the tag information will
-be preserved.
+Non-zero tags are not preserved in the fault address fields
+siginfo.si_addr or sigcontext.fault_address when delivering
+signals. This means that signal handlers in applications making use
+of tags cannot rely on the tag information for user virtual addresses
+being maintained in these fields. One exception to this rule is for
+signals raised in response to watchpoint debug exceptions, where the
+tag information will be preserved.
+
+The fault address tag is preserved in the fault_addr_top_byte field of
+the signal frame record fault_addr_top_byte_context, which is present
+for signals raised in response to data aborts and instruction aborts.

 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }

 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 240fe5e5b720..63185be29ff9 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -144,7 +144,7 @@ struct thread_struct {
      void                    *sve_state;     /* SVE registers, if any */
      unsigned int            sve_vl;         /* SVE vector length */
      unsigned int            sve_vl_onexec;  /* SVE vl after next exec */
-     unsigned long           fault_address;  /* fault info */
+     unsigned long           fault_address;  /* FAR_EL1 value */
      unsigned long           fault_code;     /* ESR_EL1 value */
      struct debug_info       debug;          /* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..2a3fe3de899d 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *   0x210           fpsimd_context
  *    0x10           esr_context
+ *    0x10           fault_addr_top_byte_context
  *   0x8a0           sve_context (vl <= 64) (optional)
  *    0x20           extra_context (optional)
  *    0x10           terminator (null _aarch64_ctx)
  *
- *   0x510           (reserved for future allocation)
+ *   0x500           (reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,26 @@ struct esr_context {
      __u64 esr;
 };

+/* Top byte of fault address (normally not exposed via si_addr) */
+#define FAULT_ADDR_TOP_BYTE_MAGIC    0x46544201
+
+struct fault_addr_top_byte_context {
+     struct _aarch64_ctx head;
+     __u8 fault_addr_top_byte;
+     __u8 __reserved[7];
+};
+

Nit: the name here is a bit cumbersome (obviously bikeshedding...)


For the rest, some of my comments may be bogus -- I haven't dug into
this stuff for a little while!


Anyway:

Do we really get the whole top byte of the address in the FAR?  If so,
fine, but I'm having trouble finding a clear statement in the
architecture one way or the other.  (I didn't attempt to dive into the
pseudocode.)

I rely on this statement in the ARM:

https://developer.arm.com/docs/ddi0595/b/aarch64-system-registers/far_el1
"For a Data Abort or Watchpoint exception, if address tagging is
enabled for the address accessed by the data access that caused the
exception, then this field includes the tag."

Yes, I think that covers it.  I hadn't found a clear definition of
"tag", but I think the TBI mechanism makes it "reasonably obvious" the
non-address (i.e., tag) bits are [63:56].

quoted

And note that address tagging here essentially means TBI (which is
always enabled on Linux), and not memory tagging.

quoted

Also, since we're burning 16 bytes here, I'd prefer if we make this
extensible.  At present the __reserved[7] is unusable because
userspace has no way to know whether it's valid or not.

Options include an additional flag byte (0 for now), or just making
the whole thing a __u64.  In that case we can leave the top byte bits
in their original positions if we want, but it would be a good idea to
include a flag to say that field is valid at all.  (See comments below
on Synchronous external abort.)

So, say, foo_context->fault_info = (esr & (~0ULL << 56)) | TOP_BYTE_VALID.
(with #defines for the bits/fields as appropriate).

The flag bits seem like a good idea. Thinking ahead to the MTE sync
tag fault (which might not provide us with bits 60-63), we may
consider having separate bits to indicate "bits 56-59 valid" and "bits
60-63 valid", set both bits for regular data aborts and only the
former for sync tag faults, which would avoid the need to define a
separate context for these faults. And if a future architecture
revision provides us with bits 60-63 for tag faults, we could start
setting both flag bits even for tag faults.

Seems reasonable, but a "tag mask" field of some sort might be
preferable to hard-wiring, just in case a future update to MTE supports
more than 4 bits.

That's fine with me.

quoted

 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, fault_addr_top_byte_context and
+ * extra_context must be placed in sigcontext.__reserved[] if present.
+ * They cannot be placed in the extra space.  Any other record can be
+ * placed either in the extra space or in sigcontext.__reserved[],
+ * unless otherwise specified in this file.
  *
  * 2) There must not be more than one extra_context.
  *

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c839b5bf1904..045b4f518836 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
      unsigned long far = read_sysreg(far_el1);

      local_daif_inherit(regs);
-     far = untagged_addr(far);
      do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)

      user_exit_irqoff();
      local_daif_restore(DAIF_PROCCTX);
-     far = untagged_addr(far);
      do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..baa88dc02e5c 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {

      unsigned long fpsimd_offset;
      unsigned long esr_offset;
+     unsigned long ftb_offset;
      unsigned long sve_offset;
      unsigned long extra_offset;
      unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
                      break;

              case ESR_MAGIC:
+             case FAULT_ADDR_TOP_BYTE_MAGIC:
                      /* ignore */
                      break;

@@ -581,6 +583,12 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
                                   sizeof(struct esr_context));
              if (err)
                      return err;
+
+             err = sigframe_alloc(
+                     user, &user->ftb_offset,
+                     sizeof(struct fault_addr_top_byte_context));

Nit: inconsistent indentation?

(Mostly just because it makes the change look odd against the hunk
context, but not a big deal.)

With consistent indentation we violate 80 cols due to the extra long
struct name. The indentation is what clang-format is giving me.

I suspected that might be why.  Fair enough (though a shorter name would
be no bad thing, it's not worth changing that just for nicer indentation).

quoted

+             if (err)
+                     return err;
      }

      if (system_supports_sve()) {

@@ -621,7 +629,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
      __put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
      __put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);

-     __put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+     __put_user_error(untagged_addr(current->thread.fault_address),
+                      &sf->uc.uc_mcontext.fault_address, err);

      err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
@@ -641,6 +650,17 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
              __put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
      }

+     if (err == 0 && user->ftb_offset) {
+             struct fault_addr_top_byte_context __user *ftb_ctx =
+                     apply_user_offset(user, user->ftb_offset);
+
+             __put_user_error(FAULT_ADDR_TOP_BYTE_MAGIC,
+                              &ftb_ctx->head.magic, err);
+             __put_user_error(sizeof(*ftb_ctx), &ftb_ctx->head.size, err);
+             __put_user_error(current->thread.fault_address >> 56,
+                              &ftb_ctx->fault_addr_top_byte, err);
+     }
+

How do we handle the fact that the top byte of FAR is sometimes UNKNOWN?

For Synchronous external aborts in particular, those bits are documented
as UNKNOWN, but I don't see any special handling,  There may be other
cases I haven't spotted.

For preference we can omit this record entirely if we don't have any
information we can report, but certainly we shouldn't expose UNKNOWN
bits.

In this case we mask out the top byte in do_sea before passing the
address to arm64_notify_die (which clears fault_address and passes the
address argument on to arm64_force_sig_fault to be exposed via
si_addr). So the record would always contain a 0 byte. It seems
reasonable to omit the record in this case instead.

Ah, right.  Missed that.

The record is already omitted when fault_code == 0 IIUC, so perhaps
we're already doing the right thing for synchronous external aborts.

quoted

[ Aside:

Also, what if we're not reporting a memory abort at all?  Does
thread.fault_address just contain junk from the last fault?  I see
nothing anywhere that cleans this up.  (This is historical and not
your fault, but it would be good to close this down while we're about
it.)


Hmmm, looking at the code I think we probably leak fault_address etc.
across execve() too, so it may even be stale junk from an old process
:/

Maybe I just confused myself.

End aside. ]

Yes, it's unclear whether we always manage to not expose a fault
address if we're not reporting a data or instruction abort. The code
would need to arrange for fault_code to be set to 0 in order to avoid
exposing previous fault_address values via future signals. I don't see
anywhere where we're resetting these fields after delivering a signal,
so it seems possible by calling arm64_force_sig_fault without first
setting fault_code (most callers do this, but the calls in
arch/arm64/kernel/debug-monitors.c and arch/arm64/kernel/ptrace.c seem
not to), or simply by calling force_sig_fault (which happens in many
places throughout the kernel).

Maybe something like this would do the trick? (Untested, and forgive
spaces instead of tabs, grumble grumble gmail):

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index baa88dc02e5c..5867f2fdbe64 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -648,6 +648,7 @@ static int setup_sigframe(struct

rt_sigframe_user_layout *user,
                __put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
                __put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
                __put_user_error(current->thread.fault_code,
&esr_ctx->esr, err);
+               current->thread.fault_code = 0;

Perhaps, but we'd need to be careful.  For example, can we run out of
user stack before this and deliver a SIGSEGV, but with the old
fault_code still set?  Then we'd emit the old fault code with the
new "can't deliver signal" signal, which doesn't make sense.

Stuff may also go wrong with signal prioritisation.

If a higher-priority signal (say SIGINT) comes in after a data abort
enters the kernel but before the resulting SIGSEGV is dequeued for
delivery, wouldn't we deliver SIGINT first, with the bogus fault code?
With your change we'd then have cleared the fault code by the time we
deliver the SIGSEGV it actually relates to, if I've understood right.

Today, I think we just attach that fault code to every signal that's
delivered until something overwrites or resets it, which means that
a signal that needs fault_code gets it, at the expense of attaching
it to a bunch of other random signals too.


Checking the signal number and si_code might help us to know what we
should be doing with fault_code.  We need to have sure userspace can't
trick us with a non kernel generated signal here.  It would also be
necessary to check how PTRACE_SETSIGINFO interacts with this.

With these possible interactions in mind I think we should store the
fault code and fault address in kernel_siginfo instead of
thread_struct (and clear these fields when we receive a siginfo from
userspace, i.e. in copy_siginfo_from_user which is used by
ptrace(PTRACE_SETSIGINFO) among other places). That way, the
information is clearly associated with the signal itself and not the
thread, so we don't need to worry about our signal being delivered out
of order.

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Will Deacon <will@kernel.org>
Date: 2020-05-20 08:55:13

On Tue, May 19, 2020 at 03:00:12PM -0700, Peter Collingbourne wrote:

On Mon, May 18, 2020 at 2:53 AM Dave Martin [off-list ref] wrote:

quoted

On Thu, May 14, 2020 at 05:58:21PM -0700, Peter Collingbourne wrote:

quoted

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index baa88dc02e5c..5867f2fdbe64 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -648,6 +648,7 @@ static int setup_sigframe(struct

rt_sigframe_user_layout *user,
                __put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
                __put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
                __put_user_error(current->thread.fault_code,
&esr_ctx->esr, err);
+               current->thread.fault_code = 0;

Perhaps, but we'd need to be careful.  For example, can we run out of
user stack before this and deliver a SIGSEGV, but with the old
fault_code still set?  Then we'd emit the old fault code with the
new "can't deliver signal" signal, which doesn't make sense.

Stuff may also go wrong with signal prioritisation.

If a higher-priority signal (say SIGINT) comes in after a data abort
enters the kernel but before the resulting SIGSEGV is dequeued for
delivery, wouldn't we deliver SIGINT first, with the bogus fault code?
With your change we'd then have cleared the fault code by the time we
deliver the SIGSEGV it actually relates to, if I've understood right.

Today, I think we just attach that fault code to every signal that's
delivered until something overwrites or resets it, which means that
a signal that needs fault_code gets it, at the expense of attaching
it to a bunch of other random signals too.


Checking the signal number and si_code might help us to know what we
should be doing with fault_code.  We need to have sure userspace can't
trick us with a non kernel generated signal here.  It would also be
necessary to check how PTRACE_SETSIGINFO interacts with this.

With these possible interactions in mind I think we should store the
fault code and fault address in kernel_siginfo instead of
thread_struct (and clear these fields when we receive a siginfo from
userspace, i.e. in copy_siginfo_from_user which is used by
ptrace(PTRACE_SETSIGINFO) among other places). That way, the
information is clearly associated with the signal itself and not the
thread, so we don't need to worry about our signal being delivered out
of order.

Hmm, I can't see a way to do that that isn't horribly invasive in the core
signal code. Can you?

But generally, I agree: the per-thread handling of fault_address and
fault_code appears to be quite broken in the face of signal prioritisation
and signals that don't correspond directly to hardware trap. It would be
nice to have some tests for this...

If we want to pile on more bodges, perhaps we could stash the signal number
to which the fault_{address,code} relate, and then check that at delivery
and clear on a match. I hate it.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Dave Martin <Dave.Martin@arm.com>
Date: 2020-05-20 09:26:07

On Wed, May 20, 2020 at 09:55:03AM +0100, Will Deacon wrote:

On Tue, May 19, 2020 at 03:00:12PM -0700, Peter Collingbourne wrote:

quoted

On Mon, May 18, 2020 at 2:53 AM Dave Martin [off-list ref] wrote:

quoted

On Thu, May 14, 2020 at 05:58:21PM -0700, Peter Collingbourne wrote:

quoted

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index baa88dc02e5c..5867f2fdbe64 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -648,6 +648,7 @@ static int setup_sigframe(struct

rt_sigframe_user_layout *user,
                __put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
                __put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
                __put_user_error(current->thread.fault_code,
&esr_ctx->esr, err);
+               current->thread.fault_code = 0;

Perhaps, but we'd need to be careful.  For example, can we run out of
user stack before this and deliver a SIGSEGV, but with the old
fault_code still set?  Then we'd emit the old fault code with the
new "can't deliver signal" signal, which doesn't make sense.

Stuff may also go wrong with signal prioritisation.

If a higher-priority signal (say SIGINT) comes in after a data abort
enters the kernel but before the resulting SIGSEGV is dequeued for
delivery, wouldn't we deliver SIGINT first, with the bogus fault code?
With your change we'd then have cleared the fault code by the time we
deliver the SIGSEGV it actually relates to, if I've understood right.

Today, I think we just attach that fault code to every signal that's
delivered until something overwrites or resets it, which means that
a signal that needs fault_code gets it, at the expense of attaching
it to a bunch of other random signals too.


Checking the signal number and si_code might help us to know what we
should be doing with fault_code.  We need to have sure userspace can't
trick us with a non kernel generated signal here.  It would also be
necessary to check how PTRACE_SETSIGINFO interacts with this.

With these possible interactions in mind I think we should store the
fault code and fault address in kernel_siginfo instead of
thread_struct (and clear these fields when we receive a siginfo from
userspace, i.e. in copy_siginfo_from_user which is used by
ptrace(PTRACE_SETSIGINFO) among other places). That way, the
information is clearly associated with the signal itself and not the
thread, so we don't need to worry about our signal being delivered out
of order.

Hmm, I can't see a way to do that that isn't horribly invasive in the core
signal code. Can you?

But generally, I agree: the per-thread handling of fault_address and
fault_code appears to be quite broken in the face of signal prioritisation
and signals that don't correspond directly to hardware trap. It would be
nice to have some tests for this...

If we want to pile on more bodges, perhaps we could stash the signal number
to which the fault_{address,code} relate, and then check that at delivery
and clear on a match. I hate it.

I agree with Daniel's suggestion in principle, but I was also concerned
about whether it would be too invasive elsewhere.

Question though: does the core code take special care to make sure that
a force_sig cannot be outprioritised by a regular signal?  If so,
perhaps we get away with it.  I ask this, because the same same issue
may be hitting other arches otherwise.

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-21 02:29:12

On Wed, May 20, 2020 at 2:26 AM Dave Martin [off-list ref] wrote:

On Wed, May 20, 2020 at 09:55:03AM +0100, Will Deacon wrote:

quoted

On Tue, May 19, 2020 at 03:00:12PM -0700, Peter Collingbourne wrote:

quoted

On Mon, May 18, 2020 at 2:53 AM Dave Martin [off-list ref] wrote:

quoted

On Thu, May 14, 2020 at 05:58:21PM -0700, Peter Collingbourne wrote:

quoted

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index baa88dc02e5c..5867f2fdbe64 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -648,6 +648,7 @@ static int setup_sigframe(struct

rt_sigframe_user_layout *user,
                __put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
                __put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
                __put_user_error(current->thread.fault_code,
&esr_ctx->esr, err);
+               current->thread.fault_code = 0;

Perhaps, but we'd need to be careful.  For example, can we run out of
user stack before this and deliver a SIGSEGV, but with the old
fault_code still set?  Then we'd emit the old fault code with the
new "can't deliver signal" signal, which doesn't make sense.

Stuff may also go wrong with signal prioritisation.

If a higher-priority signal (say SIGINT) comes in after a data abort
enters the kernel but before the resulting SIGSEGV is dequeued for
delivery, wouldn't we deliver SIGINT first, with the bogus fault code?
With your change we'd then have cleared the fault code by the time we
deliver the SIGSEGV it actually relates to, if I've understood right.

Today, I think we just attach that fault code to every signal that's
delivered until something overwrites or resets it, which means that
a signal that needs fault_code gets it, at the expense of attaching
it to a bunch of other random signals too.


Checking the signal number and si_code might help us to know what we
should be doing with fault_code.  We need to have sure userspace can't
trick us with a non kernel generated signal here.  It would also be
necessary to check how PTRACE_SETSIGINFO interacts with this.

With these possible interactions in mind I think we should store the
fault code and fault address in kernel_siginfo instead of
thread_struct (and clear these fields when we receive a siginfo from
userspace, i.e. in copy_siginfo_from_user which is used by
ptrace(PTRACE_SETSIGINFO) among other places). That way, the
information is clearly associated with the signal itself and not the
thread, so we don't need to worry about our signal being delivered out
of order.

Hmm, I can't see a way to do that that isn't horribly invasive in the core
signal code. Can you?

I think I've come up with a way that doesn't seem to be too invasive.
See patch #1 of the series that I'm about to send out.

quoted

But generally, I agree: the per-thread handling of fault_address and
fault_code appears to be quite broken in the face of signal prioritisation
and signals that don't correspond directly to hardware trap. It would be
nice to have some tests for this...

If we want to pile on more bodges, perhaps we could stash the signal number
to which the fault_{address,code} relate, and then check that at delivery
and clear on a match. I hate it.

I agree with Daniel's suggestion in principle, but I was also concerned
about whether it would be too invasive elsewhere.

Question though: does the core code take special care to make sure that
a force_sig cannot be outprioritised by a regular signal?  If so,
perhaps we get away with it.  I ask this, because the same same issue
may be hitting other arches otherwise.

Not as far as I can tell. There does appear to be prioritisation for
synchronous signals [1] but as far as I can tell nothing to
distinguish one of these signals from one with the same signal number
sent from userspace (e.g. via kill(2)).

Peter

[1] https://github.com/torvalds/linux/blob/b85051e755b0e9d6dd8f17ef1da083851b83287d/kernel/signal.c#L222

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6 0/3] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-21 02:30:09

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Peter Collingbourne (3):
  signal: Allow architectures to store arch-specific data in
    kernel_siginfo
  arm64: Move fault address and fault code into kernel_siginfo
  arm64: Expose FAR_EL1 tag bits in sigcontext

 Documentation/arm64/tagged-pointers.rst  |  17 ++--
 arch/arm64/include/asm/exception.h       |   2 +-
 arch/arm64/include/asm/processor.h       |   2 -
 arch/arm64/include/asm/signal.h          |  19 ++++
 arch/arm64/include/asm/traps.h           |   8 +-
 arch/arm64/include/uapi/asm/sigcontext.h |  24 +++--
 arch/arm64/kernel/debug-monitors.c       |   4 +-
 arch/arm64/kernel/entry-common.c         |   2 -
 arch/arm64/kernel/probes/uprobes.c       |  18 ++--
 arch/arm64/kernel/ptrace.c               |   2 +-
 arch/arm64/kernel/signal.c               |  42 ++++++--
 arch/arm64/kernel/signal32.c             |  15 ++-
 arch/arm64/kernel/sys_compat.c           |   9 +-
 arch/arm64/kernel/traps.c                | 121 +++++++++++++++++++----
 arch/arm64/mm/fault.c                    | 111 ++++++---------------
 include/linux/signal.h                   |   2 -
 include/linux/signal_types.h             |   3 +
 kernel/signal.c                          |  17 +++-
 18 files changed, 259 insertions(+), 159 deletions(-)
 create mode 100644 arch/arm64/include/asm/signal.h

-- 
2.26.2.761.g0e0b3e54be-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6 1/3] signal: Allow architectures to store arch-specific data in kernel_siginfo

From: Peter Collingbourne <hidden>
Date: 2020-05-21 02:30:38

In some cases we would like to store architecture-specific data in
the kernel's siginfo but not in the userspace one. This is generally
data is conceptually part of siginfo but is not stored there for one
reason or another. For example, on arm64, the arch-specific fault
code register ESR_EL1 is exposed to signal handlers, but since it is
associated with many different types of signals it does not fit well
into siginfo and appears in sigcontext instead.

Currently this data is stored in thread_struct, which is error-prone
because the data is associated with the signal itself and not the task,
and as a result it could get out of sync with the signal that is
currently being delivered.

To help avoid these types of errors, introduce a way for architectures
to store architecture-specific data in the kernel_siginfo. This part
of the kernel_siginfo is not exposed to userspace so the architecture
can use it in any way that it likes without ABI concerns. A follow-up
change will start using this mechanism on arm64 to store the fault
code and fault address.

Signed-off-by: Peter Collingbourne <redacted>
---
 include/linux/signal.h       |  2 --
 include/linux/signal_types.h |  3 +++
 kernel/signal.c              | 17 +++++++++++++----
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/linux/signal.h b/include/linux/signal.h
index 05bacd2ab135..34bf18932e9b 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h

@@ -22,8 +22,6 @@ static inline void clear_siginfo(kernel_siginfo_t *info)
 	memset(info, 0, sizeof(*info));
 }
 
-#define SI_EXPANSION_SIZE (sizeof(struct siginfo) - sizeof(struct kernel_siginfo))
-
 int copy_siginfo_to_user(siginfo_t __user *to, const kernel_siginfo_t *from);
 int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from);

diff --git a/include/linux/signal_types.h b/include/linux/signal_types.h
index f8a90ae9c6ec..a8635eeb890b 100644
--- a/include/linux/signal_types.h
+++ b/include/linux/signal_types.h

@@ -11,6 +11,9 @@
 
 typedef struct kernel_siginfo {
 	__SIGINFO;
+#ifdef __ARCH_HAS_PRIVATE_SIGINFO
+	struct arch_private_siginfo arch;
+#endif
 } kernel_siginfo_t;
 
 /*

diff --git a/kernel/signal.c b/kernel/signal.c
index 284fc1600063..a33df2280ed5 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c

@@ -3180,15 +3180,21 @@ enum siginfo_layout siginfo_layout(unsigned sig, int si_code)
 	return layout;
 }
 
+struct shared_siginfo {
+	__SIGINFO;
+};
+
+#define SI_EXPANSION_SIZE (sizeof(struct siginfo) - sizeof(struct shared_siginfo))
+
 static inline char __user *si_expansion(const siginfo_t __user *info)
 {
-	return ((char __user *)info) + sizeof(struct kernel_siginfo);
+	return ((char __user *)info) + sizeof(struct shared_siginfo);
 }
 
 int copy_siginfo_to_user(siginfo_t __user *to, const kernel_siginfo_t *from)
 {
 	char __user *expansion = si_expansion(to);
-	if (copy_to_user(to, from , sizeof(struct kernel_siginfo)))
+	if (copy_to_user(to, from , sizeof(struct shared_siginfo)))
 		return -EFAULT;
 	if (clear_user(expansion, SI_EXPANSION_SIZE))
 		return -EFAULT;

@@ -3198,6 +3204,9 @@ int copy_siginfo_to_user(siginfo_t __user *to, const kernel_siginfo_t *from)
 static int post_copy_siginfo_from_user(kernel_siginfo_t *info,
 				       const siginfo_t __user *from)
 {
+#ifdef __ARCH_HAS_PRIVATE_SIGINFO
+	memset(&info->arch, 0, sizeof(info->arch));
+#endif
 	if (unlikely(!known_siginfo_layout(info->si_signo, info->si_code))) {
 		char __user *expansion = si_expansion(from);
 		char buf[SI_EXPANSION_SIZE];

@@ -3221,7 +3230,7 @@ static int post_copy_siginfo_from_user(kernel_siginfo_t *info,
 static int __copy_siginfo_from_user(int signo, kernel_siginfo_t *to,
 				    const siginfo_t __user *from)
 {
-	if (copy_from_user(to, from, sizeof(struct kernel_siginfo)))
+	if (copy_from_user(to, from, sizeof(struct shared_siginfo)))
 		return -EFAULT;
 	to->si_signo = signo;
 	return post_copy_siginfo_from_user(to, from);

@@ -3229,7 +3238,7 @@ static int __copy_siginfo_from_user(int signo, kernel_siginfo_t *to,
 
 int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from)
 {
-	if (copy_from_user(to, from, sizeof(struct kernel_siginfo)))
+	if (copy_from_user(to, from, sizeof(struct shared_siginfo)))
 		return -EFAULT;
 	return post_copy_siginfo_from_user(to, from);
 }

-- 
2.26.2.761.g0e0b3e54be-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6 2/3] arm64: Move fault address and fault code into kernel_siginfo

From: Peter Collingbourne <hidden>
Date: 2020-05-21 02:30:52

Previously this data was being stored in thread_struct, which is
error-prone because the data is associated with the signal itself
and not the task, and as a result it could get out of sync with the
signal that is currently being delivered. To avoid this problem,
move the fields to kernel_siginfo using the newly-introduced generic
support for doing so.

The new fields store the raw FAR_EL1 and ESR_EL1 values instead of
the cooked ones as we were doing before. For FAR_EL1 this will be
necessary in order to expose the high bits of FAR_EL1 in sigcontext
in an upcoming change. Do the same for ESR_EL1 for consistency and
to make the code less error-prone.

While here, fix an apparent compat bug where, when delivering a
SIGILL signal in response to an invalid syscall, the syscall number
was being interpreted as an ESR_EL1 value and translated into an
FSR before being stored in sigcontext.error_code, rather than being
stored in error_code directly, as was the intention (and the behaviour
of the code in arch/arm). This is achieved by moving the error_code
translation early so that the syscall number can avoid it.

Signed-off-by: Peter Collingbourne <redacted>
---
 arch/arm64/include/asm/exception.h |   2 +-
 arch/arm64/include/asm/processor.h |   2 -
 arch/arm64/include/asm/signal.h    |  16 ++++
 arch/arm64/include/asm/traps.h     |   8 +-
 arch/arm64/kernel/debug-monitors.c |   4 +-
 arch/arm64/kernel/entry-common.c   |   2 -
 arch/arm64/kernel/probes/uprobes.c |  18 ++---
 arch/arm64/kernel/ptrace.c         |   2 +-
 arch/arm64/kernel/signal.c         |  17 +++--
 arch/arm64/kernel/signal32.c       |  15 ++--
 arch/arm64/kernel/sys_compat.c     |   9 ++-
 arch/arm64/kernel/traps.c          | 117 ++++++++++++++++++++++++-----
 arch/arm64/mm/fault.c              | 111 +++++++--------------------
 13 files changed, 183 insertions(+), 140 deletions(-)
 create mode 100644 arch/arm64/include/asm/signal.h

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 7a6e81ca23a8..90e772d9b2cd 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h

@@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr)
 }
 
 asmlinkage void enter_from_user_mode(void);
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs);
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs);
 void do_undefinstr(struct pt_regs *regs);
 asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 240fe5e5b720..b326bfbcea62 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -144,8 +144,6 @@ struct thread_struct {
 	void			*sve_state;	/* SVE registers, if any */
 	unsigned int		sve_vl;		/* SVE vector length */
 	unsigned int		sve_vl_onexec;	/* SVE vl after next exec */
-	unsigned long		fault_address;	/* fault info */
-	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH
 	struct ptrauth_keys_user	keys_user;

diff --git a/arch/arm64/include/asm/signal.h b/arch/arm64/include/asm/signal.h
new file mode 100644
index 000000000000..f5c001b0a125
--- /dev/null
+++ b/arch/arm64/include/asm/signal.h

@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARM64_SIGNAL_H
+#define _ASM_ARM64_SIGNAL_H
+
+#include <uapi/asm/signal.h>
+
+#define __ARCH_HAS_PRIVATE_SIGINFO
+struct arch_private_siginfo {
+	/* FAR_EL1 value */
+	unsigned long fault_address;
+
+	/* Sanitized ESR_EL1 value, or FSR/syscall number in compat mode */
+	unsigned long error_code;
+};
+
+#endif

diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index cee5928e1b7d..5ed5be5347e6 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h

@@ -26,8 +26,12 @@ void register_undef_hook(struct undef_hook *hook);
 void unregister_undef_hook(struct undef_hook *hook);
 void force_signal_inject(int signal, int code, unsigned long address);
 void arm64_notify_segfault(unsigned long addr);
-void arm64_force_sig_fault(int signo, int code, void __user *addr, const char *str);
-void arm64_force_sig_mceerr(int code, void __user *addr, short lsb, const char *str);
+void arm64_force_sig_fault(int signo, int code, void __user *addr,
+			   unsigned long far, unsigned long esr,
+			   const char *str);
+void arm64_force_sig_mceerr(int code, void __user *addr, short lsb,
+			    unsigned long far, unsigned long esr,
+			    const char *str);
 void arm64_force_sig_ptrace_errno_trap(int errno, void __user *addr, const char *str);
 
 /*

diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 48222a4760c2..498e6393b2ca 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c

@@ -232,8 +232,8 @@ static void send_user_sigtrap(int si_code)
 		local_irq_enable();
 
 	arm64_force_sig_fault(SIGTRAP, si_code,
-			     (void __user *)instruction_pointer(regs),
-			     "User debug trap");
+			      (void __user *)instruction_pointer(regs), 0, 0,
+			      "User debug trap");
 }
 
 static int single_step_handler(unsigned long unused, unsigned int esr,

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c839b5bf1904..045b4f518836 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c

@@ -22,7 +22,6 @@ static void notrace el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	local_daif_inherit(regs);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el1_abort);

@@ -104,7 +103,6 @@ static void notrace el0_da(struct pt_regs *regs, unsigned long esr)
 
 	user_exit_irqoff();
 	local_daif_restore(DAIF_PROCCTX);
-	far = untagged_addr(far);
 	do_mem_abort(far, esr, regs);
 }
 NOKPROBE_SYMBOL(el0_da);

diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
index a412d8edbcd2..5bbcd2e813f0 100644
--- a/arch/arm64/kernel/probes/uprobes.c
+++ b/arch/arm64/kernel/probes/uprobes.c

@@ -9,8 +9,6 @@
 
 #include "decode-insn.h"
 
-#define UPROBE_INV_FAULT_CODE	UINT_MAX
-
 void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 		void *src, unsigned long len)
 {

@@ -63,9 +61,6 @@ int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
 {
 	struct uprobe_task *utask = current->utask;
 
-	/* Initialize with an invalid fault code to detect if ol insn trapped */
-	current->thread.fault_code = UPROBE_INV_FAULT_CODE;
-
 	/* Instruction points to execute ol */
 	instruction_pointer_set(regs, utask->xol_vaddr);

@@ -78,7 +73,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
 {
 	struct uprobe_task *utask = current->utask;
 
-	WARN_ON_ONCE(current->thread.fault_code != UPROBE_INV_FAULT_CODE);
+	WARN_ON_ONCE(arch_uprobe_xol_was_trapped(current));
 
 	/* Instruction points to execute next to breakpoint address */
 	instruction_pointer_set(regs, utask->vaddr + 4);

@@ -89,13 +84,16 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
 }
 bool arch_uprobe_xol_was_trapped(struct task_struct *t)
 {
+	struct sigqueue *q;
+
 	/*
 	 * Between arch_uprobe_pre_xol and arch_uprobe_post_xol, if an xol
-	 * insn itself is trapped, then detect the case with the help of
-	 * invalid fault code which is being set in arch_uprobe_pre_xol
+	 * insn itself is trapped, then detect the case by checking for
+	 * non-zero esr_el1 in the task's pending signals.
 	 */
-	if (t->thread.fault_code != UPROBE_INV_FAULT_CODE)
-		return true;
+	list_for_each_entry (q, &t->pending.list, list)
+		if (q->info.arch.error_code)
+			return true;
 
 	return false;
 }

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index b3d3005d9515..51bb8bcaf24b 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c

@@ -198,7 +198,7 @@ static void ptrace_hbptriggered(struct perf_event *bp,
 	}
 #endif
 	arm64_force_sig_fault(SIGTRAP, TRAP_HWBKPT,
-			      (void __user *)(bkpt->trigger),
+			      (void __user *)(bkpt->trigger), 0, 0,
 			      desc);
 }

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 339882db5a91..10d7e9832a89 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -566,6 +566,7 @@ SYSCALL_DEFINE0(rt_sigreturn)
  *	of the task.
  */
 static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
+				 struct kernel_siginfo *info,
 				 bool add_all)
 {
 	int err;

@@ -576,7 +577,7 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 		return err;
 
 	/* fault information, if valid */
-	if (add_all || current->thread.fault_code) {
+	if (add_all || info->arch.error_code) {
 		err = sigframe_alloc(user, &user->esr_offset,
 				     sizeof(struct esr_context));
 		if (err)

@@ -605,7 +606,8 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 }
 
 static int setup_sigframe(struct rt_sigframe_user_layout *user,
-			  struct pt_regs *regs, sigset_t *set)
+			  struct pt_regs *regs, sigset_t *set,
+			  struct kernel_siginfo *info)
 {
 	int i, err = 0;
 	struct rt_sigframe __user *sf = user->sigframe;

@@ -621,7 +623,8 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	__put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
 	__put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);
 
-	__put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+	__put_user_error(untagged_addr(info->arch.fault_address),
+			 &sf->uc.uc_mcontext.fault_address, err);
 
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));

@@ -638,7 +641,7 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 
 		__put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
 		__put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
-		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
+		__put_user_error(info->arch.error_code, &esr_ctx->esr, err);
 	}
 
 	/* Scalable Vector Extension state, if present */

@@ -701,7 +704,7 @@ static int get_sigframe(struct rt_sigframe_user_layout *user,
 	int err;
 
 	init_user_layout(user);
-	err = setup_sigframe_layout(user, false);
+	err = setup_sigframe_layout(user, &ksig->info, false);
 	if (err)
 		return err;

@@ -758,7 +761,7 @@ static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
 	__put_user_error(NULL, &frame->uc.uc_link, err);
 
 	err |= __save_altstack(&frame->uc.uc_stack, regs->sp);
-	err |= setup_sigframe(&user, regs, set);
+	err |= setup_sigframe(&user, regs, set, &ksig->info);
 	if (err == 0) {
 		setup_return(regs, &ksig->ka, &user, usig);
 		if (ksig->ka.sa.sa_flags & SA_SIGINFO) {

@@ -958,7 +961,7 @@ void __init minsigstksz_setup(void)
 	 * If this fails, SIGFRAME_MAXSZ needs to be enlarged.  It won't
 	 * be big enough, but it's our best guess:
 	 */
-	if (WARN_ON(setup_sigframe_layout(&user, true)))
+	if (WARN_ON(setup_sigframe_layout(&user, 0, true)))
 		return;
 
 	signal_minsigstksz = sigframe_size(&user) +

diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c
index 82feca6f7052..b302689b6651 100644
--- a/arch/arm64/kernel/signal32.c
+++ b/arch/arm64/kernel/signal32.c

@@ -37,8 +37,6 @@ struct compat_vfp_sigframe {
 #define VFP_MAGIC		0x56465001
 #define VFP_STORAGE_SIZE	sizeof(struct compat_vfp_sigframe)
 
-#define FSR_WRITE_SHIFT		(11)
-
 struct compat_aux_sigframe {
 	struct compat_vfp_sigframe	vfp;

@@ -384,7 +382,8 @@ static void compat_setup_return(struct pt_regs *regs, struct k_sigaction *ka,
 }
 
 static int compat_setup_sigframe(struct compat_sigframe __user *sf,
-				 struct pt_regs *regs, sigset_t *set)
+				 struct pt_regs *regs, sigset_t *set,
+				 struct kernel_siginfo *info)
 {
 	struct compat_aux_sigframe __user *aux;
 	unsigned long psr = pstate_to_compat_psr(regs->pstate);

@@ -409,10 +408,8 @@ static int compat_setup_sigframe(struct compat_sigframe __user *sf,
 	__put_user_error(psr, &sf->uc.uc_mcontext.arm_cpsr, err);
 
 	__put_user_error((compat_ulong_t)0, &sf->uc.uc_mcontext.trap_no, err);
-	/* set the compat FSR WnR */
-	__put_user_error(!!(current->thread.fault_code & ESR_ELx_WNR) <<
-			 FSR_WRITE_SHIFT, &sf->uc.uc_mcontext.error_code, err);
-	__put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+	__put_user_error(info->arch.error_code, &sf->uc.uc_mcontext.error_code, err);
+	__put_user_error(info->arch.fault_address, &sf->uc.uc_mcontext.fault_address, err);
 	__put_user_error(set->sig[0], &sf->uc.uc_mcontext.oldmask, err);
 
 	err |= put_sigset_t(&sf->uc.uc_sigmask, set);

@@ -447,7 +444,7 @@ int compat_setup_rt_frame(int usig, struct ksignal *ksig,
 
 	err |= __compat_save_altstack(&frame->sig.uc.uc_stack, regs->compat_sp);
 
-	err |= compat_setup_sigframe(&frame->sig, regs, set);
+	err |= compat_setup_sigframe(&frame->sig, regs, set, &ksig->info);
 
 	if (err == 0) {
 		compat_setup_return(regs, &ksig->ka, frame->sig.retcode, frame, usig);

@@ -471,7 +468,7 @@ int compat_setup_frame(int usig, struct ksignal *ksig, sigset_t *set,
 
 	__put_user_error(0x5ac3c35a, &frame->uc.uc_flags, err);
 
-	err |= compat_setup_sigframe(frame, regs, set);
+	err |= compat_setup_sigframe(frame, regs, set, &ksig->info);
 	if (err == 0)
 		compat_setup_return(regs, &ksig->ka, frame->retcode, frame, usig);

diff --git a/arch/arm64/kernel/sys_compat.c b/arch/arm64/kernel/sys_compat.c
index 3c18c2454089..d7a0b93a8d9f 100644
--- a/arch/arm64/kernel/sys_compat.c
+++ b/arch/arm64/kernel/sys_compat.c

@@ -69,6 +69,7 @@ do_compat_cache_op(unsigned long start, unsigned long end, int flags)
 long compat_arm_syscall(struct pt_regs *regs, int scno)
 {
 	void __user *addr;
+	struct kernel_siginfo info;
 
 	switch (scno) {
 	/*

@@ -114,7 +115,11 @@ long compat_arm_syscall(struct pt_regs *regs, int scno)
 	addr  = (void __user *)instruction_pointer(regs) -
 		(compat_thumb_mode(regs) ? 2 : 4);
 
-	arm64_notify_die("Oops - bad compat syscall(2)", regs,
-			 SIGILL, ILL_ILLTRP, addr, scno);
+	clear_siginfo(&info);
+	info.si_signo = SIGILL;
+	info.si_code = ILL_ILLTRP;
+	info.si_addr = addr;
+	info.arch.error_code = scno;
+	force_sig_info(&info);
 	return 0;
 }

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index cf402be5c573..4545fe067ea9 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c

@@ -44,6 +44,8 @@
 #include <asm/system_misc.h>
 #include <asm/sysreg.h>
 
+#define FSR_WRITE_SHIFT		(11)
+
 static const char *handler[]= {
 	"Synchronous Abort",
 	"IRQ",

@@ -209,12 +211,11 @@ void die(const char *str, struct pt_regs *regs, int err)
 		do_exit(SIGSEGV);
 }
 
-static void arm64_show_signal(int signo, const char *str)
+static void arm64_show_signal(int signo, unsigned long esr, const char *str)
 {
 	static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
 				      DEFAULT_RATELIMIT_BURST);
 	struct task_struct *tsk = current;
-	unsigned int esr = tsk->thread.fault_code;
 	struct pt_regs *regs = task_pt_regs(tsk);
 
 	/* Leave if the signal won't be shown */

@@ -225,7 +226,7 @@ static void arm64_show_signal(int signo, const char *str)
 
 	pr_info("%s[%d]: unhandled exception: ", tsk->comm, task_pid_nr(tsk));
 	if (esr)
-		pr_cont("%s, ESR 0x%08x, ", esr_get_class_string(esr), esr);
+		pr_cont("%s, ESR 0x%08lx, ", esr_get_class_string(esr), esr);
 
 	pr_cont("%s", str);
 	print_vma_addr(KERN_CONT " in ", regs->pc);

@@ -233,42 +234,121 @@ static void arm64_show_signal(int signo, const char *str)
 	__show_regs(regs);
 }
 
+static unsigned long esr_to_error_code(unsigned long esr, unsigned long far)
+{
+	/*
+	 * If the faulting address is in the kernel, we must sanitize the ESR.
+	 * From userspace's point of view, kernel-only mappings don't exist
+	 * at all, so we report them as level 0 translation faults.
+	 * (This is not quite the way that "no mapping there at all" behaves:
+	 * an alignment fault not caused by the memory type would take
+	 * precedence over translation fault for a real access to empty
+	 * space. Unfortunately we can't easily distinguish "alignment fault
+	 * not caused by memory type" from "alignment fault caused by memory
+	 * type", so we ignore this wrinkle and just return the translation
+	 * fault.)
+	 */
+	if (!is_ttbr0_addr(untagged_addr(far))) {
+		switch (ESR_ELx_EC(esr)) {
+		case ESR_ELx_EC_DABT_LOW:
+			/*
+			 * These bits provide only information about the
+			 * faulting instruction, which userspace knows already.
+			 * We explicitly clear bits which are architecturally
+			 * RES0 in case they are given meanings in future.
+			 * We always report the ESR as if the fault was taken
+			 * to EL1 and so ISV and the bits in ISS[23:14] are
+			 * clear. (In fact it always will be a fault to EL1.)
+			 */
+			esr &= ESR_ELx_EC_MASK | ESR_ELx_IL |
+				ESR_ELx_CM | ESR_ELx_WNR;
+			esr |= ESR_ELx_FSC_FAULT;
+			break;
+		case ESR_ELx_EC_IABT_LOW:
+			/*
+			 * Claim a level 0 translation fault.
+			 * All other bits are architecturally RES0 for faults
+			 * reported with that DFSC value, so we clear them.
+			 */
+			esr &= ESR_ELx_EC_MASK | ESR_ELx_IL;
+			esr |= ESR_ELx_FSC_FAULT;
+			break;
+		default:
+			/*
+			 * This should never happen (entry.S only brings us
+			 * into this code for insn and data aborts from a lower
+			 * exception level). Fail safe by not providing an ESR
+			 * context record at all.
+			 */
+			WARN(1, "ESR 0x%x is not DABT or IABT from EL0\n", esr);
+			esr = 0;
+			break;
+		}
+	}
+
+	if (is_compat_task()) {
+		/* Use the compat FSR WnR */
+		return !!(esr & ESR_ELx_WNR) << FSR_WRITE_SHIFT;
+	}
+
+	return esr;
+}
+
 void arm64_force_sig_fault(int signo, int code, void __user *addr,
+			   unsigned long far, unsigned long esr,
 			   const char *str)
 {
-	arm64_show_signal(signo, str);
-	if (signo == SIGKILL)
+	arm64_show_signal(signo, esr, str);
+	if (signo == SIGKILL) {
 		force_sig(SIGKILL);
-	else
-		force_sig_fault(signo, code, addr);
+	} else {
+		struct kernel_siginfo info;
+		clear_siginfo(&info);
+		info.si_signo = signo;
+		info.si_errno = 0;
+		info.si_code = code;
+		info.si_addr = addr;
+		info.arch.fault_address = far;
+		info.arch.error_code = esr_to_error_code(esr, far);
+		force_sig_info(&info);
+	}
 }
 
 void arm64_force_sig_mceerr(int code, void __user *addr, short lsb,
+			    unsigned long far, unsigned long esr,
 			    const char *str)
 {
-	arm64_show_signal(SIGBUS, str);
-	force_sig_mceerr(code, addr, lsb);
+	struct kernel_siginfo info;
+
+	arm64_show_signal(SIGBUS, esr, str);
+
+	clear_siginfo(&info);
+	info.si_signo = SIGBUS;
+	info.si_errno = 0;
+	info.si_code = code;
+	info.si_addr = addr;
+	info.si_addr_lsb = lsb;
+	info.arch.fault_address = far;
+	info.arch.error_code = esr_to_error_code(esr, far);
+	force_sig_info(&info);
 }
 
 void arm64_force_sig_ptrace_errno_trap(int errno, void __user *addr,
 				       const char *str)
 {
-	arm64_show_signal(SIGTRAP, str);
+	arm64_show_signal(SIGTRAP, 0, str);
 	force_sig_ptrace_errno_trap(errno, addr);
 }
 
 void arm64_notify_die(const char *str, struct pt_regs *regs,
 		      int signo, int sicode, void __user *addr,
-		      int err)
+		      int esr)
 {
 	if (user_mode(regs)) {
 		WARN_ON(regs != current_pt_regs());
-		current->thread.fault_address = 0;
-		current->thread.fault_code = err;
-
-		arm64_force_sig_fault(signo, sicode, addr, str);
+		arm64_force_sig_fault(signo, sicode, addr, 0, esr, str);
 	} else {
-		die(str, regs, err);
+		die(str, regs, esr);
 	}
 }

@@ -813,10 +893,7 @@ void bad_el0_sync(struct pt_regs *regs, int reason, unsigned int esr)
 {
 	void __user *pc = (void __user *)instruction_pointer(regs);
 
-	current->thread.fault_address = 0;
-	current->thread.fault_code = esr;
-
-	arm64_force_sig_fault(SIGILL, ILL_ILLOPC, pc,
+	arm64_force_sig_fault(SIGILL, ILL_ILLOPC, pc, 0, esr,
 			      "Bad EL0 synchronous exception");
 }

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c9cedc0432d2..a7bada1392b3 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -41,7 +41,7 @@
 #include <asm/traps.h>
 
 struct fault_info {
-	int	(*fn)(unsigned long addr, unsigned int esr,
+	int	(*fn)(unsigned long far, unsigned int esr,
 		      struct pt_regs *regs);
 	int	sig;
 	int	code;

@@ -320,75 +320,19 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	die_kernel_fault(msg, addr, esr, regs);
 }
 
-static void set_thread_esr(unsigned long address, unsigned int esr)
+static void do_bad_area(unsigned long far, unsigned int esr,
+			struct pt_regs *regs)
 {
-	current->thread.fault_address = address;
+	unsigned long addr = untagged_addr(far);
 
-	/*
-	 * If the faulting address is in the kernel, we must sanitize the ESR.
-	 * From userspace's point of view, kernel-only mappings don't exist
-	 * at all, so we report them as level 0 translation faults.
-	 * (This is not quite the way that "no mapping there at all" behaves:
-	 * an alignment fault not caused by the memory type would take
-	 * precedence over translation fault for a real access to empty
-	 * space. Unfortunately we can't easily distinguish "alignment fault
-	 * not caused by memory type" from "alignment fault caused by memory
-	 * type", so we ignore this wrinkle and just return the translation
-	 * fault.)
-	 */
-	if (!is_ttbr0_addr(current->thread.fault_address)) {
-		switch (ESR_ELx_EC(esr)) {
-		case ESR_ELx_EC_DABT_LOW:
-			/*
-			 * These bits provide only information about the
-			 * faulting instruction, which userspace knows already.
-			 * We explicitly clear bits which are architecturally
-			 * RES0 in case they are given meanings in future.
-			 * We always report the ESR as if the fault was taken
-			 * to EL1 and so ISV and the bits in ISS[23:14] are
-			 * clear. (In fact it always will be a fault to EL1.)
-			 */
-			esr &= ESR_ELx_EC_MASK | ESR_ELx_IL |
-				ESR_ELx_CM | ESR_ELx_WNR;
-			esr |= ESR_ELx_FSC_FAULT;
-			break;
-		case ESR_ELx_EC_IABT_LOW:
-			/*
-			 * Claim a level 0 translation fault.
-			 * All other bits are architecturally RES0 for faults
-			 * reported with that DFSC value, so we clear them.
-			 */
-			esr &= ESR_ELx_EC_MASK | ESR_ELx_IL;
-			esr |= ESR_ELx_FSC_FAULT;
-			break;
-		default:
-			/*
-			 * This should never happen (entry.S only brings us
-			 * into this code for insn and data aborts from a lower
-			 * exception level). Fail safe by not providing an ESR
-			 * context record at all.
-			 */
-			WARN(1, "ESR 0x%x is not DABT or IABT from EL0\n", esr);
-			esr = 0;
-			break;
-		}
-	}
-
-	current->thread.fault_code = esr;
-}
-
-static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
-{
 	/*
 	 * If we are in kernel mode at this point, we have no context to
 	 * handle this fault with.
 	 */
 	if (user_mode(regs)) {
 		const struct fault_info *inf = esr_to_fault_info(esr);
-
-		set_thread_esr(addr, esr);
 		arm64_force_sig_fault(inf->sig, inf->code, (void __user *)addr,
-				      inf->name);
+				      far, esr, inf->name);
 	} else {
 		__do_kernel_fault(addr, esr, regs);
 	}

@@ -439,7 +383,7 @@ static bool is_write_abort(unsigned int esr)
 	return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM);
 }
 
-static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
+static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 				   struct pt_regs *regs)
 {
 	const struct fault_info *inf;

@@ -447,6 +391,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	vm_fault_t fault, major = 0;
 	unsigned long vm_flags = VM_ACCESS_FLAGS;
 	unsigned int mm_flags = FAULT_FLAG_DEFAULT;
+	unsigned long addr = untagged_addr(far);
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;

@@ -570,13 +515,12 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	}
 
 	inf = esr_to_fault_info(esr);
-	set_thread_esr(addr, esr);
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to successfully fix up
 		 * this page fault.
 		 */
-		arm64_force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)addr,
+		arm64_force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)addr, far, esr,
 				      inf->name);
 	} else if (fault & (VM_FAULT_HWPOISON_LARGE | VM_FAULT_HWPOISON)) {
 		unsigned int lsb;

@@ -586,7 +530,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 			lsb = hstate_index_to_shift(VM_FAULT_GET_HINDEX(fault));
 
 		arm64_force_sig_mceerr(BUS_MCEERR_AR, (void __user *)addr, lsb,
-				       inf->name);
+				       far, esr, inf->name);
 	} else {
 		/*
 		 * Something tried to access memory that isn't in our memory

@@ -594,7 +538,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 		 */
 		arm64_force_sig_fault(SIGSEGV,
 				      fault == VM_FAULT_BADACCESS ? SEGV_ACCERR : SEGV_MAPERR,
-				      (void __user *)addr,
+				      (void __user *)addr, far, esr,
 				      inf->name);
 	}

@@ -605,30 +549,32 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	return 0;
 }
 
-static int __kprobes do_translation_fault(unsigned long addr,
+static int __kprobes do_translation_fault(unsigned long far,
 					  unsigned int esr,
 					  struct pt_regs *regs)
 {
+	unsigned long addr = untagged_addr(far);
+
 	if (is_ttbr0_addr(addr))
-		return do_page_fault(addr, esr, regs);
+		return do_page_fault(far, esr, regs);
 
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_alignment_fault(unsigned long addr, unsigned int esr,
+static int do_alignment_fault(unsigned long far, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	do_bad_area(addr, esr, regs);
+	do_bad_area(far, esr, regs);
 	return 0;
 }
 
-static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_bad(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	return 1; /* "fault" */
 }
 
-static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+static int do_sea(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf;
 	void __user *siaddr;

@@ -644,7 +590,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	if (esr & ESR_ELx_FnV)
 		siaddr = NULL;
 	else
-		siaddr  = (void __user *)addr;
+		siaddr  = (void __user *)untagged_addr(far);
 	arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
 
 	return 0;

@@ -717,11 +663,12 @@ static const struct fault_info fault_info[] = {
 	{ do_bad,		SIGKILL, SI_KERNEL,	"unknown 63"			},
 };
 
-void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs)
 {
 	const struct fault_info *inf = esr_to_fault_info(esr);
+	unsigned long addr = untagged_addr(far);
 
-	if (!inf->fn(addr, esr, regs))
+	if (!inf->fn(far, esr, regs))
 		return;
 
 	if (!user_mode(regs)) {

@@ -730,8 +677,8 @@ void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 		show_pte(addr);
 	}
 
-	arm64_notify_die(inf->name, regs,
-			 inf->sig, inf->code, (void __user *)addr, esr);
+	arm64_notify_die(inf->name, regs, inf->sig, inf->code,
+			 (void __user *)addr, esr);
 }
 NOKPROBE_SYMBOL(do_mem_abort);

@@ -744,8 +691,8 @@ NOKPROBE_SYMBOL(do_el0_irq_bp_hardening);
 
 void do_sp_pc_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 {
-	arm64_notify_die("SP/PC alignment exception", regs,
-			 SIGBUS, BUS_ADRALN, (void __user *)addr, esr);
+	arm64_notify_die("SP/PC alignment exception", regs, SIGBUS, BUS_ADRALN,
+			 (void __user *)addr, esr);
 }
 NOKPROBE_SYMBOL(do_sp_pc_abort);

@@ -871,8 +818,8 @@ void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,
 		arm64_apply_bp_hardening();
 
 	if (inf->fn(addr_if_watchpoint, esr, regs)) {
-		arm64_notify_die(inf->name, regs,
-				 inf->sig, inf->code, (void __user *)pc, esr);
+		arm64_notify_die(inf->name, regs, inf->sig, inf->code,
+				 (void __user *)pc, esr);
 	}
 
 	debug_exception_exit(regs);

-- 
2.26.2.761.g0e0b3e54be-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6 3/3] arm64: Expose FAR_EL1 tag bits in sigcontext

From: Peter Collingbourne <hidden>
Date: 2020-05-21 02:31:11

The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
address exposed via siginfo.si_addr and sigcontext.fault_address. However,
the tag bits may be needed by tools in order to accurately diagnose
memory errors, such as HWASan [1] or future tools based on the Memory
Tagging Extension (MTE).

We should not stop clearing these bits in the existing fault address fields,
because there may be existing userspace applications that are expecting the tag
bits to be cleared. Instead, create a fault_addr_top_byte_context in sigcontext
(similar to the existing esr_context), and store the tag bits of FAR_EL1 there.

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Signed-off-by: Peter Collingbourne <redacted>
---
v6:
- move fault address and fault code into the kernel_siginfo data structure
- split the patch in three since it was getting large and now has
  generic and arch-specific parts

v5:
- add padding to fault_addr_top_byte_context in order to ensure the correct
  size and preserve sp alignment

v4:
- expose only the tag bits in the context instead of the entire FAR_EL1
- remove mention of the new context from the sigcontext.__reserved[] note

v3:
- add documentation to tagged-pointers.rst
- update comments in sigcontext.h

v2:
- revert changes to hw_breakpoint.c
- rename set_thread_esr to set_thread_far_esr

 Documentation/arm64/tagged-pointers.rst  | 17 ++++++++++------
 arch/arm64/include/asm/signal.h          |  3 +++
 arch/arm64/include/asm/traps.h           |  4 ++--
 arch/arm64/include/uapi/asm/sigcontext.h | 24 +++++++++++++++++------
 arch/arm64/kernel/debug-monitors.c       |  2 +-
 arch/arm64/kernel/ptrace.c               |  2 +-
 arch/arm64/kernel/signal.c               | 25 ++++++++++++++++++++++++
 arch/arm64/kernel/traps.c                | 10 ++++++----
 arch/arm64/mm/fault.c                    |  8 ++++----
 9 files changed, 71 insertions(+), 24 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index eab4323609b9..c6e9592a9dea 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst

@@ -53,12 +53,17 @@ visibility.
 Preserving tags
 ---------------
 
-Non-zero tags are not preserved when delivering signals. This means that
-signal handlers in applications making use of tags cannot rely on the
-tag information for user virtual addresses being maintained for fields
-inside siginfo_t. One exception to this rule is for signals raised in
-response to watchpoint debug exceptions, where the tag information will
-be preserved.
+Non-zero tags are not preserved in the fault address fields
+siginfo.si_addr or sigcontext.fault_address when delivering
+signals. This means that signal handlers in applications making use
+of tags cannot rely on the tag information for user virtual addresses
+being maintained in these fields. One exception to this rule is for
+signals raised in response to watchpoint debug exceptions, where the
+tag information will be preserved.
+
+The fault address tag is preserved in the fault_addr_top_byte field of
+the signal frame record fault_addr_top_byte_context, which is present
+for signals raised in response to data aborts and instruction aborts.
 
 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.

diff --git a/arch/arm64/include/asm/signal.h b/arch/arm64/include/asm/signal.h
index f5c001b0a125..c80eb3b3ea40 100644
--- a/arch/arm64/include/asm/signal.h
+++ b/arch/arm64/include/asm/signal.h

@@ -9,6 +9,9 @@ struct arch_private_siginfo {
 	/* FAR_EL1 value */
 	unsigned long fault_address;
 
+	/* Mask of defined bits in the top byte of FAR_EL1 */
+	unsigned char fault_address_top_byte_mask;
+
 	/* Sanitized ESR_EL1 value, or FSR/syscall number in compat mode */
 	unsigned long error_code;
 };

diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 5ed5be5347e6..981b930b1e6f 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h

@@ -27,8 +27,8 @@ void unregister_undef_hook(struct undef_hook *hook);
 void force_signal_inject(int signal, int code, unsigned long address);
 void arm64_notify_segfault(unsigned long addr);
 void arm64_force_sig_fault(int signo, int code, void __user *addr,
-			   unsigned long far, unsigned long esr,
-			   const char *str);
+			   unsigned long far, unsigned char far_tb_mask,
+			   unsigned long esr, const char *str);
 void arm64_force_sig_mceerr(int code, void __user *addr, short lsb,
 			    unsigned long far, unsigned long esr,
 			    const char *str);

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 8b0ebce92427..6ce5e1bb7efd 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h

@@ -44,11 +44,12 @@ struct sigcontext {
  *
  *	0x210		fpsimd_context
  *	 0x10		esr_context
+ *	 0x10		fault_addr_top_byte_context
  *	0x8a0		sve_context (vl <= 64) (optional)
  *	 0x20		extra_context (optional)
  *	 0x10		terminator (null _aarch64_ctx)
  *
- *	0x510		(reserved for future allocation)
+ *	0x500		(reserved for future allocation)
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

@@ -94,17 +95,28 @@ struct esr_context {
 	__u64 esr;
 };
 
+/* Top byte of fault address (normally not exposed via si_addr) */
+#define FAULT_ADDR_TOP_BYTE_MAGIC	0x46544201
+
+struct fault_addr_top_byte_context {
+	struct _aarch64_ctx head;
+	__u8 flags;
+	__u8 fault_addr_top_byte;
+	__u8 fault_addr_top_byte_mask;
+	__u8 __reserved[5];
+};
+
 /*
  * extra_context: describes extra space in the signal frame for
  * additional structures that don't fit in sigcontext.__reserved[].
  *
  * Note:
  *
- * 1) fpsimd_context, esr_context and extra_context must be placed in
- * sigcontext.__reserved[] if present.  They cannot be placed in the
- * extra space.  Any other record can be placed either in the extra
- * space or in sigcontext.__reserved[], unless otherwise specified in
- * this file.
+ * 1) fpsimd_context, esr_context, fault_addr_top_byte_context and
+ * extra_context must be placed in sigcontext.__reserved[] if present.
+ * They cannot be placed in the extra space.  Any other record can be
+ * placed either in the extra space or in sigcontext.__reserved[],
+ * unless otherwise specified in this file.
  *
  * 2) There must not be more than one extra_context.
  *

diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 498e6393b2ca..575a907cd4fc 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c

@@ -232,7 +232,7 @@ static void send_user_sigtrap(int si_code)
 		local_irq_enable();
 
 	arm64_force_sig_fault(SIGTRAP, si_code,
-			      (void __user *)instruction_pointer(regs), 0, 0,
+			      (void __user *)instruction_pointer(regs), 0, 0, 0,
 			      "User debug trap");
 }

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 51bb8bcaf24b..9b20284df88b 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c

@@ -198,7 +198,7 @@ static void ptrace_hbptriggered(struct perf_event *bp,
 	}
 #endif
 	arm64_force_sig_fault(SIGTRAP, TRAP_HWBKPT,
-			      (void __user *)(bkpt->trigger), 0, 0,
+			      (void __user *)(bkpt->trigger), 0, 0, 0,
 			      desc);
 }

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 10d7e9832a89..f6783de54412 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c

@@ -55,6 +55,7 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long ftb_offset;
 	unsigned long sve_offset;
 	unsigned long extra_offset;
 	unsigned long end_offset;

@@ -383,6 +384,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			break;
 
 		case ESR_MAGIC:
+		case FAULT_ADDR_TOP_BYTE_MAGIC:
 			/* ignore */
 			break;

@@ -582,6 +584,14 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
 				     sizeof(struct esr_context));
 		if (err)
 			return err;
+        }
+
+        if (add_all || info->arch.fault_address_top_byte_mask) {
+		err = sigframe_alloc(
+			user, &user->ftb_offset,
+			sizeof(struct fault_addr_top_byte_context));
+		if (err)
+			return err;
 	}
 
 	if (system_supports_sve()) {

@@ -644,6 +654,21 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(info->arch.error_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->ftb_offset) {
+		struct fault_addr_top_byte_context __user *ftb_ctx =
+			apply_user_offset(user, user->ftb_offset);
+
+		__put_user_error(FAULT_ADDR_TOP_BYTE_MAGIC,
+				 &ftb_ctx->head.magic, err);
+		__put_user_error(sizeof(*ftb_ctx), &ftb_ctx->head.size, err);
+		__put_user_error(0, &ftb_ctx->flags, err);
+		__put_user_error((info->arch.fault_address >> 56) &
+					 info->arch.fault_address_top_byte_mask,
+				 &ftb_ctx->fault_addr_top_byte, err);
+		__put_user_error(info->arch.fault_address_top_byte_mask,
+				 &ftb_ctx->fault_addr_top_byte_mask, err);
+	}
+
 	/* Scalable Vector Extension state, if present */
 	if (system_supports_sve() && err == 0 && user->sve_offset) {
 		struct sve_context __user *sve_ctx =

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 4545fe067ea9..8154f2562f74 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c

@@ -295,8 +295,8 @@ static unsigned long esr_to_error_code(unsigned long esr, unsigned long far)
 }
 
 void arm64_force_sig_fault(int signo, int code, void __user *addr,
-			   unsigned long far, unsigned long esr,
-			   const char *str)
+			   unsigned long far, unsigned char far_tb_mask,
+			   unsigned long esr, const char *str)
 {
 	arm64_show_signal(signo, esr, str);
 	if (signo == SIGKILL) {

@@ -309,6 +309,7 @@ void arm64_force_sig_fault(int signo, int code, void __user *addr,
 		info.si_code = code;
 		info.si_addr = addr;
 		info.arch.fault_address = far;
+		info.arch.fault_address_top_byte_mask = far_tb_mask;
 		info.arch.error_code = esr_to_error_code(esr, far);
 		force_sig_info(&info);
 	}

@@ -329,6 +330,7 @@ void arm64_force_sig_mceerr(int code, void __user *addr, short lsb,
 	info.si_addr = addr;
 	info.si_addr_lsb = lsb;
 	info.arch.fault_address = far;
+	info.arch.fault_address_top_byte_mask = 0xff;
 	info.arch.error_code = esr_to_error_code(esr, far);
 	force_sig_info(&info);
 }

@@ -346,7 +348,7 @@ void arm64_notify_die(const char *str, struct pt_regs *regs,
 {
 	if (user_mode(regs)) {
 		WARN_ON(regs != current_pt_regs());
-		arm64_force_sig_fault(signo, sicode, addr, 0, esr, str);
+		arm64_force_sig_fault(signo, sicode, addr, 0, 0, esr, str);
 	} else {
 		die(str, regs, esr);
 	}

@@ -893,7 +895,7 @@ void bad_el0_sync(struct pt_regs *regs, int reason, unsigned int esr)
 {
 	void __user *pc = (void __user *)instruction_pointer(regs);
 
-	arm64_force_sig_fault(SIGILL, ILL_ILLOPC, pc, 0, esr,
+	arm64_force_sig_fault(SIGILL, ILL_ILLOPC, pc, 0, 0, esr,
 			      "Bad EL0 synchronous exception");
 }

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index a7bada1392b3..1ba95f308c10 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c

@@ -332,7 +332,7 @@ static void do_bad_area(unsigned long far, unsigned int esr,
 	if (user_mode(regs)) {
 		const struct fault_info *inf = esr_to_fault_info(esr);
 		arm64_force_sig_fault(inf->sig, inf->code, (void __user *)addr,
-				      far, esr, inf->name);
+				      far, 0xff, esr, inf->name);
 	} else {
 		__do_kernel_fault(addr, esr, regs);
 	}

@@ -520,8 +520,8 @@ static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 		 * We had some memory, but were unable to successfully fix up
 		 * this page fault.
 		 */
-		arm64_force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)addr, far, esr,
-				      inf->name);
+		arm64_force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)addr,
+				      far, 0xff, esr, inf->name);
 	} else if (fault & (VM_FAULT_HWPOISON_LARGE | VM_FAULT_HWPOISON)) {
 		unsigned int lsb;

@@ -538,7 +538,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
 		 */
 		arm64_force_sig_fault(SIGSEGV,
 				      fault == VM_FAULT_BADACCESS ? SEGV_ACCERR : SEGV_MAPERR,
-				      (void __user *)addr, far, esr,
+				      (void __user *)addr, far, 0xff, esr,
 				      inf->name);
 	}

-- 
2.26.2.761.g0e0b3e54be-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6 2/3] arm64: Move fault address and fault code into kernel_siginfo

From: kbuild test robot <hidden>
Date: 2020-05-21 13:35:26

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm-perf/for-next/perf]
[also build test WARNING on linus/master v5.7-rc6]
[cannot apply to arm64/for-next/core next-20200519]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Peter-Collingbourne/arm64-Expose-FAR_EL1-tag-bits-in-sigcontext/20200521-103345
base:   https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-next/perf
config: arm64-randconfig-r005-20200520 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 3393cc4cebf9969db94dc424b7a2b6195589c33b)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <redacted>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

quoted

arch/arm64/kernel/traps.c:283:55: warning: format specifies type 'unsigned int' but the argument has type 'unsigned long' [-Wformat]

WARN(1, "ESR 0x%x is not DABT or IABT from EL0n", esr);
~~                                  ^~~
%lx
include/asm-generic/bug.h:124:29: note: expanded from macro 'WARN'
__WARN_printf(TAINT_WARN, format);                                                                   ^~~~~~
include/asm-generic/bug.h:92:17: note: expanded from macro '__WARN_printf'
__warn_printk(arg);                                                                      ^~~
arch/arm64/kernel/traps.c:826:26: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_UNKNOWN]            = "Unknown/Uncategorized",
^~~~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:827:22: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_WFx]                = "WFI/WFE",
^~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:828:26: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_CP15_32]            = "CP15 MCR/MRC",
^~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:829:26: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_CP15_64]            = "CP15 MCRR/MRRC",
^~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:830:26: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_CP14_MR]            = "CP14 MCR/MRC",
^~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:831:26: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_CP14_LS]            = "CP14 LDC/STC",
^~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:832:27: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_FP_ASIMD]           = "ASIMD",
^~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:833:26: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_CP10_ID]            = "CP10 MRC/VMRS",
^~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:834:22: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_PAC]                = "PAC",
^~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:835:26: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_CP14_64]            = "CP14 MCRR/MRRC",
^~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:836:22: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_ILL]                = "PSTATE.IL",
^~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:837:24: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_SVC32]              = "SVC (AArch32)",
^~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:838:24: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_HVC32]              = "HVC (AArch32)",
^~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:839:24: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_SMC32]              = "SMC (AArch32)",
^~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:840:24: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
[ESR_ELx_EC_SVC64]              = "SVC (AArch64)",
^~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:825:28: note: previous initialization is here
[0 ... ESR_ELx_EC_MAX]          = "UNRECOGNIZED EC",
^~~~~~~~~~~~~~~~~
arch/arm64/kernel/traps.c:841:24: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]

vim +283 arch/arm64/kernel/traps.c

   236	
   237	static unsigned long esr_to_error_code(unsigned long esr, unsigned long far)
   238	{
   239		/*
   240		 * If the faulting address is in the kernel, we must sanitize the ESR.
   241		 * From userspace's point of view, kernel-only mappings don't exist
   242		 * at all, so we report them as level 0 translation faults.
   243		 * (This is not quite the way that "no mapping there at all" behaves:
   244		 * an alignment fault not caused by the memory type would take
   245		 * precedence over translation fault for a real access to empty
   246		 * space. Unfortunately we can't easily distinguish "alignment fault
   247		 * not caused by memory type" from "alignment fault caused by memory
   248		 * type", so we ignore this wrinkle and just return the translation
   249		 * fault.)
   250		 */
   251		if (!is_ttbr0_addr(untagged_addr(far))) {
   252			switch (ESR_ELx_EC(esr)) {
   253			case ESR_ELx_EC_DABT_LOW:
   254				/*
   255				 * These bits provide only information about the
   256				 * faulting instruction, which userspace knows already.
   257				 * We explicitly clear bits which are architecturally
   258				 * RES0 in case they are given meanings in future.
   259				 * We always report the ESR as if the fault was taken
   260				 * to EL1 and so ISV and the bits in ISS[23:14] are
   261				 * clear. (In fact it always will be a fault to EL1.)
   262				 */
   263				esr &= ESR_ELx_EC_MASK | ESR_ELx_IL |
   264					ESR_ELx_CM | ESR_ELx_WNR;
   265				esr |= ESR_ELx_FSC_FAULT;
   266				break;
   267			case ESR_ELx_EC_IABT_LOW:
   268				/*
   269				 * Claim a level 0 translation fault.
   270				 * All other bits are architecturally RES0 for faults
   271				 * reported with that DFSC value, so we clear them.
   272				 */
   273				esr &= ESR_ELx_EC_MASK | ESR_ELx_IL;
   274				esr |= ESR_ELx_FSC_FAULT;
   275				break;
   276			default:
   277				/*
   278				 * This should never happen (entry.S only brings us
   279				 * into this code for insn and data aborts from a lower
   280				 * exception level). Fail safe by not providing an ESR
   281				 * context record at all.
   282				 */
 > 283				WARN(1, "ESR 0x%x is not DABT or IABT from EL0\n", esr);
   284				esr = 0;
   285				break;
   286			}
   287		}
   288	
   289		if (is_compat_task()) {
   290			/* Use the compat FSR WnR */
   291			return !!(esr & ESR_ELx_WNR) << FSR_WRITE_SHIFT;
   292		}
   293	
   294		return esr;
   295	}
   296	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help