Re: [PATCH v14 8/8] arm64: expose FAR_EL1 tag bits in siginfo

Peter Collingbourne [off-list ref] writes:

On Mon, Nov 9, 2020 at 5:13 PM Eric W. Biederman [off-list ref] 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, create a new pair of
fields in siginfo._sigfault, and store the tag bits of FAR_EL1 there,
together with a mask specifying which bits are valid.

A flag is added to si_faultflags to allow userspace to determine whether
the values in the fields are valid.

I think I am missing some things:

Today it is documented that the tag bits are cleared, and so we can't
use the highbits to hold the tag bits by default.

Why do you need to deliver which tag bits are valid?  That feels like an
implementation detail that is needed to setup the tag bits.  It feels
like it would be constant per process.  So I don't understand why the
siginfo needs to report information the process should already have.

It isn't constant as it may vary depending on the specific type of
fault being delivered. For example on arm64 the architecture only
provides us with bits 56-59 of the tag for tag check faults, while all
other data aborts also provide bits 60-63. Now although the user
program may distinguish the two cases by checking the si_code, we
would also like to allow future architecture extensions to provide
bits 60-63 for tag check faults as well and allow distinguishing
between "bits 60-63 were zero" and "bits 60-63 were unknown" (which is
important when providing error reports).

Does that mean that bits 60-63 are effectively unusable as tag bits
if the tag check fault won't report them?

If you can use bits 60-63 as tag bits how does that work if they aren't
reported?

I suppose that you could imagine that, if the "bits 60-63 available"
extension ever comes to pass, it could be exposed via a bit in
getauxval(AT_HWCAP2) (or maybe something like AT_HWCAP3 depending on
how long it takes to arrive) and that would provide a way for
userspace to know which bits are valid. But it seems like allowing
this to vary per signal is relatively cheap, allows the information to
be made available trivially architecture independently and keeps our
options open for the future (because we don't know if some future
architecture will actually make this a per-signal attribute).

That being said, maybe we could solve this problem at the point at
which we do encounter such an architecture though.

My goal would be to extend things to the minimum extent necessary to
handle today's reality well.  Usually architectures evolve in
unanticipated directions so simple and straight forward usually wins for
handling future evolution.  As there is simply less old baggage to carry
around.

However I don't understand if reporting the valid bits on a signal by
signal basis has a real advantage today or not.  If it helps today we
will find room for the field.

quoted

Want prevents adding a sigaction sa_flag SA_EXPOSE_TABITS that when set
causes the high bits to be set, and when clear (the default) will have
the signal delivery code clear those bits.

That should be enough for code that wants the tag bits to ask for them.
As userspace would need to be updated to get the new bits

Even if you have chained handlers.  The chaining mechanism would need to
be updated and it could call the aware handlers first then clear the tag
bits and call the rest of the handlers.

It feels like always passing the tag bits in the address and then
clearing them in the copy to userspace if the signal handler is
not ready for them would be easier to maintain.

I think that approach might work. Although it may make life harder for
callers of ptrace(PTRACE_SETSIGINFO) since they may need to know the
value of the bit in order to prepare a correct siginfo structure, if
we can reasonably expect them to always be delivering an exact copy of
a signal that was received before then maybe that is okay.

I think we can reasonably expect callers of PTRACE_SETSIGINFO to be able
to either deal the full reality of what is going on, or to only generate
signals that they fully understand.

Other than the use by CRIU it is a debugging facility and it is not
expected for ordinary usage.  The non-CRIU use case would really seem to
be what happens if I inject arbitrary signal X into process Y.  For that
you need the ability to inject an arbitrary signal.

My real sense with PTRACE_SETSIGINFO is that if we wind up with a
regression we can deal with it then.

Assuming that this is an alternative to introducing
si_addr_tag_bits_mask, the userspace code would need to use the flag
bit support detection protocol for SA_EXPOSE_TAGBITS in order to be
able to distinguish between "no bits valid" and "some bits valid", and
then use an architecture-specific mechanism to determine exactly which
bits are valid. Is that okay for a generic feature?

Unless I am mistaken setting the bits is already architecture specific
so having some architecture specific code in there should not be a big
problem.

But I really don't understand the arm case well enough to know if we can
get away without si_addr_tag_bits_mask, and in turn without the flags
field that indicates the si_addr_tag_bits_mask is present.

So I am asking questions so I can understand just what we get from
si_addr_tag_bits_mask.

Eric