[PATCH v20] arm64: expose FAR_EL1 tag bits in siginfo

[Peter Collingbourne]

On Fri, Nov 20, 2020 at 11:24 AM Eric W. Biederman
<ebiederm at xmission.com> wrote:
>
> Peter Collingbourne <pcc at google.com> writes:
>
> > On Fri, Nov 20, 2020 at 9:44 AM Eric W. Biederman <ebiederm at xmission.com> wrote:
> >>
> >> Peter Collingbourne <pcc at google.com> writes:
> >>
> >> > The kernel currently clears the tag bits (i.e. bits 56-63) in the fault
> >> > address exposed via siginfo.si_addr and sigcontext.fault_address. However,
> >> > the tag bits may be needed by tools in order to accurately diagnose
> >> > memory errors, such as HWASan [1] or future tools based on the Memory
> >> > Tagging Extension (MTE).
> >> >
> >> > We should not stop clearing these bits in the existing fault address
> >> > fields, because there may be existing userspace applications that are
> >> > expecting the tag bits to be cleared. Instead, introduce a flag in
> >> > sigaction.sa_flags, SA_EXPOSE_TAGBITS, and only expose the tag bits
> >> > there if the signal handler has this flag set.
> >> >
> >> > [1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html
> >>
> >> For the generic bits:
> >> Acked-by: "Eric W. Biederman" <ebiederm at xmission.com>
> >
> > Thanks for the review.
> >
> >> Some of the arm bits look wrong.  There are a couple of cases where
> >> it looks like you are deliberately passing an untagged address into
> >> functions that normally take tagged addresses.
> >>
> >> It might be a good idea to have a type distinction between the two.
> >> Perhaps "(void __user *)" vs "(unsigned long)" so that accidentally
> >> using the wrong one generates a type error.
> >>
> >> I don't think I am really qualified to review all of the arm details,
> >> and I certainly don't want to be in the middle of any arm bugs this
> >> code might introduce.  If you will split out the generic bits of this
> >> patch I will take it.  The this whole thing can be merged into the arm
> >
> > Okay, I'll do that.
>
> Thank you.

Sent out v21 with the generic parts split out.

> >> tree and you can ensure the arm bits are correct.
> >
> > So you want Catalin to merge your signal-for-v5.11 branch with the
> > generic patches into the arm64 tree and then add the arm64-specific
> > patch on top? Works for me.
>
> I think that is what we discussed.  Unless he has objections I would
> prefer that.  I based the branch on -rc3 in hopes that I would
> not cause problems for his process.
>
> In the cases where I was confused you probably want comments describing
> why the tag bits are being cleared.  It may be obvious when you know the
> architecture intimately but it certainly was not for me.  I certainly
> don't know the details of arm64 well enough to understand the
> architecture specific nuances.

I think that makes sense. I've added comments to those two cases.

Peter