Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext

From: Peter Collingbourne <pcc@google.com>
To: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Kostya Serebryany <kcc@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH v3] arm64: Expose original FAR_EL1 value in sigcontext
Date: Wed, 29 Apr 2020 14:42:01 -0700	[thread overview]
Message-ID: <CAMn1gO7Xh2nnZhcjpwA3Y-8C5ePPOwHS2ORMASuQA4UaJgrFfg@mail.gmail.com> (raw)
In-Reply-To: <20200429210826.GA8604@willie-the-truck>

On Wed, Apr 29, 2020 at 2:08 PM Will Deacon <will@kernel.org> wrote:
>
> On Fri, Mar 27, 2020 at 12:19:15PM -0700, Peter Collingbourne wrote:
> > 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 far_context in
> > sigcontext (similar to the existing esr_context), and store the original
> > value of FAR_EL1 (including the tag bits) there.
> >
> > [1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html
> >
> > Signed-off-by: Peter Collingbourne <pcc@google.com>
> > ---
> > v3:
> > - add documentation to tagged-pointers.rst
> > - update comments in sigcontext.h
>
> Hmm, although the code looks fine, why don't we just expose the tag in the
> new field, rather than duplicate the address information? I'm nervous about
> exposing privileged registers directly to userspace.

I have no strong opinion on whether this should just contain the tag or not.

> Also, Catalin, could you elaborate on the MTE use-case please? The
> architecture says that FAR_EL1[63:60] are UNKNOWN on a synchronous tag
> check fault, so we'd have to *avoid* exposing them in that case!

The basic use case is to allow a signal handler to identify which
allocation was accessed improperly in order to provide better
diagnostics. For example, if you have granules tagged 1,2,3
consecutively and see an access with pointer tag 1 on the granule
tagged 2, you can tell that it was probably a buffer overflow from the
1 granule and you can report that to the user.

It seems unfortunate that bits 63:60 are now UNKNOWN on synchronous
tag check faults. It seems to be a recent change to the specification.
I can think of a number of use cases for bits 63:60 after a
synchronous tag check fault -- for example, an allocator could store
an additional set of random bits there, and later use them to
determine with more accuracy which allocation was used after free or
out of bounds. That being said, we aren't planning to do this in the
initial version of the MTE-aware scudo allocator.

If there is no chance of changing the architecture at this point, I
will send a v4 with the bits masked out when handling a tag check
fault.

Peter

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