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

[Szabolcs Nagy <szabolcs.nagy@arm.com>]

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.)