Skip to content

Commit

Permalink
arm64: hw_breakpoint: Don't invoke overflow handler on uaccess watchp…
Browse files Browse the repository at this point in the history
…oints

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 <[email protected]>
Cc: James Morse <[email protected]>
Fixes: 57f4959 ("arm64: kernel: Add support for User Access Override")
Reported-by: Luis Machado <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
  • Loading branch information
willdeacon committed Jun 18, 2020
1 parent bf508ec commit 24ebec2
Showing 1 changed file with 26 additions and 18 deletions.
44 changes: 26 additions & 18 deletions arch/arm64/kernel/hw_breakpoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,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)
{
Expand All @@ -739,7 +760,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);
Expand Down Expand Up @@ -777,25 +797,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)
Expand Down

0 comments on commit 24ebec2

Please sign in to comment.