linux-kernel - Re: [PATCH 10/16] arm64: Make KPTI strict CPU local feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180126122519.GR5862@e103592.cambridge.arm.com>
Date:   Fri, 26 Jan 2018 12:25:19 +0000
From:   Dave Martin <Dave.Martin@....com>
To:     Suzuki K Poulose <suzuki.poulose@....com>
Cc:     linux-arm-kernel@...ts.infradead.org, mark.rutland@....com,
        ckadabi@...eaurora.org, ard.biesheuvel@...aro.org,
        marc.zyngier@....com, catalin.marinas@....com, will.deacon@....com,
        linux-kernel@...r.kernel.org, jnair@...iumnetworks.com
Subject: Re: [PATCH 10/16] arm64: Make KPTI strict CPU local feature

On Tue, Jan 23, 2018 at 12:28:03PM +0000, Suzuki K Poulose wrote:
> KPTI capability is a security feature which should be enabled
> when at least one CPU on the system needs it. Any late CPU
> which needs the kernel support, should be prevented from
> booting (and thus making the system unsecure) if the feature
> was not already enabled.

Is there an actual change to behaviour here?

It's not very obvious from the commit message, or the patch when read in
isolation.

> 
> Cc: Will Deacon <will.deacon@....com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>
> ---
>  arch/arm64/kernel/cpufeature.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 111f6c4b4cd7..2627a836e99d 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -862,9 +862,8 @@ static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int __unus
>  static int __kpti_forced; /* 0: not forced, >0: forced on, <0: forced off */
>  
>  static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,

[Nit: All the other .matches functions have a predicate-style name, so
"should_unmap_kernel_at_el0" or "must_unmap_kernel_at_el0" might be
better names...  However, the name wasn't introduced by this series and
it's not a huge deal.] 

> -				int __unused)
> +				int scope)
>  {
> -	u64 pfr0 = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
>  
>  	/* Forced on command line? */
>  	if (__kpti_forced) {
> @@ -878,8 +877,7 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
>  		return true;
>  
>  	/* Defer to CPU feature registers */
> -	return !cpuid_feature_extract_unsigned_field(pfr0,
> -						     ID_AA64PFR0_CSV3_SHIFT);
> +	return !has_cpuid_feature(entry, scope);
>  }
>  
>  static int __init parse_kpti(char *str)
> @@ -1003,7 +1001,10 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  	{
>  		.desc = "Kernel page table isolation (KPTI)",
>  		.capability = ARM64_UNMAP_KERNEL_AT_EL0,
> -		.type = ARM64_CPUCAP_BOOT_SYSTEM_FEATURE,
> +		.type = ARM64_CPUCAP_STRICT_CPU_LOCAL_FEATURE,
> +		.sys_reg = SYS_ID_AA64PFR0_EL1,
> +		.field_pos = ID_AA64PFR0_CSV3_SHIFT,
> +		.min_field_value = 1,
>  		.matches = unmap_kernel_at_el0,
>  	},
>  #endif

[...]

Cheers
---Dave