stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 4.9/4.14 BACKPORT 1/2] arm64: debug: Don't propagate UNKNOWN FAR into si_code for debug signals
@ 2019-04-03 10:33 Will Deacon
  2019-04-03 10:33 ` [PATCH 4.9 BACKPORT 2/2] arm64: debug: Ensure debug handlers check triggering exception level Will Deacon
       [not found] ` <20190405141522.584C621850@mail.kernel.org>
  0 siblings, 2 replies; 3+ messages in thread
From: Will Deacon @ 2019-04-03 10:33 UTC (permalink / raw)
  To: gregkh; +Cc: stable, linux-kernel, sashal, Will Deacon, Catalin Marinas

Commit b9a4b9d084d978f80eb9210727c81804588b42ff upstream.

FAR_EL1 is UNKNOWN for all debug exceptions other than those caused by
taking a hardware watchpoint. Unfortunately, if a debug handler returns
a non-zero value, then we will propagate the UNKNOWN FAR value to
userspace via the si_addr field of the SIGTRAP siginfo_t.

Instead, let's set si_addr to take on the PC of the faulting instruction,
which we have available in the current pt_regs.

Cc: <stable@vger.kernel.org> # 4.9.y and 4.14.y
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/mm/fault.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index ad49ae8f3967..051b32084776 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -673,11 +673,12 @@ void __init hook_debug_fault_code(int nr,
 	debug_fault_info[nr].name	= name;
 }
 
-asmlinkage int __exception do_debug_exception(unsigned long addr,
+asmlinkage int __exception do_debug_exception(unsigned long addr_if_watchpoint,
 					      unsigned int esr,
 					      struct pt_regs *regs)
 {
 	const struct fault_info *inf = debug_fault_info + DBG_ESR_EVT(esr);
+	unsigned long pc = instruction_pointer(regs);
 	struct siginfo info;
 	int rv;
 
@@ -688,19 +689,19 @@ asmlinkage int __exception do_debug_exception(unsigned long addr,
 	if (interrupts_enabled(regs))
 		trace_hardirqs_off();
 
-	if (user_mode(regs) && instruction_pointer(regs) > TASK_SIZE)
+	if (user_mode(regs) && pc > TASK_SIZE)
 		arm64_apply_bp_hardening();
 
-	if (!inf->fn(addr, esr, regs)) {
+	if (!inf->fn(addr_if_watchpoint, esr, regs)) {
 		rv = 1;
 	} else {
 		pr_alert("Unhandled debug exception: %s (0x%08x) at 0x%016lx\n",
-			 inf->name, esr, addr);
+			 inf->name, esr, pc);
 
 		info.si_signo = inf->sig;
 		info.si_errno = 0;
 		info.si_code  = inf->code;
-		info.si_addr  = (void __user *)addr;
+		info.si_addr  = (void __user *)pc;
 		arm64_notify_die("", regs, &info, 0);
 		rv = 0;
 	}
-- 
2.11.0


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

* [PATCH 4.9 BACKPORT 2/2] arm64: debug: Ensure debug handlers check triggering exception level
  2019-04-03 10:33 [PATCH 4.9/4.14 BACKPORT 1/2] arm64: debug: Don't propagate UNKNOWN FAR into si_code for debug signals Will Deacon
@ 2019-04-03 10:33 ` Will Deacon
       [not found] ` <20190405141522.584C621850@mail.kernel.org>
  1 sibling, 0 replies; 3+ messages in thread
From: Will Deacon @ 2019-04-03 10:33 UTC (permalink / raw)
  To: gregkh; +Cc: stable, linux-kernel, sashal, Will Deacon, Catalin Marinas

Commit 6bd288569b50bc89fa5513031086746968f585cb upstream.

Debug exception handlers may be called for exceptions generated both by
user and kernel code. In many cases, this is checked explicitly, but
in other cases things either happen to work by happy accident or they
go slightly wrong. For example, executing 'brk #4' from userspace will
enter the kprobes code and be ignored, but the instruction will be
retried forever in userspace instead of delivering a SIGTRAP.

Fix this issue in the most stable-friendly fashion by simply adding
explicit checks of the triggering exception level to all of our debug
exception handlers.

Cc: <stable@vger.kernel.org> # 4.9.y
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/kernel/kgdb.c           | 15 ++++++++++++---
 arch/arm64/kernel/probes/kprobes.c |  6 ++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index e017a9493b92..72a660a74ff9 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -231,24 +231,33 @@ int kgdb_arch_handle_exception(int exception_vector, int signo,
 
 static int kgdb_brk_fn(struct pt_regs *regs, unsigned int esr)
 {
+	if (user_mode(regs))
+		return DBG_HOOK_ERROR;
+
 	kgdb_handle_exception(1, SIGTRAP, 0, regs);
-	return 0;
+	return DBG_HOOK_HANDLED;
 }
 NOKPROBE_SYMBOL(kgdb_brk_fn)
 
 static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned int esr)
 {
+	if (user_mode(regs))
+		return DBG_HOOK_ERROR;
+
 	compiled_break = 1;
 	kgdb_handle_exception(1, SIGTRAP, 0, regs);
 
-	return 0;
+	return DBG_HOOK_HANDLED;
 }
 NOKPROBE_SYMBOL(kgdb_compiled_brk_fn);
 
 static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
 {
+	if (user_mode(regs))
+		return DBG_HOOK_ERROR;
+
 	kgdb_handle_exception(1, SIGTRAP, 0, regs);
-	return 0;
+	return DBG_HOOK_HANDLED;
 }
 NOKPROBE_SYMBOL(kgdb_step_brk_fn);
 
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index d2b1b624ddc3..17f647103ed7 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -450,6 +450,9 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr)
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 	int retval;
 
+	if (user_mode(regs))
+		return DBG_HOOK_ERROR;
+
 	/* return error if this is not our step */
 	retval = kprobe_ss_hit(kcb, instruction_pointer(regs));
 
@@ -466,6 +469,9 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr)
 int __kprobes
 kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
 {
+	if (user_mode(regs))
+		return DBG_HOOK_ERROR;
+
 	kprobe_handler(regs);
 	return DBG_HOOK_HANDLED;
 }
-- 
2.11.0


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

* Re: [PATCH 4.9/4.14 BACKPORT 1/2] arm64: debug: Don't propagate UNKNOWN FAR into si_code for debug signals
       [not found] ` <20190405141522.584C621850@mail.kernel.org>
@ 2019-04-05 14:16   ` Will Deacon
  0 siblings, 0 replies; 3+ messages in thread
From: Will Deacon @ 2019-04-05 14:16 UTC (permalink / raw)
  To: Sasha Levin; +Cc: gregkh, stable, linux-kernel

On Fri, Apr 05, 2019 at 02:15:21PM +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: 4.14.+
> 
> The bot has tested the following trees: v5.0.6, v4.19.33, v4.14.110.
> 
> v4.19.33: Failed to apply! Possible dependencies:
>     Unable to calculate
> 
> v4.14.110: Build OK!
> 
> How should we proceed with this patch?

I did separate backports for the various stable trees, and annotated them
all accordingly in the subject and the @stable line. I think Greg has
already taken care of them.

Will

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

end of thread, other threads:[~2019-04-05 14:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-03 10:33 [PATCH 4.9/4.14 BACKPORT 1/2] arm64: debug: Don't propagate UNKNOWN FAR into si_code for debug signals Will Deacon
2019-04-03 10:33 ` [PATCH 4.9 BACKPORT 2/2] arm64: debug: Ensure debug handlers check triggering exception level Will Deacon
     [not found] ` <20190405141522.584C621850@mail.kernel.org>
2019-04-05 14:16   ` [PATCH 4.9/4.14 BACKPORT 1/2] arm64: debug: Don't propagate UNKNOWN FAR into si_code for debug signals Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).