| Message ID | 20181207183931.4285-9-kristina.martsenko@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 12/7/18 12:39 PM, Kristina Martsenko wrote: > From: Mark Rutland <mark.rutland@arm.com> > > When pointer authentication is in use, data/instruction pointers have a > number of PAC bits inserted into them. The number and position of these > bits depends on the configured TCR_ELx.TxSZ and whether tagging is > enabled. ARMv8.3 allows tagging to differ for instruction and data > pointers. > > For userspace debuggers to unwind the stack and/or to follow pointer > chains, they need to be able to remove the PAC bits before attempting to > use a pointer. > > This patch adds a new structure with masks describing the location of > the PAC bits in userspace instruction and data pointers (i.e. those > addressable via TTBR0), which userspace can query via PTRACE_GETREGSET. > By clearing these bits from pointers (and replacing them with the value > of bit 55), userspace can acquire the PAC-less versions. > > This new regset is exposed when the kernel is built with (user) pointer > authentication support, and the address authentication feature is > enabled. Otherwise, the regset is hidden. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > --- > arch/arm64/include/asm/pointer_auth.h | 8 ++++++++ > arch/arm64/include/uapi/asm/ptrace.h | 7 +++++++ > arch/arm64/kernel/ptrace.c | 38 +++++++++++++++++++++++++++++++++++ > include/uapi/linux/elf.h | 1 + > 4 files changed, 54 insertions(+) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 12/7/18 12:39 PM, Kristina Martsenko wrote: > When pointer authentication is in use, data/instruction pointers have a > number of PAC bits inserted into them. The number and position of these > bits depends on the configured TCR_ELx.TxSZ and whether tagging is > enabled. ARMv8.3 allows tagging to differ for instruction and data > pointers. At this point I think it's worth starting a discussion about pointer tagging, and how we can make it controllable and not mandatory. With this patch set, we are enabling 7 authentication bits: [54:48]. However, it won't be too long before someone implements support for ARMv8.2-LVA, at which point, without changes to mandatory pointer tagging, we will only have 3 authentication bits: [54:52]. This seems useless and easily brute-force-able. I assume that pointer tagging is primarily used by Android, since I'm not aware of anything else that uses it at all. Unfortunately, there is no obvious path to making this optional that does not break compatibility with Documentation/arm64/tagged-pointers.txt. I've been thinking that there ought to be some sort of global setting, akin to /proc/sys/kernel/randomize_va_space, as well as a prctl which an application could use to selectively enable TBI/TBID for an application that actually uses tagging. The global /proc setting allows the default to remain 1, which would let any application using tagging to continue working. If there are none, the sysadmin can set the default to 0. Going forward, applications could be updated to use the prctl, allowing more systems to set the default to 0. FWIW, pointer authentication continues to work when enabling TBI, but not the other way around. Thus the prctl could be used to enable TBI at any point, but if libc is built with PAuth, there's no way to turn it back off again. r~
On Sun, Dec 09, 2018 at 09:41:31AM -0600, Richard Henderson wrote: > On 12/7/18 12:39 PM, Kristina Martsenko wrote: > > When pointer authentication is in use, data/instruction pointers have a > > number of PAC bits inserted into them. The number and position of these > > bits depends on the configured TCR_ELx.TxSZ and whether tagging is > > enabled. ARMv8.3 allows tagging to differ for instruction and data > > pointers. > > At this point I think it's worth starting a discussion about pointer tagging, > and how we can make it controllable and not mandatory. > > With this patch set, we are enabling 7 authentication bits: [54:48]. > > However, it won't be too long before someone implements support for > ARMv8.2-LVA, at which point, without changes to mandatory pointer tagging, we > will only have 3 authentication bits: [54:52]. This seems useless and easily > brute-force-able. Such support is already here (about to be queued): https://lore.kernel.org/linux-arm-kernel/20181206225042.11548-1-steve.capper@arm.com/ > I assume that pointer tagging is primarily used by Android, since I'm not aware > of anything else that uses it at all. I would expect it to be enabled more widely (Linux distros), though only the support for instructions currently in the NOP space. > Unfortunately, there is no obvious path to making this optional that does not > break compatibility with Documentation/arm64/tagged-pointers.txt. There is also the ARMv8.5 MTE (memory tagging) which relies on tagged pointers. > I've been thinking that there ought to be some sort of global setting, akin to > /proc/sys/kernel/randomize_va_space, as well as a prctl which an application > could use to selectively enable TBI/TBID for an application that actually uses > tagging. An alternative would be to allow the opt-in to 52-bit VA, leaving it at 48-bit by default. However, it has the problem of changing the PAC size and not being able to return. > The global /proc setting allows the default to remain 1, which would let any > application using tagging to continue working. If there are none, the sysadmin > can set the default to 0. Going forward, applications could be updated to use > the prctl, allowing more systems to set the default to 0. > > FWIW, pointer authentication continues to work when enabling TBI, but not the > other way around. Thus the prctl could be used to enable TBI at any point, but > if libc is built with PAuth, there's no way to turn it back off again. This may work but, as you said, TBI is user ABI at this point, we can't take it away now (at the time we didn't forsee the pauth). Talking briefly with Will/Kristina/Mark, I think the best option is to make 52-bit VA default off in the kernel config. Whoever needs it enabled (enterprise systems) should be aware of the reduced PAC bits. I don't really think we have a better solution. -- Catalin
On 12/10/18 6:03 AM, Catalin Marinas wrote: >> However, it won't be too long before someone implements support for >> ARMv8.2-LVA, at which point, without changes to mandatory pointer tagging, we >> will only have 3 authentication bits: [54:52]. This seems useless and easily >> brute-force-able. > > Such support is already here (about to be queued): > > https://lore.kernel.org/linux-arm-kernel/20181206225042.11548-1-steve.capper@arm.com/ Thanks for the pointer. >> Unfortunately, there is no obvious path to making this optional that does not >> break compatibility with Documentation/arm64/tagged-pointers.txt. > > There is also the ARMv8.5 MTE (memory tagging) which relies on tagged > pointers. So it does. I hadn't read through that extension completely before. > An alternative would be to allow the opt-in to 52-bit VA, leaving it at > 48-bit by default. However, it has the problem of changing the PAC size > and not being able to return. Perhaps the opt-in should be at exec time, with ELF flags (or equivalent) on the application. Because, as you say, changing the shape of the PAC in the middle of execution is in general not possible. It isn't perfect, since old kernels won't fail to exec an application setting flags that can't be supported. And it requires tooling changes. r~
On Mon, Dec 10, 2018 at 08:22:06AM -0600, Richard Henderson wrote: > On 12/10/18 6:03 AM, Catalin Marinas wrote: > >> However, it won't be too long before someone implements support for > >> ARMv8.2-LVA, at which point, without changes to mandatory pointer tagging, we > >> will only have 3 authentication bits: [54:52]. This seems useless and easily > >> brute-force-able. > > > > Such support is already here (about to be queued): > > > > https://lore.kernel.org/linux-arm-kernel/20181206225042.11548-1-steve.capper@arm.com/ > > Thanks for the pointer. > > >> Unfortunately, there is no obvious path to making this optional that does not > >> break compatibility with Documentation/arm64/tagged-pointers.txt. > > > > There is also the ARMv8.5 MTE (memory tagging) which relies on tagged > > pointers. > > So it does. I hadn't read through that extension completely before. > > > An alternative would be to allow the opt-in to 52-bit VA, leaving it at > > 48-bit by default. However, it has the problem of changing the PAC size > > and not being able to return. > > Perhaps the opt-in should be at exec time, with ELF flags (or equivalent) on > the application. Because, as you say, changing the shape of the PAC in the > middle of execution is in general not possible. I think we'd still have a potential performance problem with that approach, since we'd end up having to context-switch TCR.T0SZ, which is permitted to be cached in a TLB and would therefore force us to introduce TLB invalidation when context-switching between tasks using 52-bit VAs and tasks using 48-bit VAs. There's a chance we could get the architecture tightened here, but it's not something we've pushed for so far and it depends on what's already been built. Will
On Mon, Dec 10, 2018 at 02:29:45PM +0000, Will Deacon wrote: > On Mon, Dec 10, 2018 at 08:22:06AM -0600, Richard Henderson wrote: > > On 12/10/18 6:03 AM, Catalin Marinas wrote: > > >> However, it won't be too long before someone implements support for > > >> ARMv8.2-LVA, at which point, without changes to mandatory pointer tagging, we > > >> will only have 3 authentication bits: [54:52]. This seems useless and easily > > >> brute-force-able. [...] > > Perhaps the opt-in should be at exec time, with ELF flags (or equivalent) on > > the application. Because, as you say, changing the shape of the PAC in the > > middle of execution is in general not possible. > > I think we'd still have a potential performance problem with that approach, > since we'd end up having to context-switch TCR.T0SZ, which is permitted to > be cached in a TLB and would therefore force us to introduce TLB > invalidation when context-switching between tasks using 52-bit VAs and tasks > using 48-bit VAs. > > There's a chance we could get the architecture tightened here, but it's > not something we've pushed for so far and it depends on what's already been > built. Just a quick summary of our internal discussion: ARMv8.3 also comes with a new bit, TCR_EL1.TBIDx, which practically disables TBI for code pointers. This bit allows us to use 11 bits for code PtrAuth with 52-bit VA. Now, the problem is that TBI for code pointers is user ABI, so we can't simply disable it. We may be able to do this with memory tagging since that's an opt-in feature (prctl) where the user is aware that the top byte of a pointer is no longer ignored. However, that's probably for a future discussion. -- Catalin
diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h index fc7ffe8e326f..5721228836c1 100644 --- a/arch/arm64/include/asm/pointer_auth.h +++ b/arch/arm64/include/asm/pointer_auth.h @@ -2,9 +2,11 @@ #ifndef __ASM_POINTER_AUTH_H #define __ASM_POINTER_AUTH_H +#include <linux/bitops.h> #include <linux/random.h> #include <asm/cpufeature.h> +#include <asm/memory.h> #include <asm/sysreg.h> #ifdef CONFIG_ARM64_PTR_AUTH @@ -57,6 +59,12 @@ static inline void ptrauth_keys_switch(struct ptrauth_keys *keys) __ptrauth_key_install(APGA, keys->apga); } +/* + * The EL0 pointer bits used by a pointer authentication code. + * This is dependent on TBI0 being enabled, or bits 63:56 would also apply. + */ +#define ptrauth_pac_mask() GENMASK(54, VA_BITS) + #define ptrauth_thread_init_user(tsk) \ do { \ struct task_struct *__ptiu_tsk = (tsk); \ diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h index a36227fdb084..c2f249bcd829 100644 --- a/arch/arm64/include/uapi/asm/ptrace.h +++ b/arch/arm64/include/uapi/asm/ptrace.h @@ -229,6 +229,13 @@ struct user_sve_header { SVE_PT_SVE_OFFSET + SVE_PT_SVE_SIZE(vq, flags) \ : SVE_PT_FPSIMD_OFFSET + SVE_PT_FPSIMD_SIZE(vq, flags)) +/* pointer authentication masks (NT_ARM_PAC_MASK) */ + +struct user_pac_mask { + __u64 data_mask; + __u64 insn_mask; +}; + #endif /* __ASSEMBLY__ */ #endif /* _UAPI__ASM_PTRACE_H */ diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 1710a2d01669..6c1f63cb6c4e 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -46,6 +46,7 @@ #include <asm/debug-monitors.h> #include <asm/fpsimd.h> #include <asm/pgtable.h> +#include <asm/pointer_auth.h> #include <asm/stacktrace.h> #include <asm/syscall.h> #include <asm/traps.h> @@ -956,6 +957,30 @@ static int sve_set(struct task_struct *target, #endif /* CONFIG_ARM64_SVE */ +#ifdef CONFIG_ARM64_PTR_AUTH +static int pac_mask_get(struct task_struct *target, + const struct user_regset *regset, + unsigned int pos, unsigned int count, + void *kbuf, void __user *ubuf) +{ + /* + * The PAC bits can differ across data and instruction pointers + * depending on TCR_EL1.TBID*, which we may make use of in future, so + * we expose separate masks. + */ + unsigned long mask = ptrauth_pac_mask(); + struct user_pac_mask uregs = { + .data_mask = mask, + .insn_mask = mask, + }; + + if (!system_supports_address_auth()) + return -EINVAL; + + return user_regset_copyout(&pos, &count, &kbuf, &ubuf, &uregs, 0, -1); +} +#endif /* CONFIG_ARM64_PTR_AUTH */ + enum aarch64_regset { REGSET_GPR, REGSET_FPR, @@ -968,6 +993,9 @@ enum aarch64_regset { #ifdef CONFIG_ARM64_SVE REGSET_SVE, #endif +#ifdef CONFIG_ARM64_PTR_AUTH + REGSET_PAC_MASK, +#endif }; static const struct user_regset aarch64_regsets[] = { @@ -1037,6 +1065,16 @@ static const struct user_regset aarch64_regsets[] = { .get_size = sve_get_size, }, #endif +#ifdef CONFIG_ARM64_PTR_AUTH + [REGSET_PAC_MASK] = { + .core_note_type = NT_ARM_PAC_MASK, + .n = sizeof(struct user_pac_mask) / sizeof(u64), + .size = sizeof(u64), + .align = sizeof(u64), + .get = pac_mask_get, + /* this cannot be set dynamically */ + }, +#endif }; static const struct user_regset_view user_aarch64_view = { diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index c5358e0ae7c5..3f23273d690c 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -420,6 +420,7 @@ typedef struct elf64_shdr { #define NT_ARM_HW_WATCH 0x403 /* ARM hardware watchpoint registers */ #define NT_ARM_SYSTEM_CALL 0x404 /* ARM system call number */ #define NT_ARM_SVE 0x405 /* ARM Scalable Vector Extension registers */ +#define NT_ARM_PAC_MASK 0x406 /* ARM pointer authentication code masks */ #define NT_ARC_V2 0x600 /* ARCv2 accumulator/extra registers */ #define NT_VMCOREDD 0x700 /* Vmcore Device Dump Note */ #define NT_MIPS_DSP 0x800 /* MIPS DSP ASE registers */