Re: [PATCH v16 6/6] arm64: expose FAR_EL1 tag bits in siginfo

[Peter Collingbourne <pcc@google.com>]

On Mon, Nov 16, 2020 at 5:36 AM Dave Martin <Dave.Martin@arm.com> wrote:
>
> On Sun, Nov 15, 2020 at 08:08:36AM -0600, Eric W. Biederman wrote:
> > Peter Collingbourne <pcc@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.
> >
> > For future architectures that implement something similar does it make
> > sense that to hide tag bits by default?
>
> I think on arm64 this comes from the fact that the tag bits information
> is not available in all scenarios.  To keep things clean, the decision
> was taken early on to just zero them all the time in si_addr to avoid
> software getting confused.  Possibly other arches do something similar,
> but that would need digging into.
>
> There seems to be debate on whether these bits are part of the address
> or not.  For si_addr I think they probably _should_ be regarded as part
> of the address in general, and arches that can always report all these
> bits in si_addr should probably do so IMHO.
>
> > I am wondering if SA_EXPOSE_TABGITS might make sense as an architecture
> > specific sa bit.
>
> Perhaps.  Peter, do you see other arches masking out bits in si_addr?

I think it's possible, since it seems likely to me that the bits would
only be able to be exposed unconditionally if the architecture with
the tag bits is new. For existing architectures that are being
extended with tag bits (e.g. RISC-V), I can see folks running into the
same problem of software getting confused by the tag bits, so they
would want to put them behind a flag, ideally the same one.

Another point in favor of making it generic is that in many cases
where you want these bits your signal handler will for the most part
not be arch-specific. In Android's case for example we have a
tombstone signal handler which is mostly generic with the
arch-specific parts hidden behind an abstraction, and the libsigchain
signal handler that needs to hide the bits if the flag is not set, but
is otherwise generic. Making it generic avoids an #ifdef around the
points where we need to refer to the flag. Please see my AOSP
userspace changes [1] for reference. As you can see there is no #ifdef
__aarch64__ required at all except for tests (there is already a
helper function for untagging addresses that has the required #ifdef),
but there are several points at which we need to refer to the flag.
With an arch-specific flag the #ifdef would become more pervasive.

Peter

[1] https://android-review.googlesource.com/id/I57f24c07c01ceb3e5b81cfc15edf559ef7dfc740

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel