Re: [PATCH v2] arm64: allow TCR_EL1.TBID0 to be configured
From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2021-07-27 16:53:14
Also in:
linux-api
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.
quoted hunk ↗ jump to hunk
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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel