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

On Mon, Nov 16, 2020 at 5:36 AM Dave Martin [off-list ref] wrote:

On Sun, Nov 15, 2020 at 08:08:36AM -0600, Eric W. Biederman wrote:

quoted

Peter Collingbourne [off-list ref] writes:

quoted

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.

quoted

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