Skip to content

Commit d11a698

Browse files
tnovakwilldeacon
authored andcommitted
hw_breakpoint: fix single-stepping when using bpf_overflow_handler
Arm platforms use is_default_overflow_handler() to determine if the hw_breakpoint code should single-step over the breakpoint trigger or let the custom handler deal with it. Since bpf_overflow_handler() currently isn't recognized as a default handler, attaching a BPF program to a PERF_TYPE_BREAKPOINT event causes it to keep firing (the instruction triggering the data abort exception is never skipped). For example: # bpftrace -e 'watchpoint:0x10000:4:w { print("hit") }' -c ./test Attaching 1 probe... hit hit [...] ^C (./test performs a single 4-byte store to 0x10000) This patch replaces the check with uses_default_overflow_handler(), which accounts for the bpf_overflow_handler() case by also testing if one of the perf_event_output functions gets invoked indirectly, via orig_default_handler. Signed-off-by: Tomislav Novak <tnovak@meta.com> Tested-by: Samuel Gosselin <sgosselin@google.com> # arm64 Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> Acked-by: Alexei Starovoitov <ast@kernel.org> Link: https://lore.kernel.org/linux-arm-kernel/20220923203644.2731604-1-tnovak@fb.com/ Link: https://lore.kernel.org/r/20230605191923.1219974-1-tnovak@meta.com Signed-off-by: Will Deacon <will@kernel.org>
1 parent f4e2bd9 commit d11a698

File tree

3 files changed

+25
-9
lines changed

3 files changed

+25
-9
lines changed

arch/arm/kernel/hw_breakpoint.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
626626
hw->address &= ~alignment_mask;
627627
hw->ctrl.len <<= offset;
628628

629-
if (is_default_overflow_handler(bp)) {
629+
if (uses_default_overflow_handler(bp)) {
630630
/*
631631
* Mismatch breakpoints are required for single-stepping
632632
* breakpoints.
@@ -798,7 +798,7 @@ static void watchpoint_handler(unsigned long addr, unsigned int fsr,
798798
* Otherwise, insert a temporary mismatch breakpoint so that
799799
* we can single-step over the watchpoint trigger.
800800
*/
801-
if (!is_default_overflow_handler(wp))
801+
if (!uses_default_overflow_handler(wp))
802802
continue;
803803
step:
804804
enable_single_step(wp, instruction_pointer(regs));
@@ -811,7 +811,7 @@ static void watchpoint_handler(unsigned long addr, unsigned int fsr,
811811
info->trigger = addr;
812812
pr_debug("watchpoint fired: address = 0x%x\n", info->trigger);
813813
perf_bp_event(wp, regs);
814-
if (is_default_overflow_handler(wp))
814+
if (uses_default_overflow_handler(wp))
815815
enable_single_step(wp, instruction_pointer(regs));
816816
}
817817

@@ -886,7 +886,7 @@ static void breakpoint_handler(unsigned long unknown, struct pt_regs *regs)
886886
info->trigger = addr;
887887
pr_debug("breakpoint fired: address = 0x%x\n", addr);
888888
perf_bp_event(bp, regs);
889-
if (is_default_overflow_handler(bp))
889+
if (uses_default_overflow_handler(bp))
890890
enable_single_step(bp, addr);
891891
goto unlock;
892892
}

arch/arm64/kernel/hw_breakpoint.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ static int breakpoint_handler(unsigned long unused, unsigned long esr,
654654
perf_bp_event(bp, regs);
655655

656656
/* Do we need to handle the stepping? */
657-
if (is_default_overflow_handler(bp))
657+
if (uses_default_overflow_handler(bp))
658658
step = 1;
659659
unlock:
660660
rcu_read_unlock();
@@ -733,7 +733,7 @@ static u64 get_distance_from_watchpoint(unsigned long addr, u64 val,
733733
static int watchpoint_report(struct perf_event *wp, unsigned long addr,
734734
struct pt_regs *regs)
735735
{
736-
int step = is_default_overflow_handler(wp);
736+
int step = uses_default_overflow_handler(wp);
737737
struct arch_hw_breakpoint *info = counter_arch_bp(wp);
738738

739739
info->trigger = addr;

include/linux/perf_event.h

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,15 +1316,31 @@ extern int perf_event_output(struct perf_event *event,
13161316
struct pt_regs *regs);
13171317

13181318
static inline bool
1319-
is_default_overflow_handler(struct perf_event *event)
1319+
__is_default_overflow_handler(perf_overflow_handler_t overflow_handler)
13201320
{
1321-
if (likely(event->overflow_handler == perf_event_output_forward))
1321+
if (likely(overflow_handler == perf_event_output_forward))
13221322
return true;
1323-
if (unlikely(event->overflow_handler == perf_event_output_backward))
1323+
if (unlikely(overflow_handler == perf_event_output_backward))
13241324
return true;
13251325
return false;
13261326
}
13271327

1328+
#define is_default_overflow_handler(event) \
1329+
__is_default_overflow_handler((event)->overflow_handler)
1330+
1331+
#ifdef CONFIG_BPF_SYSCALL
1332+
static inline bool uses_default_overflow_handler(struct perf_event *event)
1333+
{
1334+
if (likely(is_default_overflow_handler(event)))
1335+
return true;
1336+
1337+
return __is_default_overflow_handler(event->orig_overflow_handler);
1338+
}
1339+
#else
1340+
#define uses_default_overflow_handler(event) \
1341+
is_default_overflow_handler(event)
1342+
#endif
1343+
13281344
extern void
13291345
perf_event_header__init_id(struct perf_event_header *header,
13301346
struct perf_sample_data *data,

0 commit comments

Comments
 (0)