[PATCH v2] arm64: allow TCR_EL1.TBID0 to be configured
Catalin Marinas
catalin.marinas at arm.com
Tue Jul 27 09:51:09 PDT 2021
Hi Peter,
On Mon, Jun 21, 2021 at 10:12:04PM -0700, Peter Collingbourne wrote:
> Introduce a command line flag that controls whether TCR_EL1.TBID0
> is set at boot time. Since this is a change to the userspace ABI the
> option defaults to off for now, although it seems likely that we'll
> be able to change the default at some future point.
>
> 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.
I'm a bit uneasy about the ABI change and not so keen on constraining
the ABI through the kernel command line. Ideally we should make this an
opt-in per application (prctl()) but that has some aspects to address
first: (a) this bit is permitted to be cached in the TLB so we'd need
some TLBI when setting it (and a clarification in the specs that it is
tagged by ASID/VMID, probably fine) and (b) we'd need to context-switch
TCR_EL1, with a small performance penalty (I don't think it's
significant but worth testing).
Unfortunately, we can't turn TBID0 off dynamically when we detect a
tagged PC since this would break authentication of already encoded
pointers.
Prior to hwasan and MTE, I doubt anyone would have noticed this change
but once malloc() and friends started returning tagged pointers,
programs executing code from malloc()'ed regions would fall apart with
TBID0. I think it's a bit of stretch to argue that it's hwasan and MTE
causing the application breakage rather than a user-kernel ABI change,
since that's already working currently (though such programs should be
re-written).
Longer term, I'd like the TBID0 to be the default but transitioning
without breaking the user is tricky, hence my first option would be
per-application with an opt-in.
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 871c82ab0a30..9ee32afe121c 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -529,11 +529,23 @@ static int __kprobes do_page_fault(unsigned long far, unsigned int esr,
> vm_fault_t fault;
> unsigned long vm_flags;
> unsigned int mm_flags = FAULT_FLAG_DEFAULT;
> - unsigned long addr = untagged_addr(far);
> + unsigned long addr;
>
> if (kprobe_page_fault(regs, esr))
> return 0;
>
> + /*
> + * If TBID0 is set then we may get an IABT with a tagged address here as
> + * a result of branching to a tagged address. In this case we want to
> + * avoid untagging the address, let the VMA lookup fail and get a
> + * SIGSEGV. Leaving the address as is will also work if TBID0 is clear
> + * or unsupported because the tag bits of FAR_EL1 will be clear.
> + */
> + if (is_el0_instruction_abort(esr))
> + addr = far;
> + else
> + addr = untagged_addr(far);
Should this also check for tcr_tbid0_enabled() before deciding not to
untag the address?
--
Catalin
More information about the linux-arm-kernel
mailing list