| Message ID | 20181005084754.20950-8-kristina.martsenko@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,01/17] arm64: add pointer authentication register bits | expand |
Hi Kristina, On 05/10/18 09:47, Kristina Martsenko wrote: > From: Mark Rutland <mark.rutland@arm.com> > > This patch adds basic support for pointer authentication, allowing > userspace to make use of APIAKey. The kernel maintains an APIAKey value > for each process (shared by all threads within), which is initialised to > a random value at exec() time. > > To describe that address authentication instructions are available, the > ID_AA64ISAR0.{APA,API} fields are exposed to userspace. A new hwcap, > APIA, is added to describe that the kernel manages APIAKey. > > Instructions using other keys (APIBKey, APDAKey, APDBKey) are disabled, > and will behave as NOPs. These may be made use of in future patches. > > No support is added for the generic key (APGAKey), though this cannot be > trapped or made to behave as a NOP. Its presence is not advertised with > a hwcap. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > [kristina: init keys in arch_bprm_mm_init; add AA64ISAR1.API HWCAP_CAP; use sysreg_clear_set] > Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com> > Tested-by: Adam Wallis <awallis@codeaurora.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index 0dd171c7d71e..3157685aa56a 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -1040,6 +1040,11 @@ static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused) > } > > #ifdef CONFIG_ARM64_PTR_AUTH > +static void cpu_enable_address_auth(struct arm64_cpu_capabilities const *cap) > +{ > + sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA); > +} > + > static bool has_address_auth(const struct arm64_cpu_capabilities *entry, > int __unused) > { > @@ -1267,6 +1272,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > .capability = ARM64_HAS_ADDRESS_AUTH, > .type = ARM64_CPUCAP_SYSTEM_FEATURE, > .matches = has_address_auth, > + .cpu_enable = cpu_enable_address_auth, > }, > #endif /* CONFIG_ARM64_PTR_AUTH */ > {}, > @@ -1314,6 +1320,10 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = { > #ifdef CONFIG_ARM64_SVE > HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_SVE_SHIFT, FTR_UNSIGNED, ID_AA64PFR0_SVE, CAP_HWCAP, HWCAP_SVE), > #endif > +#ifdef CONFIG_ARM64_PTR_AUTH > + HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_APA_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_APIA), > + HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_API_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_APIA), This is a bit problematic. If all the CPUs have just the IMPDEF algorithm available, we could fail to match the first entry (APA) for a late secondary CPU and thus thus fail the CPU from booting. I guess we need a custom entry which reuses the has_address_auth() as the matches(). Rest looks fine to me. Suzuki
On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote: > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h > new file mode 100644 > index 000000000000..2aefedc31d9e > --- /dev/null > +++ b/arch/arm64/include/asm/pointer_auth.h > @@ -0,0 +1,63 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#ifndef __ASM_POINTER_AUTH_H > +#define __ASM_POINTER_AUTH_H > + > +#include <linux/random.h> > + > +#include <asm/cpufeature.h> > +#include <asm/sysreg.h> > + > +#ifdef CONFIG_ARM64_PTR_AUTH > +/* > + * Each key is a 128-bit quantity which is split across a pair of 64-bit > + * registers (Lo and Hi). > + */ > +struct ptrauth_key { > + unsigned long lo, hi; > +}; > + > +/* > + * We give each process its own instruction A key (APIAKey), which is shared by > + * all threads. This is inherited upon fork(), and reinitialised upon exec*(). > + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey > + * instructions behaving as NOPs. > + */ I don't remember the past discussions but I assume the tools guys are ok with a single key shared by multiple threads. Ramana, could you ack this part, FTR? (and it would help if someone from the Android and Chrome camps can confirm) Thanks. -- Catalin
[+Cyrill Gorcunov for CRIU stuff] On Fri, Oct 19, 2018 at 12:15:43PM +0100, Catalin Marinas wrote: > On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote: > > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h > > new file mode 100644 > > index 000000000000..2aefedc31d9e > > --- /dev/null > > +++ b/arch/arm64/include/asm/pointer_auth.h > > @@ -0,0 +1,63 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#ifndef __ASM_POINTER_AUTH_H > > +#define __ASM_POINTER_AUTH_H > > + > > +#include <linux/random.h> > > + > > +#include <asm/cpufeature.h> > > +#include <asm/sysreg.h> > > + > > +#ifdef CONFIG_ARM64_PTR_AUTH > > +/* > > + * Each key is a 128-bit quantity which is split across a pair of 64-bit > > + * registers (Lo and Hi). > > + */ > > +struct ptrauth_key { > > + unsigned long lo, hi; > > +}; > > + > > +/* > > + * We give each process its own instruction A key (APIAKey), which is shared by > > + * all threads. This is inherited upon fork(), and reinitialised upon exec*(). > > + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey > > + * instructions behaving as NOPs. > > + */ > > I don't remember the past discussions but I assume the tools guys are ok > with a single key shared by multiple threads. Ramana, could you ack this > part, FTR? > > (and it would help if someone from the Android and Chrome camps can > confirm) FWIW: I think we should be entertaining a prctl() interface to use a new key on a per-thread basis. Obviously, this would need to be used with care (e.g. you'd fork(); use the prctl() and then you'd better not return from the calling function!). Assuming we want this (Kees -- I was under the impression that everything in Android would end up with the same key otherwise?), then the question is do we want: - prctl() get/set operations for the key, or - prctl() set_random_key operation, or - both of the above? Part of the answer to that may lie in the requirements of CRIU, where I strongly suspect they need explicit get/set operations, although these could be gated on CONFIG_CHECKPOINT_RESTORE=y. Will
On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <will.deacon@arm.com> wrote: > [+Cyrill Gorcunov for CRIU stuff] > > On Fri, Oct 19, 2018 at 12:15:43PM +0100, Catalin Marinas wrote: >> On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote: >> > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h >> > new file mode 100644 >> > index 000000000000..2aefedc31d9e >> > --- /dev/null >> > +++ b/arch/arm64/include/asm/pointer_auth.h >> > @@ -0,0 +1,63 @@ >> > +// SPDX-License-Identifier: GPL-2.0 >> > +#ifndef __ASM_POINTER_AUTH_H >> > +#define __ASM_POINTER_AUTH_H >> > + >> > +#include <linux/random.h> >> > + >> > +#include <asm/cpufeature.h> >> > +#include <asm/sysreg.h> >> > + >> > +#ifdef CONFIG_ARM64_PTR_AUTH >> > +/* >> > + * Each key is a 128-bit quantity which is split across a pair of 64-bit >> > + * registers (Lo and Hi). >> > + */ >> > +struct ptrauth_key { >> > + unsigned long lo, hi; >> > +}; >> > + >> > +/* >> > + * We give each process its own instruction A key (APIAKey), which is shared by >> > + * all threads. This is inherited upon fork(), and reinitialised upon exec*(). >> > + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey >> > + * instructions behaving as NOPs. >> > + */ >> >> I don't remember the past discussions but I assume the tools guys are ok >> with a single key shared by multiple threads. Ramana, could you ack this >> part, FTR? >> >> (and it would help if someone from the Android and Chrome camps can >> confirm) > > FWIW: I think we should be entertaining a prctl() interface to use a new > key on a per-thread basis. Obviously, this would need to be used with care > (e.g. you'd fork(); use the prctl() and then you'd better not return from > the calling function!). > > Assuming we want this (Kees -- I was under the impression that everything in > Android would end up with the same key otherwise?), then the question is > do we want: > > - prctl() get/set operations for the key, or > - prctl() set_random_key operation, or > - both of the above? > > Part of the answer to that may lie in the requirements of CRIU, where I > strongly suspect they need explicit get/set operations, although these > could be gated on CONFIG_CHECKPOINT_RESTORE=y. Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes. No reason to allow explicit access to the key (and selected algo) if we don't have to. As for per-thread or not, having a "pick a new key now" prctl() sounds good, but I'd like to have an eye toward having it just be "automatic" on clone(). -Kees -- Kees Cook Pixel Security
On Fri, Oct 19, 2018 at 08:36:45AM -0700, Kees Cook wrote: > On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <will.deacon@arm.com> wrote: > > FWIW: I think we should be entertaining a prctl() interface to use a new > > key on a per-thread basis. Obviously, this would need to be used with care > > (e.g. you'd fork(); use the prctl() and then you'd better not return from > > the calling function!). > > > > Assuming we want this (Kees -- I was under the impression that everything in > > Android would end up with the same key otherwise?), then the question is > > do we want: > > > > - prctl() get/set operations for the key, or > > - prctl() set_random_key operation, or > > - both of the above? > > > > Part of the answer to that may lie in the requirements of CRIU, where I > > strongly suspect they need explicit get/set operations, although these > > could be gated on CONFIG_CHECKPOINT_RESTORE=y. > > Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes. > No reason to allow explicit access to the key (and selected algo) if > we don't have to. Makes sense. > As for per-thread or not, having a "pick a new key now" prctl() sounds > good, but I'd like to have an eye toward having it just be "automatic" > on clone(). I thought about that too, but we're out of clone() flags afaict and there's no arch hook in there. We could add yet another clone syscall, but yuck (and I reckon viro would kill us). Or are you saying that we could infer the behaviour from the existing set of flags? Will
On Fri, Oct 19, 2018 at 08:36:45AM -0700, Kees Cook wrote: > On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <will.deacon@arm.com> wrote: > > Assuming we want this (Kees -- I was under the impression that everything in > > Android would end up with the same key otherwise?), then the question is > > do we want: > > > > - prctl() get/set operations for the key, or > > - prctl() set_random_key operation, or > > - both of the above? > > > > Part of the answer to that may lie in the requirements of CRIU, where I > > strongly suspect they need explicit get/set operations, although these > > could be gated on CONFIG_CHECKPOINT_RESTORE=y. > > Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes. > No reason to allow explicit access to the key (and selected algo) if > we don't have to. As a minor aside, the PAC algorithm (which can be IMPLEMENTATION DEFINED) is fixed in HW, and cannot be selected dynamically. Thus if a process is using pointer authentication, it would not be possible for CRIU to migrate that process to a CPU with a different PAC algorithm. Thanks, Mark.
On Fri, Oct 19, 2018 at 8:49 AM, Will Deacon <will.deacon@arm.com> wrote: > On Fri, Oct 19, 2018 at 08:36:45AM -0700, Kees Cook wrote: >> On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <will.deacon@arm.com> wrote: >> > FWIW: I think we should be entertaining a prctl() interface to use a new >> > key on a per-thread basis. Obviously, this would need to be used with care >> > (e.g. you'd fork(); use the prctl() and then you'd better not return from >> > the calling function!). >> > >> > Assuming we want this (Kees -- I was under the impression that everything in >> > Android would end up with the same key otherwise?), then the question is >> > do we want: >> > >> > - prctl() get/set operations for the key, or >> > - prctl() set_random_key operation, or >> > - both of the above? >> > >> > Part of the answer to that may lie in the requirements of CRIU, where I >> > strongly suspect they need explicit get/set operations, although these >> > could be gated on CONFIG_CHECKPOINT_RESTORE=y. >> >> Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes. >> No reason to allow explicit access to the key (and selected algo) if >> we don't have to. > > Makes sense. > >> As for per-thread or not, having a "pick a new key now" prctl() sounds >> good, but I'd like to have an eye toward having it just be "automatic" >> on clone(). > > I thought about that too, but we're out of clone() flags afaict and there's > no arch hook in there. We could add yet another clone syscall, but yuck (and > I reckon viro would kill us). > > Or are you saying that we could infer the behaviour from the existing set > of flags? I mean if it's starting a new thread, it should get a new key automatically, just like the ssp canary happens in dup_task_struct(). (Or did I miss some context for why that's not possible?) -Kees -- Kees Cook Pixel Security
On Fri, Oct 19, 2018 at 09:05:57AM -0700, Kees Cook wrote: > On Fri, Oct 19, 2018 at 8:49 AM, Will Deacon <will.deacon@arm.com> wrote: > > On Fri, Oct 19, 2018 at 08:36:45AM -0700, Kees Cook wrote: > >> On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <will.deacon@arm.com> wrote: > >> > FWIW: I think we should be entertaining a prctl() interface to use a new > >> > key on a per-thread basis. Obviously, this would need to be used with care > >> > (e.g. you'd fork(); use the prctl() and then you'd better not return from > >> > the calling function!). > >> > > >> > Assuming we want this (Kees -- I was under the impression that everything in > >> > Android would end up with the same key otherwise?), then the question is > >> > do we want: > >> > > >> > - prctl() get/set operations for the key, or > >> > - prctl() set_random_key operation, or > >> > - both of the above? > >> > > >> > Part of the answer to that may lie in the requirements of CRIU, where I > >> > strongly suspect they need explicit get/set operations, although these > >> > could be gated on CONFIG_CHECKPOINT_RESTORE=y. > >> > >> Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes. > >> No reason to allow explicit access to the key (and selected algo) if > >> we don't have to. > > > > Makes sense. > > > >> As for per-thread or not, having a "pick a new key now" prctl() sounds > >> good, but I'd like to have an eye toward having it just be "automatic" > >> on clone(). > > > > I thought about that too, but we're out of clone() flags afaict and there's > > no arch hook in there. We could add yet another clone syscall, but yuck (and > > I reckon viro would kill us). > > > > Or are you saying that we could infer the behaviour from the existing set > > of flags? > > I mean if it's starting a new thread, it should get a new key > automatically, just like the ssp canary happens in dup_task_struct(). > > (Or did I miss some context for why that's not possible?) The problem with that is if the child thread (in userspace) returns from the function that called fork(), then it will explode because the link register will have been signed with the parent key. Will
On Fri, Oct 19, 2018 at 12:24:04PM +0100, Will Deacon wrote: > > FWIW: I think we should be entertaining a prctl() interface to use a new > key on a per-thread basis. Obviously, this would need to be used with care > (e.g. you'd fork(); use the prctl() and then you'd better not return from > the calling function!). > > Assuming we want this (Kees -- I was under the impression that everything in > Android would end up with the same key otherwise?), then the question is > do we want: > > - prctl() get/set operations for the key, or > - prctl() set_random_key operation, or > - both of the above? > > Part of the answer to that may lie in the requirements of CRIU, where I > strongly suspect they need explicit get/set operations, although these > could be gated on CONFIG_CHECKPOINT_RESTORE=y. Indeed. Without get/set I think we won't be able to restore programs.
On 19/10/2018 12:15, Catalin Marinas wrote: > On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote: >> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h >> new file mode 100644 >> index 000000000000..2aefedc31d9e >> --- /dev/null >> +++ b/arch/arm64/include/asm/pointer_auth.h >> @@ -0,0 +1,63 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +#ifndef __ASM_POINTER_AUTH_H >> +#define __ASM_POINTER_AUTH_H >> + >> +#include <linux/random.h> >> + >> +#include <asm/cpufeature.h> >> +#include <asm/sysreg.h> >> + >> +#ifdef CONFIG_ARM64_PTR_AUTH >> +/* >> + * Each key is a 128-bit quantity which is split across a pair of 64-bit >> + * registers (Lo and Hi). >> + */ >> +struct ptrauth_key { >> + unsigned long lo, hi; >> +}; >> + >> +/* >> + * We give each process its own instruction A key (APIAKey), which is shared by >> + * all threads. This is inherited upon fork(), and reinitialised upon exec*(). >> + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey >> + * instructions behaving as NOPs. >> + */ > > I don't remember the past discussions but I assume the tools guys are ok > with a single key shared by multiple threads. Ramana, could you ack this > part, FTR? Sorry about the slow response, I've been traveling. Ack and Will's response covers the reasons why pretty well. A prctl call would be a good enhancement. regards Ramana
On Tue, Oct 23, 2018 at 09:36:16AM +0100, Ramana Radhakrishnan wrote: > On 19/10/2018 12:15, Catalin Marinas wrote: > > On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote: > >> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h > >> new file mode 100644 > >> index 000000000000..2aefedc31d9e > >> --- /dev/null > >> +++ b/arch/arm64/include/asm/pointer_auth.h > >> @@ -0,0 +1,63 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +#ifndef __ASM_POINTER_AUTH_H > >> +#define __ASM_POINTER_AUTH_H > >> + > >> +#include <linux/random.h> > >> + > >> +#include <asm/cpufeature.h> > >> +#include <asm/sysreg.h> > >> + > >> +#ifdef CONFIG_ARM64_PTR_AUTH > >> +/* > >> + * Each key is a 128-bit quantity which is split across a pair of 64-bit > >> + * registers (Lo and Hi). > >> + */ > >> +struct ptrauth_key { > >> + unsigned long lo, hi; > >> +}; > >> + > >> +/* > >> + * We give each process its own instruction A key (APIAKey), which is shared by > >> + * all threads. This is inherited upon fork(), and reinitialised upon exec*(). > >> + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey > >> + * instructions behaving as NOPs. > >> + */ > > > > I don't remember the past discussions but I assume the tools guys are ok > > with a single key shared by multiple threads. Ramana, could you ack this > > part, FTR? > > Sorry about the slow response, I've been traveling. > > Ack and Will's response covers the reasons why pretty well. A prctl call > would be a good enhancement. One minor "gotcha" with that is that the glibc prctl() wrapper would need to be annotated not to use pointer auth, or we'd have to issue the syscall in-line. Will
Hi all, On Fri, Oct 19, 2018 at 12:24:04PM +0100, Will Deacon wrote: > On Fri, Oct 19, 2018 at 12:15:43PM +0100, Catalin Marinas wrote: > > On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote: > > > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h > > > new file mode 100644 > > > index 000000000000..2aefedc31d9e > > > --- /dev/null > > > +++ b/arch/arm64/include/asm/pointer_auth.h > > > @@ -0,0 +1,63 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +#ifndef __ASM_POINTER_AUTH_H > > > +#define __ASM_POINTER_AUTH_H > > > + > > > +#include <linux/random.h> > > > + > > > +#include <asm/cpufeature.h> > > > +#include <asm/sysreg.h> > > > + > > > +#ifdef CONFIG_ARM64_PTR_AUTH > > > +/* > > > + * Each key is a 128-bit quantity which is split across a pair of 64-bit > > > + * registers (Lo and Hi). > > > + */ > > > +struct ptrauth_key { > > > + unsigned long lo, hi; > > > +}; > > > + > > > +/* > > > + * We give each process its own instruction A key (APIAKey), which is shared by > > > + * all threads. This is inherited upon fork(), and reinitialised upon exec*(). > > > + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey > > > + * instructions behaving as NOPs. > > > + */ > > > > I don't remember the past discussions but I assume the tools guys are ok > > with a single key shared by multiple threads. Ramana, could you ack this > > part, FTR? > > > > (and it would help if someone from the Android and Chrome camps can > > confirm) > > FWIW: I think we should be entertaining a prctl() interface to use a new > key on a per-thread basis. Obviously, this would need to be used with care > (e.g. you'd fork(); use the prctl() and then you'd better not return from > the calling function!). > > Assuming we want this (Kees -- I was under the impression that everything in > Android would end up with the same key otherwise?), then the question is > do we want: > > - prctl() get/set operations for the key, or > - prctl() set_random_key operation, or > - both of the above? > > Part of the answer to that may lie in the requirements of CRIU, where I > strongly suspect they need explicit get/set operations, although these > could be gated on CONFIG_CHECKPOINT_RESTORE=y. I managed to speak to the CRIU developers at LPC. The good news is that their preference is for a ptrace()-based interface for getting and setting the keys, so the only prctl() operation we need is to set a random key (separately for A and B). Will
On Wed, Nov 14, 2018 at 06:11:39PM +0000, Will Deacon wrote: [...] > On Fri, Oct 19, 2018 at 12:24:04PM +0100, Will Deacon wrote: [...] > > FWIW: I think we should be entertaining a prctl() interface to use a new > > key on a per-thread basis. Obviously, this would need to be used with care > > (e.g. you'd fork(); use the prctl() and then you'd better not return from > > the calling function!). > > > > Assuming we want this (Kees -- I was under the impression that everything in > > Android would end up with the same key otherwise?), then the question is > > do we want: > > > > - prctl() get/set operations for the key, or > > - prctl() set_random_key operation, or > > - both of the above? > > > > Part of the answer to that may lie in the requirements of CRIU, where I > > strongly suspect they need explicit get/set operations, although these > > could be gated on CONFIG_CHECKPOINT_RESTORE=y. > > I managed to speak to the CRIU developers at LPC. The good news is that > their preference is for a ptrace()-based interface for getting and setting > the keys, so the only prctl() operation we need is to set a random key > (separately for A and B). That's good if it works for them, and it seems the cleaner approach. _If_ they run the new thread up to a checkpoint, restoring the memory and doing all the setup that requires in-thread syscalls, then stop it in ptrace to finally inject the regs, then it makes sense to set the keys at that stop -- i.e., you set the keys atomically* with the final setting of the thread's PC. (* with respect to the target thread) So long as you're confident they've understood the implications of ptrauth for CRIU, I guess this can work. (In other news, they will also need to do some work to support SVE, but that's unrelated to ptrauth.) Cheers ---Dave
diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h index dd320df0d026..f6480ea7b0d5 100644 --- a/arch/arm64/include/asm/mmu.h +++ b/arch/arm64/include/asm/mmu.h @@ -25,10 +25,15 @@ #ifndef __ASSEMBLY__ +#include <asm/pointer_auth.h> + typedef struct { atomic64_t id; void *vdso; unsigned long flags; +#ifdef CONFIG_ARM64_PTR_AUTH + struct ptrauth_keys ptrauth_keys; +#endif } mm_context_t; /* diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h index 39ec0b8a689e..983f80925566 100644 --- a/arch/arm64/include/asm/mmu_context.h +++ b/arch/arm64/include/asm/mmu_context.h @@ -29,7 +29,6 @@ #include <asm/cacheflush.h> #include <asm/cpufeature.h> #include <asm/proc-fns.h> -#include <asm-generic/mm_hooks.h> #include <asm/cputype.h> #include <asm/pgtable.h> #include <asm/sysreg.h> @@ -216,6 +215,8 @@ static inline void __switch_mm(struct mm_struct *next) return; } + mm_ctx_ptrauth_switch(&next->context); + check_and_switch_context(next, cpu); } @@ -241,6 +242,19 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next, void verify_cpu_asid_bits(void); void post_ttbr_update_workaround(void); +static inline void arch_bprm_mm_init(struct mm_struct *mm, + struct vm_area_struct *vma) +{ + mm_ctx_ptrauth_init(&mm->context); +} +#define arch_bprm_mm_init arch_bprm_mm_init + +/* + * We need to override arch_bprm_mm_init before including the generic hooks, + * which are otherwise sufficient for us. + */ +#include <asm-generic/mm_hooks.h> + #endif /* !__ASSEMBLY__ */ #endif /* !__ASM_MMU_CONTEXT_H */ diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h new file mode 100644 index 000000000000..2aefedc31d9e --- /dev/null +++ b/arch/arm64/include/asm/pointer_auth.h @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: GPL-2.0 +#ifndef __ASM_POINTER_AUTH_H +#define __ASM_POINTER_AUTH_H + +#include <linux/random.h> + +#include <asm/cpufeature.h> +#include <asm/sysreg.h> + +#ifdef CONFIG_ARM64_PTR_AUTH +/* + * Each key is a 128-bit quantity which is split across a pair of 64-bit + * registers (Lo and Hi). + */ +struct ptrauth_key { + unsigned long lo, hi; +}; + +/* + * We give each process its own instruction A key (APIAKey), which is shared by + * all threads. This is inherited upon fork(), and reinitialised upon exec*(). + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey + * instructions behaving as NOPs. + */ +struct ptrauth_keys { + struct ptrauth_key apia; +}; + +static inline void ptrauth_keys_init(struct ptrauth_keys *keys) +{ + if (!cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH)) + return; + + get_random_bytes(keys, sizeof(*keys)); +} + +#define __ptrauth_key_install(k, v) \ +do { \ + struct ptrauth_key __pki_v = (v); \ + write_sysreg_s(__pki_v.lo, SYS_ ## k ## KEYLO_EL1); \ + write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1); \ +} while (0) + +static inline void ptrauth_keys_switch(struct ptrauth_keys *keys) +{ + if (!cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH)) + return; + + __ptrauth_key_install(APIA, keys->apia); +} + +#define mm_ctx_ptrauth_init(ctx) \ + ptrauth_keys_init(&(ctx)->ptrauth_keys) + +#define mm_ctx_ptrauth_switch(ctx) \ + ptrauth_keys_switch(&(ctx)->ptrauth_keys) + +#else /* CONFIG_ARM64_PTR_AUTH */ +#define mm_ctx_ptrauth_init(ctx) +#define mm_ctx_ptrauth_switch(ctx) +#endif /* CONFIG_ARM64_PTR_AUTH */ + +#endif /* __ASM_POINTER_AUTH_H */ diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h index 17c65c8f33cb..01f02ac500ae 100644 --- a/arch/arm64/include/uapi/asm/hwcap.h +++ b/arch/arm64/include/uapi/asm/hwcap.h @@ -48,5 +48,6 @@ #define HWCAP_USCAT (1 << 25) #define HWCAP_ILRCPC (1 << 26) #define HWCAP_FLAGM (1 << 27) +#define HWCAP_APIA (1 << 28) #endif /* _UAPI__ASM_HWCAP_H */ diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 0dd171c7d71e..3157685aa56a 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -1040,6 +1040,11 @@ static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused) } #ifdef CONFIG_ARM64_PTR_AUTH +static void cpu_enable_address_auth(struct arm64_cpu_capabilities const *cap) +{ + sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA); +} + static bool has_address_auth(const struct arm64_cpu_capabilities *entry, int __unused) { @@ -1267,6 +1272,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .capability = ARM64_HAS_ADDRESS_AUTH, .type = ARM64_CPUCAP_SYSTEM_FEATURE, .matches = has_address_auth, + .cpu_enable = cpu_enable_address_auth, }, #endif /* CONFIG_ARM64_PTR_AUTH */ {}, @@ -1314,6 +1320,10 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = { #ifdef CONFIG_ARM64_SVE HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_SVE_SHIFT, FTR_UNSIGNED, ID_AA64PFR0_SVE, CAP_HWCAP, HWCAP_SVE), #endif +#ifdef CONFIG_ARM64_PTR_AUTH + HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_APA_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_APIA), + HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_API_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_APIA), +#endif {}, }; diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c index e9ab7b3ed317..608411e3aaff 100644 --- a/arch/arm64/kernel/cpuinfo.c +++ b/arch/arm64/kernel/cpuinfo.c @@ -81,6 +81,7 @@ static const char *const hwcap_str[] = { "uscat", "ilrcpc", "flagm", + "apia", NULL };