| Message ID | 20210208145554.2164638-3-daniel.kiss@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: split ARM64_PTR_AUTH option to userspace and kernel | expand |
On Mon, Feb 08, 2021 at 03:55:54PM +0100, Daniel Kiss wrote: > If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL=y, > then no PACI/AUTI instructions are expected while the kernel is running > so the kernel's key will not be used. Write of a system regiters > is expensive therefore avoid it not required. What happens if somebody tries to load a module built with PAC into a kernel where CONFIG_ARM64_PTR_AUTH_KERNEL=n? Do we reject the module? I'm not sure how much we care, but I'm a bit worried that it might not go "obviously" wrong. > Signed-off-by: Daniel Kiss <daniel.kiss@arm.com> > --- > arch/arm64/include/asm/asm_pointer_auth.h | 63 +++++++++++++---------- > arch/arm64/include/asm/pointer_auth.h | 20 +++++-- > arch/arm64/include/asm/processor.h | 2 + > arch/arm64/kernel/asm-offsets.c | 2 + > 4 files changed, 54 insertions(+), 33 deletions(-) [...] > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h > index c6b4f0603024..b34aebb95757 100644 > --- a/arch/arm64/include/asm/pointer_auth.h > +++ b/arch/arm64/include/asm/pointer_auth.h > @@ -30,9 +30,11 @@ struct ptrauth_keys_user { > struct ptrauth_key apga; > }; > > +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL > struct ptrauth_keys_kernel { > struct ptrauth_key apia; > }; > +#endif > > static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys) > { > @@ -54,6 +56,8 @@ do { \ > write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1); \ > } while (0) > > +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL > + > static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys) > { > if (system_supports_address_auth()) > @@ -69,6 +73,8 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne > isb(); > } > > +#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */ Can you group this with the struct and avoid having two #ifdef blocks? Will
> On 29 Mar 2021, at 16:51, Will Deacon <will@kernel.org> wrote: > > On Mon, Feb 08, 2021 at 03:55:54PM +0100, Daniel Kiss wrote: >> If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL=y, >> then no PACI/AUTI instructions are expected while the kernel is running >> so the kernel's key will not be used. Write of a system regiters >> is expensive therefore avoid it not required. > > What happens if somebody tries to load a module built with PAC into a kernel > where CONFIG_ARM64_PTR_AUTH_KERNEL=n? Do we reject the module? I think it will be loaded, but actually nothing will happen because then the instruction will be just NOPs. Other way will work as well. Enforcement will be trick because module might sneak the compiler flag in that overrides the kconfig. > I'm not sure how much we care, but I'm a bit worried that it might not go > "obviously" wrong. > >> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com> >> --- >> arch/arm64/include/asm/asm_pointer_auth.h | 63 +++++++++++++---------- >> arch/arm64/include/asm/pointer_auth.h | 20 +++++-- >> arch/arm64/include/asm/processor.h | 2 + >> arch/arm64/kernel/asm-offsets.c | 2 + >> 4 files changed, 54 insertions(+), 33 deletions(-) > > [...] > >> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h >> index c6b4f0603024..b34aebb95757 100644 >> --- a/arch/arm64/include/asm/pointer_auth.h >> +++ b/arch/arm64/include/asm/pointer_auth.h >> @@ -30,9 +30,11 @@ struct ptrauth_keys_user { >> struct ptrauth_key apga; >> }; >> >> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL >> struct ptrauth_keys_kernel { >> struct ptrauth_key apia; >> }; >> +#endif >> >> static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys) >> { >> @@ -54,6 +56,8 @@ do { \ >> write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1); \ >> } while (0) >> >> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL >> + >> static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys) >> { >> if (system_supports_address_auth()) >> @@ -69,6 +73,8 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne >> isb(); >> } >> >> +#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */ > > Can you group this with the struct and avoid having two #ifdef blocks? Sure, I'll send a version. > > Will
diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h index 52dead2a8640..413cc36fdd5c 100644 --- a/arch/arm64/include/asm/asm_pointer_auth.h +++ b/arch/arm64/include/asm/asm_pointer_auth.h @@ -7,6 +7,38 @@ #include <asm/cpufeature.h> #include <asm/sysreg.h> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL + .macro __ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3 + mov \tmp1, #THREAD_KEYS_KERNEL + add \tmp1, \tsk, \tmp1 + ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_KERNEL_KEY_APIA] + msr_s SYS_APIAKEYLO_EL1, \tmp2 + msr_s SYS_APIAKEYHI_EL1, \tmp3 + .endm + + .macro ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3 +alternative_if ARM64_HAS_ADDRESS_AUTH + __ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3 +alternative_else_nop_endif + .endm + + .macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3 +alternative_if ARM64_HAS_ADDRESS_AUTH + __ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3 + isb +alternative_else_nop_endif + .endm + +#else /* CONFIG_ARM64_PTR_AUTH_KERNEL */ + + .macro ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3 + .endm + + .macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3 + .endm + +#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */ + #ifdef CONFIG_ARM64_PTR_AUTH /* * thread.keys_user.ap* as offset exceeds the #imm offset range @@ -39,27 +71,6 @@ alternative_if ARM64_HAS_GENERIC_AUTH alternative_else_nop_endif .endm - .macro __ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3 - mov \tmp1, #THREAD_KEYS_KERNEL - add \tmp1, \tsk, \tmp1 - ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_KERNEL_KEY_APIA] - msr_s SYS_APIAKEYLO_EL1, \tmp2 - msr_s SYS_APIAKEYHI_EL1, \tmp3 - .endm - - .macro ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3 -alternative_if ARM64_HAS_ADDRESS_AUTH - __ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3 -alternative_else_nop_endif - .endm - - .macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3 -alternative_if ARM64_HAS_ADDRESS_AUTH - __ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3 - isb -alternative_else_nop_endif - .endm - .macro __ptrauth_keys_init_cpu tsk, tmp1, tmp2, tmp3 mrs \tmp1, id_aa64isar1_el1 ubfx \tmp1, \tmp1, #ID_AA64ISAR1_APA_SHIFT, #8 @@ -69,7 +80,9 @@ alternative_else_nop_endif mrs \tmp2, sctlr_el1 orr \tmp2, \tmp2, \tmp1 msr sctlr_el1, \tmp2 +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL __ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3 +#endif isb .Lno_addr_auth\@: .endm @@ -82,17 +95,11 @@ alternative_else_nop_endif .Lno_addr_auth\@: .endm -#else /* CONFIG_ARM64_PTR_AUTH */ +#else /* !CONFIG_ARM64_PTR_AUTH */ .macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3 .endm - .macro ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3 - .endm - - .macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3 - .endm - #endif /* CONFIG_ARM64_PTR_AUTH */ #endif /* __ASM_ASM_POINTER_AUTH_H */ diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h index c6b4f0603024..b34aebb95757 100644 --- a/arch/arm64/include/asm/pointer_auth.h +++ b/arch/arm64/include/asm/pointer_auth.h @@ -30,9 +30,11 @@ struct ptrauth_keys_user { struct ptrauth_key apga; }; +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL struct ptrauth_keys_kernel { struct ptrauth_key apia; }; +#endif static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys) { @@ -54,6 +56,8 @@ do { \ write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1); \ } while (0) +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL + static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys) { if (system_supports_address_auth()) @@ -69,6 +73,8 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne isb(); } +#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */ + extern int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg); static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr) @@ -78,17 +84,21 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr) #define ptrauth_thread_init_user(tsk) \ ptrauth_keys_init_user(&(tsk)->thread.keys_user) -#define ptrauth_thread_init_kernel(tsk) \ - ptrauth_keys_init_kernel(&(tsk)->thread.keys_kernel) -#define ptrauth_thread_switch_kernel(tsk) \ - ptrauth_keys_switch_kernel(&(tsk)->thread.keys_kernel) #else /* CONFIG_ARM64_PTR_AUTH */ #define ptrauth_prctl_reset_keys(tsk, arg) (-EINVAL) #define ptrauth_strip_insn_pac(lr) (lr) #define ptrauth_thread_init_user(tsk) +#endif /* CONFIG_ARM64_PTR_AUTH */ + +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL +#define ptrauth_thread_init_kernel(tsk) \ + ptrauth_keys_init_kernel(&(tsk)->thread.keys_kernel) +#define ptrauth_thread_switch_kernel(tsk) \ + ptrauth_keys_switch_kernel(&(tsk)->thread.keys_kernel) +#else #define ptrauth_thread_init_kernel(tsk) #define ptrauth_thread_switch_kernel(tsk) -#endif /* CONFIG_ARM64_PTR_AUTH */ +#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */ #endif /* __ASM_POINTER_AUTH_H */ diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index ca2cd75d3286..318e0e7cf9e1 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h @@ -148,8 +148,10 @@ struct thread_struct { struct debug_info debug; /* debugging */ #ifdef CONFIG_ARM64_PTR_AUTH struct ptrauth_keys_user keys_user; +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL struct ptrauth_keys_kernel keys_kernel; #endif +#endif #ifdef CONFIG_ARM64_MTE u64 sctlr_tcf0; u64 gcr_user_excl; diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index 21c9f98f868d..3b58804e4224 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -150,7 +150,9 @@ int main(void) DEFINE(PTRAUTH_USER_KEY_APDA, offsetof(struct ptrauth_keys_user, apda)); DEFINE(PTRAUTH_USER_KEY_APDB, offsetof(struct ptrauth_keys_user, apdb)); DEFINE(PTRAUTH_USER_KEY_APGA, offsetof(struct ptrauth_keys_user, apga)); +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL DEFINE(PTRAUTH_KERNEL_KEY_APIA, offsetof(struct ptrauth_keys_kernel, apia)); +#endif BLANK(); #endif return 0;
If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL=y, then no PACI/AUTI instructions are expected while the kernel is running so the kernel's key will not be used. Write of a system regiters is expensive therefore avoid it not required. Signed-off-by: Daniel Kiss <daniel.kiss@arm.com> --- arch/arm64/include/asm/asm_pointer_auth.h | 63 +++++++++++++---------- arch/arm64/include/asm/pointer_auth.h | 20 +++++-- arch/arm64/include/asm/processor.h | 2 + arch/arm64/kernel/asm-offsets.c | 2 + 4 files changed, 54 insertions(+), 33 deletions(-)