linux-kernel - Re: [PATCH] ARM: hw_breakpoint: Handle inexact watchpoint addresses

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]

Date:   Mon, 6 Jan 2020 17:40:46 +0000
From:   Will Deacon <will@...nel.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Mark Rutland <mark.rutland@....com>,
        Pavel Labath <labath@...gle.com>,
        Pratyush Anand <panand@...hat.com>,
        Matthias Kaehlcke <mka@...omium.org>,
        Kazuhiro Inaba <kinaba@...gle.com>,
        Russell King <linux@...linux.org.uk>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARM: hw_breakpoint: Handle inexact watchpoint addresses

On Mon, Dec 02, 2019 at 08:36:19AM -0800, Doug Anderson wrote:
> On Wed, Nov 20, 2019 at 11:18 AM Will Deacon <will@...nel.org> wrote:
> >
> > On Sat, Oct 19, 2019 at 11:12:26AM -0700, Douglas Anderson wrote:
> > > This is commit fdfeff0f9e3d ("arm64: hw_breakpoint: Handle inexact
> > > watchpoint addresses") but ported to arm32, which has the same
> > > problem.
> > >
> > > This problem was found by Android CTS tests, notably the
> > > "watchpoint_imprecise" test [1].  I tested locally against a copycat
> > > (simplified) version of the test though.
> > >
> > > [1] https://android.googlesource.com/platform/bionic/+/master/tests/sys_ptrace_test.cpp
> > >
> > > Signed-off-by: Douglas Anderson <dianders@...omium.org>
> > > ---
> > >
> > >  arch/arm/kernel/hw_breakpoint.c | 96 ++++++++++++++++++++++++---------
> > >  1 file changed, 70 insertions(+), 26 deletions(-)
> >
> > Sorry for taking so long to look at this. After wrapping my head around the
> > logic again
> 
> Yeah.  It was a little weird and (unfortunately) arbitrarily different
> in some places compared to the arm64 code.
> 
> 
> > I think it looks fine, so please put it into the patch system
> > with my Ack:
> >
> > Acked-by: Will Deacon <will@...nel.org>
> 
> Thanks!  Submitted as:
> 
> https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8944/1
> 
> 
> > One interesting difference between the implementation here and the arm64
> > code is that I think if you have multiple watchpoints, all of which fire
> > with a distance != 0, then arm32 will actually report them all whereas
> > you'd only get one on arm64.
> 
> Are you sure about that?  The "/* No exact match found. */" code is
> outside the for loop so it should only be able to trigger for exactly
> one breakpoint, no?

I didn't test it, but I think that we'll convert the first watchpoint into a
mismatch breakpoint on arm32 and then when we resume execution, we'll hit
the subsequent watchpoint and so on until we actually manage to "step" the
instruction. On arm64, we'll use hardware step directly and therefore
disable all watchpoints prior to performing the step.

Will