Re: [PATCH v3 2/2] arm64: Conditionally configure PTR_AUTH key of the kernel.

From: Daniel Kiss <Daniel.Kiss@arm.com>
To: Will Deacon <will@kernel.org>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Ard Biesheuvel <Ard.Biesheuvel@arm.com>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	"pcc@google.com" <pcc@google.com>
Subject: Re: [PATCH v3 2/2] arm64: Conditionally configure PTR_AUTH key of the kernel.
Date: Mon, 12 Apr 2021 20:17:05 +0000	[thread overview]
Message-ID: <04AA56B1-FF63-4DB1-826B-037A7AF03CCD@arm.com> (raw)
In-Reply-To: <20210329145137.GE4203@willie-the-truck>


> 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel