| Message ID | 20190529190332.29753-2-kristina.martsenko@arm.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | arm64: return address signing | expand |
On Wed, May 29, 2019 at 08:03:26PM +0100, Kristina Martsenko wrote: > To enable pointer auth for the kernel, we're going to need to check for > the presence of address auth and generic auth using alternative_if. We > currently have two cpucaps for each, but alternative_if needs to check a > single cpucap. So define meta-capabilities that are present when either > of the current two capabilities is present. > > Leave the existing four cpucaps in place, as they are still needed to > check for mismatched systems where one CPU has the architected algorithm > but another has the IMP DEF algorithm. > > Note, the meta-capabilities were present before but were removed in > commits a56005d32105 ("arm64: cpufeature: Reduce number of pointer auth > CPU caps from 6 to 4") and 1e013d06120c ("arm64: cpufeature: Rework ptr > auth hwcaps using multi_entry_cap_matches"), as they were not needed > then. Note, unlike before, the current patch checks the cpucap values > directly, instead of reading the CPU ID register value. > > Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com> Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > > Changes since RFC v1: > - New patch, as the meta-capabilities have been removed upstream > > arch/arm64/include/asm/cpucaps.h | 4 +++- > arch/arm64/include/asm/cpufeature.h | 6 ++---- > arch/arm64/kernel/cpufeature.c | 25 ++++++++++++++++++++++++- > 3 files changed, 29 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h > index f6a76e43f39e..601183b7b484 100644 > --- a/arch/arm64/include/asm/cpucaps.h > +++ b/arch/arm64/include/asm/cpucaps.h > @@ -61,7 +61,9 @@ > #define ARM64_HAS_GENERIC_AUTH_ARCH 40 > #define ARM64_HAS_GENERIC_AUTH_IMP_DEF 41 > #define ARM64_HAS_IRQ_PRIO_MASKING 42 > +#define ARM64_HAS_ADDRESS_AUTH 43 > +#define ARM64_HAS_GENERIC_AUTH 44 > > -#define ARM64_NCAPS 43 > +#define ARM64_NCAPS 45 > > #endif /* __ASM_CPUCAPS_H */ > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index e505e1fbd2b9..0522ea674253 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -605,15 +605,13 @@ static inline bool system_supports_cnp(void) > static inline bool system_supports_address_auth(void) > { > return IS_ENABLED(CONFIG_ARM64_PTR_AUTH) && > - (cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH_ARCH) || > - cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH_IMP_DEF)); > + cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH); > } > > static inline bool system_supports_generic_auth(void) > { > return IS_ENABLED(CONFIG_ARM64_PTR_AUTH) && > - (cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH_ARCH) || > - cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH_IMP_DEF)); > + cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH); > } > > static inline bool system_uses_irq_prio_masking(void) > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index 4061de10cea6..166584deaed2 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -1205,6 +1205,20 @@ static void cpu_enable_address_auth(struct arm64_cpu_capabilities const *cap) > sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | > SCTLR_ELx_ENDA | SCTLR_ELx_ENDB); > } > + > +static bool has_address_auth(const struct arm64_cpu_capabilities *entry, > + int __unused) > +{ > + return cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH_ARCH) || > + cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH_IMP_DEF); > +} > + > +static bool has_generic_auth(const struct arm64_cpu_capabilities *entry, > + int __unused) > +{ > + return cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH_ARCH) || > + cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH_IMP_DEF); > +} > #endif /* CONFIG_ARM64_PTR_AUTH */ > > #ifdef CONFIG_ARM64_PSEUDO_NMI > @@ -1466,7 +1480,6 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > .field_pos = ID_AA64ISAR1_APA_SHIFT, > .min_field_value = ID_AA64ISAR1_APA_ARCHITECTED, > .matches = has_cpuid_feature, > - .cpu_enable = cpu_enable_address_auth, > }, > { > .desc = "Address authentication (IMP DEF algorithm)", > @@ -1477,6 +1490,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > .field_pos = ID_AA64ISAR1_API_SHIFT, > .min_field_value = ID_AA64ISAR1_API_IMP_DEF, > .matches = has_cpuid_feature, > + }, > + { > + .capability = ARM64_HAS_ADDRESS_AUTH, > + .type = ARM64_CPUCAP_SYSTEM_FEATURE, > + .matches = has_address_auth, > .cpu_enable = cpu_enable_address_auth, > }, > { > @@ -1499,6 +1517,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > .min_field_value = ID_AA64ISAR1_GPI_IMP_DEF, > .matches = has_cpuid_feature, > }, > + { > + .capability = ARM64_HAS_GENERIC_AUTH, > + .type = ARM64_CPUCAP_SYSTEM_FEATURE, > + .matches = has_generic_auth, > + }, > #endif /* CONFIG_ARM64_PTR_AUTH */ > #ifdef CONFIG_ARM64_PSEUDO_NMI > { > -- > 2.11.0 >
Hi Kristina, On 05/29/2019 08:03 PM, Kristina Martsenko wrote: > To enable pointer auth for the kernel, we're going to need to check for > the presence of address auth and generic auth using alternative_if. We > currently have two cpucaps for each, but alternative_if needs to check a > single cpucap. So define meta-capabilities that are present when either > of the current two capabilities is present. > > Leave the existing four cpucaps in place, as they are still needed to > check for mismatched systems where one CPU has the architected algorithm > but another has the IMP DEF algorithm. > > Note, the meta-capabilities were present before but were removed in > commits a56005d32105 ("arm64: cpufeature: Reduce number of pointer auth > CPU caps from 6 to 4") and 1e013d06120c ("arm64: cpufeature: Rework ptr > auth hwcaps using multi_entry_cap_matches"), as they were not needed > then. Note, unlike before, the current patch checks the cpucap values > directly, instead of reading the CPU ID register value. > > Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com> This patch as such looks fine to me. However, do we really make use of the individual caps for ARCH/IMPDEF support ? Do we plan to do something about them in the future ? If not we could as well remove them and have the generic ones in place. That may be done in a separate series as a cleanup. Either way, for this patch: Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
On 30/05/2019 11:50, Suzuki K Poulose wrote: > Hi Kristina, > > On 05/29/2019 08:03 PM, Kristina Martsenko wrote: >> To enable pointer auth for the kernel, we're going to need to check for >> the presence of address auth and generic auth using alternative_if. We >> currently have two cpucaps for each, but alternative_if needs to check a >> single cpucap. So define meta-capabilities that are present when either >> of the current two capabilities is present. >> >> Leave the existing four cpucaps in place, as they are still needed to >> check for mismatched systems where one CPU has the architected algorithm >> but another has the IMP DEF algorithm. >> >> Note, the meta-capabilities were present before but were removed in >> commits a56005d32105 ("arm64: cpufeature: Reduce number of pointer auth >> CPU caps from 6 to 4") and 1e013d06120c ("arm64: cpufeature: Rework ptr >> auth hwcaps using multi_entry_cap_matches"), as they were not needed >> then. Note, unlike before, the current patch checks the cpucap values >> directly, instead of reading the CPU ID register value. >> >> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com> > > This patch as such looks fine to me. However, do we really make use of > the individual caps for ARCH/IMPDEF support ? Do we plan to do something > about them in the future ? If not we could as well remove them and have > the generic ones in place. That may be done in a separate series as a > cleanup. I take that back. I think both are needed to make sure the secondaries match what the Boot CPU initially reported it has. With a generic cap, we don't have a reliable way to verify against the boot CPU. Sorry for the noise. This makes sense when we make it a boot CPU feature. > > Either way, for this patch: > > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com> > This still holds. Cheers Suzuki
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index f6a76e43f39e..601183b7b484 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -61,7 +61,9 @@ #define ARM64_HAS_GENERIC_AUTH_ARCH 40 #define ARM64_HAS_GENERIC_AUTH_IMP_DEF 41 #define ARM64_HAS_IRQ_PRIO_MASKING 42 +#define ARM64_HAS_ADDRESS_AUTH 43 +#define ARM64_HAS_GENERIC_AUTH 44 -#define ARM64_NCAPS 43 +#define ARM64_NCAPS 45 #endif /* __ASM_CPUCAPS_H */ diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index e505e1fbd2b9..0522ea674253 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -605,15 +605,13 @@ static inline bool system_supports_cnp(void) static inline bool system_supports_address_auth(void) { return IS_ENABLED(CONFIG_ARM64_PTR_AUTH) && - (cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH_ARCH) || - cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH_IMP_DEF)); + cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH); } static inline bool system_supports_generic_auth(void) { return IS_ENABLED(CONFIG_ARM64_PTR_AUTH) && - (cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH_ARCH) || - cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH_IMP_DEF)); + cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH); } static inline bool system_uses_irq_prio_masking(void) diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 4061de10cea6..166584deaed2 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -1205,6 +1205,20 @@ static void cpu_enable_address_auth(struct arm64_cpu_capabilities const *cap) sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | SCTLR_ELx_ENDA | SCTLR_ELx_ENDB); } + +static bool has_address_auth(const struct arm64_cpu_capabilities *entry, + int __unused) +{ + return cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH_ARCH) || + cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH_IMP_DEF); +} + +static bool has_generic_auth(const struct arm64_cpu_capabilities *entry, + int __unused) +{ + return cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH_ARCH) || + cpus_have_const_cap(ARM64_HAS_GENERIC_AUTH_IMP_DEF); +} #endif /* CONFIG_ARM64_PTR_AUTH */ #ifdef CONFIG_ARM64_PSEUDO_NMI @@ -1466,7 +1480,6 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .field_pos = ID_AA64ISAR1_APA_SHIFT, .min_field_value = ID_AA64ISAR1_APA_ARCHITECTED, .matches = has_cpuid_feature, - .cpu_enable = cpu_enable_address_auth, }, { .desc = "Address authentication (IMP DEF algorithm)", @@ -1477,6 +1490,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .field_pos = ID_AA64ISAR1_API_SHIFT, .min_field_value = ID_AA64ISAR1_API_IMP_DEF, .matches = has_cpuid_feature, + }, + { + .capability = ARM64_HAS_ADDRESS_AUTH, + .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .matches = has_address_auth, .cpu_enable = cpu_enable_address_auth, }, { @@ -1499,6 +1517,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .min_field_value = ID_AA64ISAR1_GPI_IMP_DEF, .matches = has_cpuid_feature, }, + { + .capability = ARM64_HAS_GENERIC_AUTH, + .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .matches = has_generic_auth, + }, #endif /* CONFIG_ARM64_PTR_AUTH */ #ifdef CONFIG_ARM64_PSEUDO_NMI {
To enable pointer auth for the kernel, we're going to need to check for the presence of address auth and generic auth using alternative_if. We currently have two cpucaps for each, but alternative_if needs to check a single cpucap. So define meta-capabilities that are present when either of the current two capabilities is present. Leave the existing four cpucaps in place, as they are still needed to check for mismatched systems where one CPU has the architected algorithm but another has the IMP DEF algorithm. Note, the meta-capabilities were present before but were removed in commits a56005d32105 ("arm64: cpufeature: Reduce number of pointer auth CPU caps from 6 to 4") and 1e013d06120c ("arm64: cpufeature: Rework ptr auth hwcaps using multi_entry_cap_matches"), as they were not needed then. Note, unlike before, the current patch checks the cpucap values directly, instead of reading the CPU ID register value. Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com> --- Changes since RFC v1: - New patch, as the meta-capabilities have been removed upstream arch/arm64/include/asm/cpucaps.h | 4 +++- arch/arm64/include/asm/cpufeature.h | 6 ++---- arch/arm64/kernel/cpufeature.c | 25 ++++++++++++++++++++++++- 3 files changed, 29 insertions(+), 6 deletions(-)