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

[Will Deacon <will@kernel.org>]

On Wed, Feb 16, 2022 at 01:34:25PM +0000, Catalin Marinas wrote:
> On Tue, Feb 15, 2022 at 06:34:56PM +0000, Will Deacon wrote:
> > On Mon, Jan 24, 2022 at 03:07:00PM +0000, Mark Brown wrote:
> > > Deployments of BTI on arm64 have run into issues interacting with
> > > systemd's MemoryDenyWriteExecute feature.  Currently for dynamically
> > > linked executables the kernel will only handle architecture specific
> > > properties like BTI for the interpreter, the expectation is that the
> > > interpreter will then handle any properties on the main executable.
> > > For BTI this means remapping the executable segments PROT_EXEC |
> > > PROT_BTI.
> > > 
> > > This interacts poorly with MemoryDenyWriteExecute since that is
> > > implemented using a seccomp filter which prevents setting PROT_EXEC on
> > > already mapped memory and lacks the context to be able to detect that
> > > memory is already mapped with PROT_EXEC.  This series resolves this by
> > > handling the BTI property for both the interpreter and the main
> > > executable.
> > 
> > This appears to be a user-visible change which cannot be detected or
> > disabled from userspace. If there is code out there which does not work
> > when BTI is enabled, won't that now explode when the kernel enables it?
> > How are we supposed to handle such a regression?
> 
> If this ever happens, the only workaround is to disable BTI on the
> kernel command line. If we need a knob closer to user, we could add a
> sysctl option (as we did for the tagged address ABI, though I doubt
> people are even aware that exists). The dynamic loader doesn't do
> anything smart when deciding to map objects with PROT_BTI (like env
> variables), it simply relies on the ELF information.
> 
> I think that's very unlikely and feedback from Szabolcs in the past and
> additional testing by Mark and Jeremy was that it should be fine. The
> architecture allows interworking between BTI and non-BTI objects and on
> distros with both BTI and MDWE enabled, this is already the case: the
> main executable is mapped without PROT_BTI while the libraries will be
> mapped with PROT_BTI. The new behaviour allows both to be mapped with
> PROT_BTI, just as if MDWE was disabled.
> 
> I think the only difference would be with a BTI-unware dynamic loader
> (e.g. older distro). Here the main executable, if compiled with BTI,
> would be mapped as executable while the rest of the libraries are
> non-BTI. The interworking should be fine but we can't test everything
> since such BTI binaries would not normally be part of the distro.
> 
> If there are dodgy libraries out there that do tricks and branch into
> the middle of a function in the main executable, they will fail with
> this series but also fail if MDWE is disabled and the dynamic linker is
> BTI-aware. So this hardly counts as a use-case.
> 
> For consistency, I think whoever does the initial mapping should also
> set the correct attributes as we do for static binaries. If you think
> another knob is needed other than the cmdline, I'm fine with it.

I still think this new behaviour should be opt-in, so adding a sysctl for
that would be my preference if we proceed with this approach.

Will