Re: [PATCH v7 0/4] arm64: Enable BTI for the executable as well as the interpreter

[Catalin Marinas <catalin.marinas@arm.com>]

On Thu, Jan 06, 2022 at 10:09:35AM -0600, Jeremy Linton wrote:
> On 1/6/22 05:00, Catalin Marinas wrote:
> > On Wed, Jan 05, 2022 at 04:42:01PM -0600, Jeremy Linton wrote:
> > > So, mentally i'm having a hard time balancing the hypothetical problem laid
> > > out, as it should only really exist in an environment similar to the MDWE
> > > one, since AFAIK, its possible today to just flip it back off unless MDWE
> > > stops that from happening.
> > 
> > That's a user ABI change and given that the first attempt was shown to
> > break with some combination of old loader and new main executable (or
> > the other way around), I'd rather keep things as they are.
> 
> This should only change the behavior for for binaries which conform to the
> new ABI containing the BTI note. So outside of the tiny window of things
> built with BTI, but run on !BTI hardware or older kernel+glibc, this
> shouldn't be a problem. (Unless i'm missing something) Put another way, now
> is the time to make a change, before there is a legacy BTI ecosystem we have
> to deal with.

The concern is that the loader may decide in the future to not enable
(or turn off) BTI for some reason (e.g. mixed libraries, old glibc on
BTI hardware). If we force BTI on the main executable, we'd take this
option away. Note also that it's not only glibc here, there are other
loaders.

> > AFAICT MDWX wants (one of the filters) to prevent a previously writable
> > mapping from becoming executable through mprotect(PROT_EXEC). How common
> > is mprotect(PROT_EXEC|PROT_BTI) outside of the dynamic loader? I doubt
> > it is, especially in an MDWX environment. So can we not change the
> > filter to allow PROT_EXEC|PROT_BTI? If your code is already exploitable
> > to allow random syscalls, all bets are off anyway.
> 
> I would expect JITs to be twittling EXEC|BTI but, those wouldn't be able to
> trivially run under MDWE anyway. So rarely?
> 
> Changing the filter to allow PROT_EXEC|PROT_BTI defeats the purpose because
> the hypothetical exploit would just add the BTI tags and turn BTI on as
> well. The filter, as is, also provides additional BTI protections because it
> makes it more difficult to disable BTI. Without that filter it seems likely
> someone could come up with a way to use an existing PROT_EXEC as a gadget to
> disable BTI anywhere they choose.

To me, MDWX is more about protecting against making some (previously)
writable buffers executable (either inadvertently or as a programmer
decision). It's not meant to prevent the hijacking of the instruction
flow or data corruption that ends up as an mprotect(PROT_EXEC|PROT_BTI)
call. If that's possible, the MDWX mitigation is really negligible.

That said, the programmer may learn that passing PROT_BTI avoids the
filter and start using it, though it's not trivial due to the need for
landing pads.

> So, to your point before, BTI+MDWE are complementary, the combination seems
> considerably more robust than either by itself.

Before we come up with a better solution, I find allowing PROT_BTI an
acceptable compromise. I don't think we lose much with the current
codebase.

As for a better solution, we need to track the state of a memory range
as BPF denying PROT_EXEC anywhere is a pretty big hammer. We could add a
VM_WAS_WRITE flag and, in combination with a prctl() to opt in to the
feature, prevent mprotect(PROT_EXEC) on such vmas. No need for seccomp
bpf filters.

-- 
Catalin