Re: [PATCH v3 06/16] arm64: enable ptrauth earlier

From: Amit Kachhap <amit.kachhap@arm.com>
To: Suzuki Kuruppassery Poulose <suzuki.poulose@arm.com>,
	linux-arm-kernel@lists.infradead.org
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kees Cook <keescook@chromium.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Kristina Martsenko <kristina.martsenko@arm.com>,
	James Morse <james.morse@arm.com>,
	Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
	Mark Brown <Mark.Brown@arm.com>,
	Vincenzo Frascino <Vincenzo.Frascino@arm.com>,
	Will Deacon <will@kernel.org>, Dave Martin <Dave.Martin@arm.com>
Subject: Re: [PATCH v3 06/16] arm64: enable ptrauth earlier
Date: Thu, 9 Jan 2020 13:59:06 +0530	[thread overview]
Message-ID: <0c93c40c-e691-d768-da25-7f191624e56c@arm.com> (raw)
In-Reply-To: <e49002f2-53fb-b997-8bd6-363e22153da3@arm.com>

Hi Suzuki,

On 1/7/20 5:05 PM, Suzuki Kuruppassery Poulose wrote:
> On 16/12/2019 08:47, Amit Daniel Kachhap wrote:
>> From: Kristina Martsenko <kristina.martsenko@arm.com>
>>
>> When the kernel is compiled with pointer auth instructions, the boot CPU
>> needs to start using address auth very early, so change the cpucap to
>> account for this.
>>
>> Pointer auth must be enabled before we call C functions, because it is
>> not possible to enter a function with pointer auth disabled and exit it
>> with pointer auth enabled. Note, mismatches between architected and
>> IMPDEF algorithms will still be caught by the cpufeature framework (the
>> separate *_ARCH and *_IMP_DEF cpucaps).
>>
>> Note the change in behavior: if the boot CPU has address auth and a late
>> CPU does not, then we park the late CPU very early in booting. Also, if
>> the boot CPU does not have address auth and the late CPU has then system
>> panic will occur little later from inside the C code. Until now we would
>> have just disabled address auth in this case.
>>
>> Leave generic authentication as a "system scope" cpucap for now, since
>> initially the kernel will only use address authentication.
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
>> [Amit: Re-worked ptrauth setup logic, comments]
>> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
>> ---
>> Changes since last version:
>> * None.
>>
>>   arch/arm64/Kconfig             |  5 +++++
>>   arch/arm64/include/asm/smp.h   |  1 +
>>   arch/arm64/kernel/cpufeature.c | 13 +++----------
>>   arch/arm64/kernel/head.S       | 20 ++++++++++++++++++++
>>   arch/arm64/kernel/smp.c        |  2 ++
>>   arch/arm64/mm/proc.S           | 31 +++++++++++++++++++++++++++++++
>>   6 files changed, 62 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index b1b4476..5aabe8a 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -1482,6 +1482,11 @@ config ARM64_PTR_AUTH
>>         be enabled. However, KVM guest also require VHE mode and hence
>>         CONFIG_ARM64_VHE=y option to use this feature.
>> +      If the feature is present on the primary CPU but not a 
>> secondary CPU,
>> +      then the secondary CPU will be parked.
> 
> ---
> 
>>    Also, if the boot CPU does not
>> +      have address auth and the late CPU has then system panic will 
>> occur.
>> +      On such a system, this option should not be selected.
> 
> Is this part of the text true ? We do not enable ptr-auth on the CPUs if
> we are missing the support on primary. So, given we disable SCTLR bits,
> the ptr-auth instructions should be a NOP and is thus safe.

I got little confused with your earlier comments [1] and made the 
secondary cpu's panic in case they have ptrauth and primary don't. In 
this case ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU will leave it running and 
not panic as you mentioned.

I will append ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU feature and update the 
comments over here accordingly in my next iteration.

[1]: https://patchwork.kernel.org/patch/11195087/

> 
> The rest looks good to me. With the above text removed,
> 
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Thanks for reviewing.
> 

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