linux-kernel - Re: [PATCH] hw_breakpoint: fix single-stepping when using bpf_overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y3OrsU9M+X2UUcRG@arm.com>
Date:   Tue, 15 Nov 2022 15:09:37 +0000
From:   Catalin Marinas <catalin.marinas@....com>
To:     Tomislav Novak <tnovak@...com>
Cc:     Alexei Starovoitov <ast@...nel.org>, Will Deacon <will@...nel.org>,
        bpf@...r.kernel.org, linux-perf-users@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Samuel Gosselin <sgosselin@...com>,
        Mark Rutland <mark.rutland@....com>,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH] hw_breakpoint: fix single-stepping when using
 bpf_overflow_handler

On Fri, Sep 23, 2022 at 01:36:45PM -0700, Tomislav Novak wrote:
> On ARM platforms is_default_overflow_handler() is used to determine if
> hw_breakpoint code should single-step over the watchpoint trigger or
> let the custom handler deal with it.
> 
> Attaching a BPF program to a watchpoint replaces the handler with
> bpf_overflow_handler, which isn't recognized as a default handler so we
> never step over the instruction triggering the data abort exception (the
> watchpoint keeps firing):
> 
>   # bpftrace -e 'watchpoint:0x10000000:4:w { printf("hit\n"); }' ./wp_test
>   Attaching 1 probe...
>   hit
>   hit
>   hit
>   [...]
> 
> (wp_test performs a single 4-byte store to address 0x10000000)
> 
> This patch replaces the check with uses_default_overflow_handler(), which
> accounts for the bpf_overflow_handler() case by also testing if the handler
> invokes one of the perf_event_output functions via orig_default_handler.
> 
> Signed-off-by: Tomislav Novak <tnovak@...com>
> Tested-by: Samuel Gosselin <sgosselin@...com> # arm64
> ---
>  arch/arm/kernel/hw_breakpoint.c   |  8 ++++----
>  arch/arm64/kernel/hw_breakpoint.c |  4 ++--
>  include/linux/perf_event.h        | 22 +++++++++++++++++++---
>  3 files changed, 25 insertions(+), 9 deletions(-)

It looks like this slipped through the cracks. I'm fine with the patch
but could you split the arm and arm64 parts in separate patches? Unless
rmk acks it and we can take the patch through the arm64 (or perf) tree.

-- 
Catalin