| From 4631057708d49876ddcde649fcc8591014dcf2b3 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Fri, 29 May 2020 14:12:18 +0100 |
| Subject: arm64: hw_breakpoint: Don't invoke overflow handler on uaccess |
| watchpoints |
| |
| From: Will Deacon <will@kernel.org> |
| |
| [ Upstream commit 24ebec25fb270100e252b19c288e21bd7d8cc7f7 ] |
| |
| Unprivileged memory accesses generated by the so-called "translated" |
| instructions (e.g. STTR) at EL1 can cause EL0 watchpoints to fire |
| unexpectedly if kernel debugging is enabled. In such cases, the |
| hw_breakpoint logic will invoke the user overflow handler which will |
| typically raise a SIGTRAP back to the current task. This is futile when |
| returning back to the kernel because (a) the signal won't have been |
| delivered and (b) userspace can't handle the thing anyway. |
| |
| Avoid invoking the user overflow handler for watchpoints triggered by |
| kernel uaccess routines, and instead single-step over the faulting |
| instruction as we would if no overflow handler had been installed. |
| |
| (Fixes tag identifies the introduction of unprivileged memory accesses, |
| which exposed this latent bug in the hw_breakpoint code) |
| |
| Cc: Catalin Marinas <catalin.marinas@arm.com> |
| Cc: James Morse <james.morse@arm.com> |
| Fixes: 57f4959bad0a ("arm64: kernel: Add support for User Access Override") |
| Reported-by: Luis Machado <luis.machado@linaro.org> |
| Signed-off-by: Will Deacon <will@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| arch/arm64/kernel/hw_breakpoint.c | 44 ++++++++++++++++++------------- |
| 1 file changed, 26 insertions(+), 18 deletions(-) |
| |
| diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c |
| index 95697a9c12451..6e96cea99a4ec 100644 |
| --- a/arch/arm64/kernel/hw_breakpoint.c |
| +++ b/arch/arm64/kernel/hw_breakpoint.c |
| @@ -738,6 +738,27 @@ static u64 get_distance_from_watchpoint(unsigned long addr, u64 val, |
| return 0; |
| } |
| |
| +static int watchpoint_report(struct perf_event *wp, unsigned long addr, |
| + struct pt_regs *regs) |
| +{ |
| + int step = is_default_overflow_handler(wp); |
| + struct arch_hw_breakpoint *info = counter_arch_bp(wp); |
| + |
| + info->trigger = addr; |
| + |
| + /* |
| + * If we triggered a user watchpoint from a uaccess routine, then |
| + * handle the stepping ourselves since userspace really can't help |
| + * us with this. |
| + */ |
| + if (!user_mode(regs) && info->ctrl.privilege == AARCH64_BREAKPOINT_EL0) |
| + step = 1; |
| + else |
| + perf_bp_event(wp, regs); |
| + |
| + return step; |
| +} |
| + |
| static int watchpoint_handler(unsigned long addr, unsigned int esr, |
| struct pt_regs *regs) |
| { |
| @@ -747,7 +768,6 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr, |
| u64 val; |
| struct perf_event *wp, **slots; |
| struct debug_info *debug_info; |
| - struct arch_hw_breakpoint *info; |
| struct arch_hw_breakpoint_ctrl ctrl; |
| |
| slots = this_cpu_ptr(wp_on_reg); |
| @@ -785,25 +805,13 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr, |
| if (dist != 0) |
| continue; |
| |
| - info = counter_arch_bp(wp); |
| - info->trigger = addr; |
| - perf_bp_event(wp, regs); |
| - |
| - /* Do we need to handle the stepping? */ |
| - if (is_default_overflow_handler(wp)) |
| - step = 1; |
| + step = watchpoint_report(wp, addr, regs); |
| } |
| - if (min_dist > 0 && min_dist != -1) { |
| - /* No exact match found. */ |
| - wp = slots[closest_match]; |
| - info = counter_arch_bp(wp); |
| - info->trigger = addr; |
| - perf_bp_event(wp, regs); |
| |
| - /* Do we need to handle the stepping? */ |
| - if (is_default_overflow_handler(wp)) |
| - step = 1; |
| - } |
| + /* No exact match found? */ |
| + if (min_dist > 0 && min_dist != -1) |
| + step = watchpoint_report(slots[closest_match], addr, regs); |
| + |
| rcu_read_unlock(); |
| |
| if (!step) |
| -- |
| 2.25.1 |
| |