Re: [PATCH] ARM: hw_breakpoint: Handle inexact watchpoint addresses

[Doug Anderson]

Hi,

On Thu, Aug 6, 2020 at 8:45 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> Hi,
>
> On Thu, Aug 6, 2020 at 8:41 AM Russell King - ARM Linux admin
> <linux@xxxxxxxxxxxxxxx> wrote:
> >
> > On Thu, Aug 06, 2020 at 08:05:10AM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Mon, Dec 2, 2019 at 8:36 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Wed, Nov 20, 2019 at 11:18 AM Will Deacon <will@xxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > > > > ---
> > > > > >
> > > > > >  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@xxxxxxxxxx>
> > > >
> > > > Thanks!  Submitted as:
> > > >
> > > > https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8944/1
> > >
> > > Oddly, I found that if I go visit that page now I see:
> > >
> > > > - - - Note 2 submitted by Russell King on 17 Jan 2020 11:16:34 (UTC) - - -
> > > > Moved to applied
> > > >
> > > > Applied to git-curr (misc branch).
> > >
> > > Yet if I go check mainline the patch is not there.  This came to my
> > > attention since we had my patch picked to the Chrome OS 4.19 tree and
> > > suddenly recently got a stable merge conflict with "ARM: 8986/1:
> > > hw_breakpoint: Don't invoke overflow handler on uaccess watchpoints".
> > >
> > > Anyone know what happened here?
> >
> > Yes.  Stephen Rothwell raised a complaint against it, which you were
> > copied with:
>
> Was a mailing list copied?  If so, do you have a lore.kernel.org link?
>  I certainly don't see the email so I can only assume it made it to
> spam.  That's unfortunate.
>
>
> > > Hi all,
> > >
> > > Commit
> > >
> > >   116375be0461 ("ARM: 8944/1: hw_breakpoint: Handle inexact watchpoint addresses")
> > >
> > > is missing a Signed-off-by from its author.
> >
> > My reply to Stephen's email was:
> >
> > > Thanks Stephen, patch dropped.
> > >
> > > It looks like Doug used his "m.disordat.com" address to submit the
> > > patch through the web interface, and there was no From: in the patch
> > > itself, so that was used as the patch author.  However, as you spotted,
> > > it was signed off using Doug's "chromium.org" address.
> > >
> > > I think it's time to make the patch system a bit more strict, checking
> > > that the submission address is mentioned in a signed-off-by tag
> > > somewhere in the commit message.
> > >
> > > Doug, the patch system does have your "chromium.org" address, if that's
> > > the one you want to use as the author, please submit using that instead.
> > > Thanks.
> > >
> > > Russell.
> >
> > Neither email got a response from you, so the patch was dropped and
> > nothing further happened.
>
> OK, I guess I'll have to rebase and resend.

Let's hope this one works:

https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=8997/1

I re-tested it and it also matches how the conflict was resolved in
the Chrome OS tree which means that (once the stable merge lands in
our tree) this conflict resolution will get a bit of testing, though
the vast majority of devices running this code have since stopped
receiving auto updates.

-Doug