[PATCH 0/9] arm64: Consolidate use of show_unhandled_signals

[PATCH 0/9] arm64: Consolidate use of show_unhandled_signals

[Will Deacon]

Hi all,

This patch series attempts to consolidate our handling of
show_unhandled_signals into arm64_force_sig_info. This helps to keep
things consistent, as well as avoid accidentally printing messages
where they aren't necessarily appropriate, or end up leaking kernel
state.

In writing this, I found a few weird behaviours with the way in which we
populate siginfo_t and also some opportunities for tidy-ups and
simplification.

Patches based on fixes/core.

Cheers,

Will

--->8

Will Deacon (9):
  arm64: signal: Make force_signal_inject more robust
  arm64: signal: Force SIGKILL for unknown signals in
    force_signal_inject
  arm64: Introduce arm64_force_sig_info and hook up in arm64_notify_die
  arm64: signal: Don't print anything directly in force_signal_inject
  arm64: Pass user fault info to arm64_notify_die instead of printing it
  arm64: mm: Rework unhandled user pagefaults to call
    arm64_force_sig_info
  arm64: signal: Call arm64_notify_segfault when failing to deliver
    signal
  arm64: Move show_unhandled_signals_ratelimited into traps.c
  arm64: Use arm64_force_sig_info instead of force_sig_info

 arch/arm64/include/asm/system_misc.h |  11 ----
 arch/arm64/include/asm/traps.h       |   8 +--
 arch/arm64/kernel/debug-monitors.c   |   3 +-
 arch/arm64/kernel/fpsimd.c           |   5 +-
 arch/arm64/kernel/ptrace.c           |   2 +-
 arch/arm64/kernel/signal.c           |   7 +--
 arch/arm64/kernel/signal32.c         |  13 +---
 arch/arm64/kernel/traps.c            |  72 ++++++++++++++-------
 arch/arm64/mm/fault.c                | 117 +++++++++++++----------------------
 9 files changed, 108 insertions(+), 130 deletions(-)

-- 
2.1.4

[PATCH 1/9] arm64: signal: Make force_signal_inject more robust

[Will Deacon]

force_signal_inject is a little flakey:

  * It only knows about SIGILL and SIGSEGV, so can potentially deliver
    other signals based on a partially initialised siginfo_t

  * It sets si_addr to point at the PC for SIGSEGV

  * It always operates on current, so doesn't need the regs argument

This patch fixes these issues by always assigning the si_addr field to
the address parameter of the function and updates the callers (including
those that indirectly call via arm64_notify_segfault) accordingly.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/traps.h |  6 ++----
 arch/arm64/kernel/fpsimd.c     |  5 ++---
 arch/arm64/kernel/traps.c      | 23 ++++++++++++-----------
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 178e338d2889..1ee63dc38579 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -35,10 +35,8 @@ struct undef_hook {
 
 void register_undef_hook(struct undef_hook *hook);
 void unregister_undef_hook(struct undef_hook *hook);
-void force_signal_inject(int signal, int code, struct pt_regs *regs,
-			 unsigned long address);
-
-void arm64_notify_segfault(struct pt_regs *regs, unsigned long addr);
+void force_signal_inject(int signal, int code, unsigned long address);
+void arm64_notify_segfault(unsigned long addr);
 
 /*
  * Move regs->pc to next instruction and do necessary setup before it
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index e7226c4c7493..6964ff867d4a 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -285,8 +285,7 @@ static void task_fpsimd_save(void)
 				 * re-enter user with corrupt state.
 				 * There's no way to recover, so kill it:
 				 */
-				force_signal_inject(
-					SIGKILL, 0, current_pt_regs(), 0);
+				force_signal_inject(SIGKILL, 0, 0);
 				return;
 			}
 
@@ -831,7 +830,7 @@ asmlinkage void do_sve_acc(unsigned int esr, struct pt_regs *regs)
 {
 	/* Even if we chose not to use SVE, the hardware could still trap: */
 	if (unlikely(!system_supports_sve()) || WARN_ON(is_compat_task())) {
-		force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
+		force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc);
 		return;
 	}
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index eb2d15147e8d..c478d8e27649 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -311,12 +311,13 @@ static int call_undef_hook(struct pt_regs *regs)
 	return fn ? fn(regs, instr) : 1;
 }
 
-void force_signal_inject(int signal, int code, struct pt_regs *regs,
-			 unsigned long address)
+void force_signal_inject(int signal, int code, unsigned long address)
 {
 	siginfo_t info;
-	void __user *pc = (void __user *)instruction_pointer(regs);
 	const char *desc;
+	struct pt_regs *regs = current_pt_regs();
+
+	clear_siginfo(&info);
 
 	switch (signal) {
 	case SIGILL:
@@ -332,15 +333,15 @@ void force_signal_inject(int signal, int code, struct pt_regs *regs,
 
 	if (unhandled_signal(current, signal) &&
 	    show_unhandled_signals_ratelimited()) {
-		pr_info("%s[%d]: %s: pc=%p\n",
-			current->comm, task_pid_nr(current), desc, pc);
+		pr_info("%s[%d]: %s: pc=%08llx\n",
+			current->comm, task_pid_nr(current), desc, regs->pc);
 		dump_instr(KERN_INFO, regs);
 	}
 
 	info.si_signo = signal;
 	info.si_errno = 0;
 	info.si_code  = code;
-	info.si_addr  = pc;
+	info.si_addr  = (void __user *)address;
 
 	arm64_notify_die(desc, regs, &info, 0);
 }
@@ -348,7 +349,7 @@ void force_signal_inject(int signal, int code, struct pt_regs *regs,
 /*
  * Set up process info to signal segmentation fault - called on access error.
  */
-void arm64_notify_segfault(struct pt_regs *regs, unsigned long addr)
+void arm64_notify_segfault(unsigned long addr)
 {
 	int code;
 
@@ -359,7 +360,7 @@ void arm64_notify_segfault(struct pt_regs *regs, unsigned long addr)
 		code = SEGV_ACCERR;
 	up_read(&current->mm->mmap_sem);
 
-	force_signal_inject(SIGSEGV, code, regs, addr);
+	force_signal_inject(SIGSEGV, code, addr);
 }
 
 asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
@@ -371,7 +372,7 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
 	if (call_undef_hook(regs) == 0)
 		return;
 
-	force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
+	force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc);
 }
 
 int cpu_enable_cache_maint_trap(void *__unused)
@@ -426,12 +427,12 @@ static void user_cache_maint_handler(unsigned int esr, struct pt_regs *regs)
 		__user_cache_maint("ic ivau", address, ret);
 		break;
 	default:
-		force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
+		force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc);
 		return;
 	}
 
 	if (ret)
-		arm64_notify_segfault(regs, address);
+		arm64_notify_segfault(address);
 	else
 		arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
 }
-- 
2.1.4

[PATCH 2/9] arm64: signal: Force SIGKILL for unknown signals in force_signal_inject

[Will Deacon]

For signals other than SIGKILL or those with siginfo_layout(signal, code)
== SIL_FAULT then force_signal_inject does not initialise the siginfo_t
properly. Since the signal number is determined solely by the caller,
simply WARN on unknown signals and force to SIGKILL.

Reported-by: Dave Martin <Dave.Martin@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/traps.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index c478d8e27649..3f52c07b4bf4 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -331,6 +331,12 @@ void force_signal_inject(int signal, int code, unsigned long address)
 		break;
 	}
 
+	/* Force signals we don't understand to SIGKILL */
+	if (WARN_ON(signal != SIGKILL ||
+		    siginfo_layout(signal, code) != SIL_FAULT)) {
+		signal = SIGKILL;
+	}
+
 	if (unhandled_signal(current, signal) &&
 	    show_unhandled_signals_ratelimited()) {
 		pr_info("%s[%d]: %s: pc=%08llx\n",
-- 
2.1.4

[PATCH 3/9] arm64: Introduce arm64_force_sig_info and hook up in arm64_notify_die

[Will Deacon]

In preparation for consolidating our handling of printing unhandled
signals, introduce a wrapper around force_sig_info which can act as
the canonical place for dealing with show_unhandled_signals.

Initially, we just hook this up to arm64_notify_die.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/traps.h |  2 ++
 arch/arm64/kernel/traps.c      | 28 +++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 1ee63dc38579..c320f3bf6c57 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -37,6 +37,8 @@ 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_info(struct siginfo *info, const char *str,
+			  struct task_struct *tsk);
 
 /*
  * Move regs->pc to next instruction and do necessary setup before it
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 3f52c07b4bf4..b46b5be5bd6d 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -223,13 +223,39 @@ void die(const char *str, struct pt_regs *regs, int err)
 		do_exit(SIGSEGV);
 }
 
+void arm64_force_sig_info(struct siginfo *info, const char *str,
+			  struct task_struct *tsk)
+{
+	unsigned int esr = tsk->thread.fault_code;
+	struct pt_regs *regs = task_pt_regs(tsk);
+
+	if (!unhandled_signal(tsk, info->si_signo))
+		goto send_sig;
+
+	if (!show_unhandled_signals_ratelimited())
+		goto send_sig;
+
+	pr_info("%s[%d]: unhandled exception: ", tsk->comm, task_pid_nr(tsk));
+	if (esr)
+		pr_cont("%s, ESR 0x%03x, ", esr_get_class_string(esr), esr);
+
+	pr_cont("%s", str);
+	print_vma_addr(KERN_CONT " in ", regs->pc);
+	pr_cont("\n");
+	__show_regs(regs);
+
+send_sig:
+	force_sig_info(info->si_signo, info, tsk);
+}
+
 void arm64_notify_die(const char *str, struct pt_regs *regs,
 		      struct siginfo *info, int err)
 {
 	if (user_mode(regs)) {
+		WARN_ON(regs != current_pt_regs());
 		current->thread.fault_address = 0;
 		current->thread.fault_code = err;
-		force_sig_info(info->si_signo, info, current);
+		arm64_force_sig_info(info, str, current);
 	} else {
 		die(str, regs, err);
 	}
-- 
2.1.4

[PATCH 4/9] arm64: signal: Don't print anything directly in force_signal_inject

[Will Deacon]

arm64_notify_die deals with printing out information regarding unhandled
signals, so there's no need to roll our own code here.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/traps.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index b46b5be5bd6d..637ae3702950 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -363,13 +363,6 @@ void force_signal_inject(int signal, int code, unsigned long address)
 		signal = SIGKILL;
 	}
 
-	if (unhandled_signal(current, signal) &&
-	    show_unhandled_signals_ratelimited()) {
-		pr_info("%s[%d]: %s: pc=%08llx\n",
-			current->comm, task_pid_nr(current), desc, regs->pc);
-		dump_instr(KERN_INFO, regs);
-	}
-
 	info.si_signo = signal;
 	info.si_errno = 0;
 	info.si_code  = code;
-- 
2.1.4

[PATCH 5/9] arm64: Pass user fault info to arm64_notify_die instead of printing it

[Will Deacon]

There's no need for callers of arm64_notify_die to print information
about user faults. Instead, they can pass a string to arm64_notify_die
which will be printed subject to show_unhandled_signals.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/mm/fault.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index bff11553eb05..fd5928afd9cd 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -582,8 +582,6 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	const struct fault_info *inf;
 
 	inf = esr_to_fault_info(esr);
-	pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
-		inf->name, esr, addr);
 
 	/*
 	 * Synchronous aborts may interrupt code which had interrupts masked.
@@ -607,7 +605,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 		info.si_addr = NULL;
 	else
 		info.si_addr  = (void __user *)addr;
-	arm64_notify_die("", regs, &info, esr);
+	arm64_notify_die(inf->name, regs, &info, esr);
 
 	return 0;
 }
@@ -698,19 +696,17 @@ asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int esr,
 	if (!inf->fn(addr, esr, regs))
 		return;
 
-	pr_alert("Unhandled fault: %s at 0x%016lx\n",
-		 inf->name, addr);
-
-	mem_abort_decode(esr);
-
-	if (!user_mode(regs))
+	if (!user_mode(regs)) {
+		pr_alert("Unhandled fault at 0x%016lx\n", addr);
+		mem_abort_decode(esr);
 		show_pte(addr);
+	}
 
 	info.si_signo = inf->sig;
 	info.si_errno = 0;
 	info.si_code  = inf->code;
 	info.si_addr  = (void __user *)addr;
-	arm64_notify_die("", regs, &info, esr);
+	arm64_notify_die(inf->name, regs, &info, esr);
 }
 
 asmlinkage void __exception do_el0_irq_bp_hardening(void)
@@ -741,7 +737,6 @@ asmlinkage void __exception do_sp_pc_abort(unsigned long addr,
 					   struct pt_regs *regs)
 {
 	struct siginfo info;
-	struct task_struct *tsk = current;
 
 	if (user_mode(regs)) {
 		if (instruction_pointer(regs) > TASK_SIZE)
@@ -749,17 +744,11 @@ asmlinkage void __exception do_sp_pc_abort(unsigned long addr,
 		local_irq_enable();
 	}
 
-	if (show_unhandled_signals && unhandled_signal(tsk, SIGBUS))
-		pr_info_ratelimited("%s[%d]: %s exception: pc=%p sp=%p\n",
-				    tsk->comm, task_pid_nr(tsk),
-				    esr_get_class_string(esr), (void *)regs->pc,
-				    (void *)regs->sp);
-
 	info.si_signo = SIGBUS;
 	info.si_errno = 0;
 	info.si_code  = BUS_ADRALN;
 	info.si_addr  = (void __user *)addr;
-	arm64_notify_die("Oops - SP/PC alignment exception", regs, &info, esr);
+	arm64_notify_die("SP/PC alignment exception", regs, &info, esr);
 }
 
 int __init early_brk64(unsigned long addr, unsigned int esr,
@@ -814,14 +803,11 @@ asmlinkage int __exception do_debug_exception(unsigned long addr,
 	if (!inf->fn(addr, esr, regs)) {
 		rv = 1;
 	} else {
-		pr_alert("Unhandled debug exception: %s (0x%08x) at 0x%016lx\n",
-			 inf->name, esr, addr);
-
 		info.si_signo = inf->sig;
 		info.si_errno = 0;
 		info.si_code  = inf->code;
 		info.si_addr  = (void __user *)addr;
-		arm64_notify_die("", regs, &info, 0);
+		arm64_notify_die(inf->name, regs, &info, esr);
 		rv = 0;
 	}
 
-- 
2.1.4

[PATCH 6/9] arm64: mm: Rework unhandled user pagefaults to call arm64_force_sig_info

[Will Deacon]

Reporting unhandled user pagefaults via arm64_force_sig_info means
that __do_user_fault can be drastically simplified, since it no longer
has to worry about printing the fault information and can consequently
just take the siginfo as a parameter.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/mm/fault.c | 87 +++++++++++++++++++++------------------------------
 1 file changed, 36 insertions(+), 51 deletions(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index fd5928afd9cd..49dfb08a6c4d 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -43,6 +43,7 @@
 #include <asm/system_misc.h>
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
+#include <asm/traps.h>
 
 #include <acpi/ghes.h>
 
@@ -289,58 +290,31 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	do_exit(SIGKILL);
 }
 
-static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
-			    unsigned int esr, unsigned int sig, int code,
-			    struct pt_regs *regs, int fault)
+static void __do_user_fault(struct siginfo *info, unsigned int esr)
 {
-	struct siginfo si;
-	const struct fault_info *inf;
-	unsigned int lsb = 0;
-
-	if (unhandled_signal(tsk, sig) && show_unhandled_signals_ratelimited()) {
-		inf = esr_to_fault_info(esr);
-		pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x",
-			tsk->comm, task_pid_nr(tsk), inf->name, sig,
-			addr, esr);
-		print_vma_addr(KERN_CONT ", in ", regs->pc);
-		pr_cont("\n");
-		__show_regs(regs);
-	}
-
-	tsk->thread.fault_address = addr;
-	tsk->thread.fault_code = esr;
-	si.si_signo = sig;
-	si.si_errno = 0;
-	si.si_code = code;
-	si.si_addr = (void __user *)addr;
-	/*
-	 * Either small page or large page may be poisoned.
-	 * In other words, VM_FAULT_HWPOISON_LARGE and
-	 * VM_FAULT_HWPOISON are mutually exclusive.
-	 */
-	if (fault & VM_FAULT_HWPOISON_LARGE)
-		lsb = hstate_index_to_shift(VM_FAULT_GET_HINDEX(fault));
-	else if (fault & VM_FAULT_HWPOISON)
-		lsb = PAGE_SHIFT;
-	si.si_addr_lsb = lsb;
-
-	force_sig_info(sig, &si, tsk);
+	current->thread.fault_address = (unsigned long)info->si_addr;
+	current->thread.fault_code = esr;
+	arm64_force_sig_info(info, esr_to_fault_info(esr)->name, current);
 }
 
 static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 {
-	struct task_struct *tsk = current;
-	const struct fault_info *inf;
-
 	/*
 	 * If we are in kernel mode at this point, we have no context to
 	 * handle this fault with.
 	 */
 	if (user_mode(regs)) {
-		inf = esr_to_fault_info(esr);
-		__do_user_fault(tsk, addr, esr, inf->sig, inf->code, regs, 0);
-	} else
+		const struct fault_info *inf = esr_to_fault_info(esr);
+		struct siginfo si = {
+			.si_signo	= inf->sig,
+			.si_code	= inf->code,
+			.si_addr	= (void __user *)addr,
+		};
+
+		__do_user_fault(&si, esr);
+	} else {
 		__do_kernel_fault(addr, esr, regs);
+	}
 }
 
 #define VM_FAULT_BADMAP		0x010000
@@ -393,7 +367,8 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 {
 	struct task_struct *tsk;
 	struct mm_struct *mm;
-	int fault, sig, code, major = 0;
+	struct siginfo si;
+	int fault, major = 0;
 	unsigned long vm_flags = VM_READ | VM_WRITE;
 	unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
 
@@ -525,27 +500,37 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 		return 0;
 	}
 
+	clear_siginfo(&si);
+	si.si_addr = (void __user *)addr;
+
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to successfully fix up
 		 * this page fault.
 		 */
-		sig = SIGBUS;
-		code = BUS_ADRERR;
-	} else if (fault & (VM_FAULT_HWPOISON | VM_FAULT_HWPOISON_LARGE)) {
-		sig = SIGBUS;
-		code = BUS_MCEERR_AR;
+		si.si_signo	= SIGBUS;
+		si.si_code	= BUS_ADRERR;
+	} else if (fault & VM_FAULT_HWPOISON_LARGE) {
+		unsigned int hindex = VM_FAULT_GET_HINDEX(fault);
+
+		si.si_signo	= SIGBUS;
+		si.si_code	= BUS_MCEERR_AR;
+		si.si_addr_lsb	= hstate_index_to_shift(hindex);
+	} else if (fault & VM_FAULT_HWPOISON) {
+		si.si_signo	= SIGBUS;
+		si.si_code	= BUS_MCEERR_AR;
+		si.si_addr_lsb	= PAGE_SHIFT;
 	} else {
 		/*
 		 * Something tried to access memory that isn't in our memory
 		 * map.
 		 */
-		sig = SIGSEGV;
-		code = fault == VM_FAULT_BADACCESS ?
-			SEGV_ACCERR : SEGV_MAPERR;
+		si.si_signo	= SIGSEGV;
+		si.si_code	= fault == VM_FAULT_BADACCESS ?
+				  SEGV_ACCERR : SEGV_MAPERR;
 	}
 
-	__do_user_fault(tsk, addr, esr, sig, code, regs, fault);
+	__do_user_fault(&si, esr);
 	return 0;
 
 no_context:
-- 
2.1.4

[PATCH 7/9] arm64: signal: Call arm64_notify_segfault when failing to deliver signal

[Will Deacon]

If we fail to deliver a signal due to taking an unhandled fault on the
stackframe, we can all arm64_notify_segfault to deliver a SEGV can deal
with printing any unhandled signal messages for us, rather than roll our
own printing code.

A side-effect of this change is that we now deliver the frame address
in si_addr along with an si_code of SEGV_{ACC,MAP}ERR, rather than an
si_addr of 0 and an si_code of SI_KERNEL as before.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/signal.c   |  7 ++-----
 arch/arm64/kernel/signal32.c | 13 +++----------
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index f60c052e8d1c..e5c656d0e316 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -40,6 +40,7 @@
 #include <asm/fpsimd.h>
 #include <asm/ptrace.h>
 #include <asm/signal32.h>
+#include <asm/traps.h>
 #include <asm/vdso.h>
 
 /*
@@ -565,11 +566,7 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return regs->regs[0];
 
 badframe:
-	if (show_unhandled_signals)
-		pr_info_ratelimited("%s[%d]: bad frame in %s: pc=%08llx sp=%08llx\n",
-				    current->comm, task_pid_nr(current), __func__,
-				    regs->pc, regs->sp);
-	force_sig(SIGSEGV, current);
+	arm64_notify_segfault(regs->sp);
 	return 0;
 }
 
diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c
index 79feb861929b..68f5e07b592b 100644
--- a/arch/arm64/kernel/signal32.c
+++ b/arch/arm64/kernel/signal32.c
@@ -26,6 +26,7 @@
 #include <asm/esr.h>
 #include <asm/fpsimd.h>
 #include <asm/signal32.h>
+#include <asm/traps.h>
 #include <linux/uaccess.h>
 #include <asm/unistd.h>
 
@@ -307,11 +308,7 @@ asmlinkage int compat_sys_sigreturn(struct pt_regs *regs)
 	return regs->regs[0];
 
 badframe:
-	if (show_unhandled_signals)
-		pr_info_ratelimited("%s[%d]: bad frame in %s: pc=%08llx sp=%08llx\n",
-				    current->comm, task_pid_nr(current), __func__,
-				    regs->pc, regs->compat_sp);
-	force_sig(SIGSEGV, current);
+	arm64_notify_segfault(regs->compat_sp);
 	return 0;
 }
 
@@ -344,11 +341,7 @@ asmlinkage int compat_sys_rt_sigreturn(struct pt_regs *regs)
 	return regs->regs[0];
 
 badframe:
-	if (show_unhandled_signals)
-		pr_info_ratelimited("%s[%d]: bad frame in %s: pc=%08llx sp=%08llx\n",
-				    current->comm, task_pid_nr(current), __func__,
-				    regs->pc, regs->compat_sp);
-	force_sig(SIGSEGV, current);
+	arm64_notify_segfault(regs->compat_sp);
 	return 0;
 }
 
-- 
2.1.4

[PATCH 8/9] arm64: Move show_unhandled_signals_ratelimited into traps.c

[Will Deacon]

show_unhandled_signals_ratelimited is only called in traps.c, so move it
out of its macro in the dreaded system_misc.h and into a static function
in traps.c

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/system_misc.h | 11 -----------
 arch/arm64/kernel/traps.c            |  7 +++++++
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
index 07aa8e3c5630..28893a0b141d 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -45,17 +45,6 @@ extern void __show_regs(struct pt_regs *);
 
 extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
 
-#define show_unhandled_signals_ratelimited()				\
-({									\
-	static DEFINE_RATELIMIT_STATE(_rs,				\
-				      DEFAULT_RATELIMIT_INTERVAL,	\
-				      DEFAULT_RATELIMIT_BURST);		\
-	bool __show_ratelimited = false;				\
-	if (show_unhandled_signals && __ratelimit(&_rs))		\
-		__show_ratelimited = true;				\
-	__show_ratelimited;						\
-})
-
 int handle_guest_sea(phys_addr_t addr, unsigned int esr);
 
 #endif	/* __ASSEMBLY__ */
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 637ae3702950..4231488a5a9f 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -223,6 +223,13 @@ void die(const char *str, struct pt_regs *regs, int err)
 		do_exit(SIGSEGV);
 }
 
+static bool show_unhandled_signals_ratelimited(void)
+{
+	static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
+				      DEFAULT_RATELIMIT_BURST);
+	return show_unhandled_signals && __ratelimit(&rs);
+}
+
 void arm64_force_sig_info(struct siginfo *info, const char *str,
 			  struct task_struct *tsk)
 {
-- 
2.1.4

[PATCH 9/9] arm64: Use arm64_force_sig_info instead of force_sig_info

[Will Deacon]

Using arm64_force_sig_info means that printing messages about unhandled
signals is dealt with for us, so use that in preference to force_sig_info
and remove any homebrew printing code.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/debug-monitors.c | 3 ++-
 arch/arm64/kernel/ptrace.c         | 2 +-
 arch/arm64/kernel/traps.c          | 9 ++-------
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 53781f5687c5..06ca574495af 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -33,6 +33,7 @@
 #include <asm/daifflags.h>
 #include <asm/debug-monitors.h>
 #include <asm/system_misc.h>
+#include <asm/traps.h>
 
 /* Determine debug architecture. */
 u8 debug_monitors_arch(void)
@@ -223,7 +224,7 @@ static void send_user_sigtrap(int si_code)
 	if (interrupts_enabled(regs))
 		local_irq_enable();
 
-	force_sig_info(SIGTRAP, &info, current);
+	arm64_force_sig_info(&info, "User debug trap", current);
 }
 
 static int single_step_handler(unsigned long addr, unsigned int esr,
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6618036ae6d4..dd359ee02e2b 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -209,7 +209,7 @@ static void ptrace_hbptriggered(struct perf_event *bp,
 		force_sig_ptrace_errno_trap(si_errno, (void __user *)bkpt->trigger);
 	}
 #endif
-	force_sig_info(SIGTRAP, &info, current);
+	arm64_force_sig_info(&info, "Hardware breakpoint trap (ptrace)", current);
 }
 
 /*
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 4231488a5a9f..cb5ad77524f3 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -633,11 +633,6 @@ asmlinkage void bad_el0_sync(struct pt_regs *regs, int reason, unsigned int esr)
 {
 	siginfo_t info;
 	void __user *pc = (void __user *)instruction_pointer(regs);
-	console_verbose();
-
-	pr_crit("Bad EL0 synchronous exception detected on CPU%d, code 0x%08x -- %s\n",
-		smp_processor_id(), esr, esr_get_class_string(esr));
-	__show_regs(regs);
 
 	info.si_signo = SIGILL;
 	info.si_errno = 0;
@@ -645,9 +640,9 @@ asmlinkage void bad_el0_sync(struct pt_regs *regs, int reason, unsigned int esr)
 	info.si_addr  = pc;
 
 	current->thread.fault_address = 0;
-	current->thread.fault_code = 0;
+	current->thread.fault_code = esr;
 
-	force_sig_info(info.si_signo, &info, current);
+	arm64_force_sig_info(&info, "Bad EL0 synchronous exception", current);
 }
 
 #ifdef CONFIG_VMAP_STACK
-- 
2.1.4

[PATCH 3/9] arm64: Introduce arm64_force_sig_info and hook up in arm64_notify_die

[Mark Rutland]

On Thu, Feb 22, 2018 at 02:00:51PM +0000, Will Deacon wrote:
> +void arm64_force_sig_info(struct siginfo *info, const char *str,
> +			  struct task_struct *tsk)
> +{
> +	unsigned int esr = tsk->thread.fault_code;
> +	struct pt_regs *regs = task_pt_regs(tsk);
> +
> +	if (!unhandled_signal(tsk, info->si_signo))
> +		goto send_sig;
> +
> +	if (!show_unhandled_signals_ratelimited())
> +		goto send_sig;
> +
> +	pr_info("%s[%d]: unhandled exception: ", tsk->comm, task_pid_nr(tsk));
> +	if (esr)
> +		pr_cont("%s, ESR 0x%03x, ", esr_get_class_string(esr), esr);

Can we make that 0x%08x please? ESR_ELx.EC occupies bits 31-26...

I guess this was copied from __do_user_fault, and it's odd there, as
wherever that's called the EC is non-zero.

Thanks,
Mark.

[PATCH 9/9] arm64: Use arm64_force_sig_info instead of force_sig_info

[Mark Rutland]

On Thu, Feb 22, 2018 at 02:00:57PM +0000, Will Deacon wrote:
> Using arm64_force_sig_info means that printing messages about unhandled
> signals is dealt with for us, so use that in preference to force_sig_info
> and remove any homebrew printing code.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/kernel/debug-monitors.c | 3 ++-
>  arch/arm64/kernel/ptrace.c         | 2 +-
>  arch/arm64/kernel/traps.c          | 9 ++-------
>  3 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index 53781f5687c5..06ca574495af 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -33,6 +33,7 @@
>  #include <asm/daifflags.h>
>  #include <asm/debug-monitors.h>
>  #include <asm/system_misc.h>
> +#include <asm/traps.h>
>  
>  /* Determine debug architecture. */
>  u8 debug_monitors_arch(void)
> @@ -223,7 +224,7 @@ static void send_user_sigtrap(int si_code)
>  	if (interrupts_enabled(regs))
>  		local_irq_enable();
>  
> -	force_sig_info(SIGTRAP, &info, current);
> +	arm64_force_sig_info(&info, "User debug trap", current);
>  }
>  
>  static int single_step_handler(unsigned long addr, unsigned int esr,
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 6618036ae6d4..dd359ee02e2b 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -209,7 +209,7 @@ static void ptrace_hbptriggered(struct perf_event *bp,
>  		force_sig_ptrace_errno_trap(si_errno, (void __user *)bkpt->trigger);
>  	}
>  #endif
> -	force_sig_info(SIGTRAP, &info, current);
> +	arm64_force_sig_info(&info, "Hardware breakpoint trap (ptrace)", current);
>  }
>  
>  /*
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 4231488a5a9f..cb5ad77524f3 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -633,11 +633,6 @@ asmlinkage void bad_el0_sync(struct pt_regs *regs, int reason, unsigned int esr)
>  {
>  	siginfo_t info;
>  	void __user *pc = (void __user *)instruction_pointer(regs);
> -	console_verbose();
> -
> -	pr_crit("Bad EL0 synchronous exception detected on CPU%d, code 0x%08x -- %s\n",
> -		smp_processor_id(), esr, esr_get_class_string(esr));
> -	__show_regs(regs);

Here we dump the CPU so that a developer can figure out which
microarchitecture the exception was taken on (e.g. to allow them to
decode an IMP DEF SError). We added that in commit:

  8051f4d16ef1d037 ("arm64: report CPU number in bad_mode")

However, in arm64_force_sig_info() we don't dump the CPU, and I guess
that may be preemptible in cases other than bad_el0_sync().

Are we happy to lose this?

Thanks,
Mark.

[PATCH 9/9] arm64: Use arm64_force_sig_info instead of force_sig_info

[Will Deacon]

On Thu, Feb 22, 2018 at 03:48:21PM +0000, Mark Rutland wrote:
> On Thu, Feb 22, 2018 at 02:00:57PM +0000, Will Deacon wrote:
> > Using arm64_force_sig_info means that printing messages about unhandled
> > signals is dealt with for us, so use that in preference to force_sig_info
> > and remove any homebrew printing code.
> > 
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm64/kernel/debug-monitors.c | 3 ++-
> >  arch/arm64/kernel/ptrace.c         | 2 +-
> >  arch/arm64/kernel/traps.c          | 9 ++-------
> >  3 files changed, 5 insertions(+), 9 deletions(-)

[...]

> > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> > index 4231488a5a9f..cb5ad77524f3 100644
> > --- a/arch/arm64/kernel/traps.c
> > +++ b/arch/arm64/kernel/traps.c
> > @@ -633,11 +633,6 @@ asmlinkage void bad_el0_sync(struct pt_regs *regs, int reason, unsigned int esr)
> >  {
> >  	siginfo_t info;
> >  	void __user *pc = (void __user *)instruction_pointer(regs);
> > -	console_verbose();
> > -
> > -	pr_crit("Bad EL0 synchronous exception detected on CPU%d, code 0x%08x -- %s\n",
> > -		smp_processor_id(), esr, esr_get_class_string(esr));
> > -	__show_regs(regs);
> 
> Here we dump the CPU so that a developer can figure out which
> microarchitecture the exception was taken on (e.g. to allow them to
> decode an IMP DEF SError). We added that in commit:
> 
>   8051f4d16ef1d037 ("arm64: report CPU number in bad_mode")
> 
> However, in arm64_force_sig_info() we don't dump the CPU, and I guess
> that may be preemptible in cases other than bad_el0_sync().

__show_regs prints the CPU number already. If we're preemptible, then it
could be inaccurate.

Will

[PATCH 9/9] arm64: Use arm64_force_sig_info instead of force_sig_info

[Mark Rutland]

On Thu, Feb 22, 2018 at 04:58:05PM +0000, Will Deacon wrote:
> On Thu, Feb 22, 2018 at 03:48:21PM +0000, Mark Rutland wrote:
> > On Thu, Feb 22, 2018 at 02:00:57PM +0000, Will Deacon wrote:
> > > Using arm64_force_sig_info means that printing messages about unhandled
> > > signals is dealt with for us, so use that in preference to force_sig_info
> > > and remove any homebrew printing code.
> > > 
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > ---
> > >  arch/arm64/kernel/debug-monitors.c | 3 ++-
> > >  arch/arm64/kernel/ptrace.c         | 2 +-
> > >  arch/arm64/kernel/traps.c          | 9 ++-------
> > >  3 files changed, 5 insertions(+), 9 deletions(-)
> 
> [...]
> 
> > > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> > > index 4231488a5a9f..cb5ad77524f3 100644
> > > --- a/arch/arm64/kernel/traps.c
> > > +++ b/arch/arm64/kernel/traps.c
> > > @@ -633,11 +633,6 @@ asmlinkage void bad_el0_sync(struct pt_regs *regs, int reason, unsigned int esr)
> > >  {
> > >  	siginfo_t info;
> > >  	void __user *pc = (void __user *)instruction_pointer(regs);
> > > -	console_verbose();
> > > -
> > > -	pr_crit("Bad EL0 synchronous exception detected on CPU%d, code 0x%08x -- %s\n",
> > > -		smp_processor_id(), esr, esr_get_class_string(esr));
> > > -	__show_regs(regs);
> > 
> > Here we dump the CPU so that a developer can figure out which
> > microarchitecture the exception was taken on (e.g. to allow them to
> > decode an IMP DEF SError). We added that in commit:
> > 
> >   8051f4d16ef1d037 ("arm64: report CPU number in bad_mode")
> > 
> > However, in arm64_force_sig_info() we don't dump the CPU, and I guess
> > that may be preemptible in cases other than bad_el0_sync().
> 
> __show_regs prints the CPU number already. If we're preemptible, then it
> could be inaccurate.

Ah, that'll do. :)

Mark.