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

On Fri, Nov 20, 2020 at 11:24 AM Eric W. Biederman
[off-list ref] wrote:

Peter Collingbourne [off-list ref] writes:

quoted

On Fri, Nov 20, 2020 at 9:44 AM 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, 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" <redacted>

Thanks for the review.

quoted

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.

quoted

quoted

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