From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: amit.kachhap@arm.com, linux-arm-kernel@lists.infradead.org
Cc: mark.rutland@arm.com, keescook@chromium.org,
catalin.marinas@arm.com, broonie@kernel.org, james.morse@arm.com,
Vincenzo.Frascino@arm.com, will@kernel.org, dave.martin@arm.com
Subject: Re: [PATCH v5 4/6] arm64: cpufeature: Modify address authentication cpufeature to exact
Date: Wed, 2 Sep 2020 15:05:46 +0100 [thread overview]
Message-ID: <ef64fef4-c23f-400c-d2d8-0df80f4492b7@arm.com> (raw)
In-Reply-To: <1597734671-23407-5-git-send-email-amit.kachhap@arm.com>
Hi Amit,
On 08/18/2020 08:11 AM, Amit Daniel Kachhap wrote:
> The current address authentication cpufeature levels are set as LOWER_SAFE
> which is not compatible with the different configurations added for Armv8.3
> ptrauth enhancements as the different levels have different behaviour and
> there is no tunable to enable the lower safe versions. This is rectified
> by setting those cpufeature type as EXACT.
>
> The current cpufeature framework also does not interfere in the booting of
> non-exact secondary cpus but rather marks them as tainted. As a workaround
> this is fixed by replacing the generic match handler with a new handler
> specific to ptrauth.
>
> After this change, if there is any variation in ptrauth configurations in
> secondary cpus from boot cpu then those mismatched cpus are parked in an
> infinite loop.
>
> Following ptrauth crash log is oberserved in Arm fastmodel with mismatched
> cpus without this fix,
>
> CPU features: SANITY CHECK: Unexpected variation in SYS_ID_AA64ISAR1_EL1. Boot CPU: 0x11111110211402, CPU4: 0x11111110211102
> CPU features: Unsupported CPU feature variation detected.
> GICv3: CPU4: found redistributor 100 region 0:0x000000002f180000
> CPU4: Booted secondary processor 0x0000000100 [0x410fd0f0]
> Unable to handle kernel paging request at virtual address bfff800010dadf3c
> Mem abort info:
> ESR = 0x86000004
> EC = 0x21: IABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> [bfff800010dadf3c] address between user and kernel address ranges
> Internal error: Oops: 86000004 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 4 PID: 29 Comm: migration/4 Tainted: G S 5.8.0-rc4-00005-ge658591d66d1-dirty #158
> Hardware name: Foundation-v8A (DT)
> pstate: 60000089 (nZCv daIf -PAN -UAO BTYPE=--)
> pc : 0xbfff800010dadf3c
> lr : __schedule+0x2b4/0x5a8
> sp : ffff800012043d70
> x29: ffff800012043d70 x28: 0080000000000000
> x27: ffff800011cbe000 x26: ffff00087ad37580
> x25: ffff00087ad37000 x24: ffff800010de7d50
> x23: ffff800011674018 x22: 0784800010dae2a8
> x21: ffff00087ad37000 x20: ffff00087acb8000
> x19: ffff00087f742100 x18: 0000000000000030
> x17: 0000000000000000 x16: 0000000000000000
> x15: ffff800011ac1000 x14: 00000000000001bd
> x13: 0000000000000000 x12: 0000000000000000
> x11: 0000000000000000 x10: 71519a147ddfeb82
> x9 : 825d5ec0fb246314 x8 : ffff00087ad37dd8
> x7 : 0000000000000000 x6 : 00000000fffedb0e
> x5 : 00000000ffffffff x4 : 0000000000000000
> x3 : 0000000000000028 x2 : ffff80086e11e000
> x1 : ffff00087ad37000 x0 : ffff00087acdc600
> Call trace:
> 0xbfff800010dadf3c
> schedule+0x78/0x110
> schedule_preempt_disabled+0x24/0x40
> __kthread_parkme+0x68/0xd0
> kthread+0x138/0x160
> ret_from_fork+0x10/0x34
> Code: bad PC value
>
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
> [Suzuki: Introduce new matching function for address authentication]
> Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> Changes since v4:
> * Added crash log in case of mismatched cpus. This was requested by
> Will earlier [1].
> * Used sanitised register instead of cached static variables. This was
> suggested by Dave [2].
>
> [1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2020-June/579526.html
> [2]: http://lists.infradead.org/pipermail/linux-arm-kernel/2020-July/589712.html
>
> arch/arm64/kernel/cpufeature.c | 43 +++++++++++++++++++++++++++-------
> 1 file changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9fae0efc80c1..2e11be019475 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -210,9 +210,9 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = {
> ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_FCMA_SHIFT, 4, 0),
> ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_JSCVT_SHIFT, 4, 0),
> ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
> - FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_API_SHIFT, 4, 0),
> + FTR_STRICT, FTR_EXACT, ID_AA64ISAR1_API_SHIFT, 4, 0),
> ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
> - FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_APA_SHIFT, 4, 0),
> + FTR_STRICT, FTR_EXACT, ID_AA64ISAR1_APA_SHIFT, 4, 0),
> ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_DPB_SHIFT, 4, 0),
> ARM64_FTR_END,
> };
> @@ -1605,11 +1605,36 @@ static void cpu_clear_disr(const struct arm64_cpu_capabilities *__unused)
> #endif /* CONFIG_ARM64_RAS_EXTN */
>
> #ifdef CONFIG_ARM64_PTR_AUTH
> -static bool has_address_auth(const struct arm64_cpu_capabilities *entry,
> - int __unused)
> +static bool has_address_auth_cpucap(const struct arm64_cpu_capabilities *entry, int scope)
> {
> - return __system_matches_cap(ARM64_HAS_ADDRESS_AUTH_ARCH) ||
> - __system_matches_cap(ARM64_HAS_ADDRESS_AUTH_IMP_DEF);
> + int boot_val, sec_val;
> +
> + /* We don't expect to be called with SCOPE_SYSTEM */
> + WARN_ON(scope == SCOPE_SYSTEM);
> + /*
> + * The ptr-auth feature levels are not intercompatible with lower
> + * levels. Hence we must match ptr-auth feature level of the secondary
> + * CPUs with that of the boot CPU. The level of boot cpu is fetched
> + * from the sanitised register whereas direct register read is done for
> + * the secondary CPUs.
You may want to extend the comment to say:
The sanitised feature state is guaranteed to match that of the
boot CPU as a mismatched secondary CPU is parked before it gets
a chance to update the state, with the capability.
> + */
> + boot_val = cpuid_feature_extract_field(read_sanitised_ftr_reg(entry->sys_reg),
> + entry->field_pos, entry->sign);
> + if (scope & SCOPE_BOOT_CPU) {
> + return boot_val >= entry->min_field_value;
> + } else if (scope & SCOPE_LOCAL_CPU) {
> + sec_val = cpuid_feature_extract_field(__read_sysreg_by_encoding(entry->sys_reg),
> +
> + return (sec_val >= entry->min_field_value) && (sec_val == boot_val);
> + }
> + return false;
> +}
> +
> +static bool has_address_auth_metacap(const struct arm64_cpu_capabilities *entry,
> + int scope)
> +{
> + return has_address_auth_cpucap(cpu_hwcaps_ptrs[ARM64_HAS_ADDRESS_AUTH_ARCH], scope) ||
> + has_address_auth_cpucap(cpu_hwcaps_ptrs[ARM64_HAS_ADDRESS_AUTH_IMP_DEF], scope);
> }
>
> static bool has_generic_auth(const struct arm64_cpu_capabilities *entry,
> @@ -1958,7 +1983,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> .sign = FTR_UNSIGNED,
> .field_pos = ID_AA64ISAR1_APA_SHIFT,
> .min_field_value = ID_AA64ISAR1_APA_ARCHITECTED,
> - .matches = has_cpuid_feature,
> + .matches = has_address_auth_cpucap,
> },
> {
> .desc = "Address authentication (IMP DEF algorithm)",
> @@ -1968,12 +1993,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> .sign = FTR_UNSIGNED,
> .field_pos = ID_AA64ISAR1_API_SHIFT,
> .min_field_value = ID_AA64ISAR1_API_IMP_DEF,
> - .matches = has_cpuid_feature,
> + .matches = has_address_auth_cpucap,
> },
> {
> .capability = ARM64_HAS_ADDRESS_AUTH,
> .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
> - .matches = has_address_auth,
> + .matches = has_address_auth_metacap,
> },
> {
With that:
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-09-02 14:02 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-18 7:11 [PATCH v5 0/6] arm64: add Armv8.3 pointer authentication enhancements Amit Daniel Kachhap
2020-08-18 7:11 ` [PATCH v5 1/6] arm64: kprobe: add checks for ARMv8.3-PAuth combined instructions Amit Daniel Kachhap
2020-09-02 15:39 ` Dave Martin
2020-09-04 6:15 ` Amit Kachhap
2020-08-18 7:11 ` [PATCH v5 2/6] arm64: traps: Allow force_signal_inject to pass esr error code Amit Daniel Kachhap
2020-09-02 15:39 ` Dave Martin
2020-08-18 7:11 ` [PATCH v5 3/6] arm64: ptrauth: Introduce Armv8.3 pointer authentication enhancements Amit Daniel Kachhap
2020-09-02 15:39 ` Dave Martin
2020-08-18 7:11 ` [PATCH v5 4/6] arm64: cpufeature: Modify address authentication cpufeature to exact Amit Daniel Kachhap
2020-09-02 14:05 ` Suzuki K Poulose [this message]
2020-08-18 7:11 ` [PATCH v5 5/6] arm64: kprobe: disable probe of fault prone ptrauth instruction Amit Daniel Kachhap
2020-08-18 7:11 ` [PATCH v5 6/6] arm64: kprobe: clarify the comment of steppable hint instructions Amit Daniel Kachhap
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ef64fef4-c23f-400c-d2d8-0df80f4492b7@arm.com \
--to=suzuki.poulose@arm.com \
--cc=Vincenzo.Frascino@arm.com \
--cc=amit.kachhap@arm.com \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=dave.martin@arm.com \
--cc=james.morse@arm.com \
--cc=keescook@chromium.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.