Re: [PATCH 2/2] arm64: allow TCR_EL1.TBID0 to be configured

[Peter Collingbourne <pcc@google.com>]

On Wed, Nov 25, 2020 at 6:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 11/24/2020 11:18, Peter Collingbourne wrote:
> > On Tue, Nov 24, 2020 at 10:47 AM Catalin Marinas
> > <catalin.marinas@arm.com> wrote:
> > > On Sat, Nov 21, 2020 at 01:59:03AM -0800, Peter Collingbourne wrote:
> > > > Introduce a Kconfig option that controls whether TCR_EL1.TBID0 is
> > > > set at boot time.
> > > >
> > > > Setting TCR_EL1.TBID0 increases the number of signature bits used by
> > > > the pointer authentication instructions for instruction addresses by 8,
> > > > which improves the security of pointer authentication, but it also has
> > > > the consequence of changing the operation of the branch instructions
> > > > so that they no longer ignore the top byte of the target address but
> > > > instead fault if they are non-zero. Since this is a change to the
> > > > userspace ABI the option defaults to off.
> > > >
> > > > Signed-off-by: Peter Collingbourne <pcc@google.com>
> > > > Link: https://linux-review.googlesource.com/id/Ife724ad708142bc475f42e8c1d9609124994bbbd
> > > > ---
> > > > This is more of an RFC. An open question is how to expose this.
> > > > Having it be a build-time flag is probably the simplest option
> > > > but I guess it could also be a boot flag. Since it involves an
> > > > ABI change we may also want a prctl() so that userspace can
> > > > figure out which mode it is in.
> > > >
> > > > I think we should try to avoid it being a per-task property
> > > > so that we don't need to swap yet another system register on
> > > > task switch.
> > >
> > > Having it changed per task at run-time is problematic as this bit may be
> > > cached in the TLB, so it would require a synchronisation across all CPUs
> > > followed by TLBI. It's not even clear to me from the ARM ARM whether
> > > this bit is tagged by ASID, which, if not, would make a per-process
> > > setting impossible.
> > >
> > > So this leaves us with a cmdline option. If we are confident that no
> > > software makes use of tagged instruction pointers, we could have it
> > > default on.
> >
> > I would be concerned about turning it on by default because tagged
> > instruction pointers may end up being used unintentionally as a result
> > of emergent behavior. For example, when booting Android under FVP with
> > this enabled I discovered that SwiftShader would crash when entering
> > JITed code because the code was being stored at a tagged address
> > (tagged because it had been allocated using Scudo's MTE allocator).
> > Arguably software shouldn't be storing executable code in memory owned
> > by the allocator as this would require changing the permissions of
> > memory that it doesn't own, but from the kernel's perspective it is
> > valid.
>
> it might be still possible to change this abi on linux by
> default, but i don't know what's the right way to manage the
> abi transition. i will have to think about it.
>
> i dont think PROT_MTE|PROT_EXEC is architecturally well
> supported (e.g. to have different colored functions or
> similar, pc relative addressing doesn't work right).
>
> (tbi for instruction pointers is unlikely to be useful, but
> extra 8 bits for pac is useful. i think we should be able to
> move to an abi that is compatible with either setting.)
>
> (i think supporting mmap/munmap/madvise/mprotect on malloced
> memory is problematic in general not just with heap tagging
> so it would be nice to fix whatever jit that marks malloced
> memory as executable.)

Hi Szabolcs,

Did you get a chance to think about this?

I propose that we start with a command line option that defaults to
off. If/when any ABI transition happens we can change the default.

Peter