[PATCH v2 0/7] add system vulnerability sysfs entries

[PATCH v2 0/7] add system vulnerability sysfs entries

[Jeremy Linton]

This series was originally by Mian Yousaf Kaukab.

Arm64 machines should be displaying a human readable
vulnerability status to speculative execution attacks in
/sys/devices/system/cpu/vulnerabilities 

This series enables that behavior by providing the expected
functions. Those functions expose the cpu errata and feature
states, as well as whether firmware is responding appropriately
to display the overall machine status. This means that in a
heterogeneous machine we will only claim the machine is mitigated
or safe if we are confident all booted cores are safe or
mitigated. Otherwise, we will display unknown or unsafe
depending on how much of the machine configuration can
be assured.

v1->v2:
	Add "Unknown" state to ABI/testing docs.
	Minor tweaks.
	
Jeremy Linton (3):
  sysfs/cpu: Add "Unknown" vulnerability state
  arm64: add sysfs vulnerability show for meltdown
  arm64: add sysfs vulnerability show for spectre v2

Mian Yousaf Kaukab (4):
  arm64: kpti: move check for non-vulnerable CPUs to a function
  arm64: add sysfs vulnerability show for spectre v1
  arm64: add sysfs vulnerability show for speculative store bypass
  arm64: enable generic CPU vulnerabilites support

 .../ABI/testing/sysfs-devices-system-cpu      |   1 +
 arch/arm64/Kconfig                            |   1 +
 arch/arm64/kernel/cpu_errata.c                | 107 +++++++++++++++++-
 arch/arm64/kernel/cpufeature.c                |  42 +++++--
 4 files changed, 138 insertions(+), 13 deletions(-)

-- 
2.17.2

[PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Jeremy Linton]

There is a lot of variation in the Arm ecosystem. Because of this,
there exist possible cases where the kernel cannot authoritatively
determine if a machine is vulnerable.

Rather than guess the vulnerability status in cases where
the mitigation is disabled or the firmware isn't responding
correctly, we need to display an "Unknown" state.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: David Woodhouse <dwmw@amazon.co.uk>
---
 Documentation/ABI/testing/sysfs-devices-system-cpu | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 9605dbd4b5b5..876103fddfa4 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -495,6 +495,7 @@ Description:	Information about CPU vulnerabilities
 		"Not affected"	  CPU is not affected by the vulnerability
 		"Vulnerable"	  CPU is affected and no mitigation in effect
 		"Mitigation: $M"  CPU is affected and mitigation $M is in effect
+		"Unknown"    	  The kernel is unable to make a determination
 
 		Details about the l1tf file can be found in
 		Documentation/admin-guide/l1tf.rst
-- 
2.17.2

[PATCH v2 2/7] arm64: kpti: move check for non-vulnerable CPUs to a function

[Jeremy Linton]

From: Mian Yousaf Kaukab <ykaukab@suse.de>

Add is_meltdown_safe() which is a whitelist of known safe cores.

Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>
[Moved location of function]
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/cpufeature.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 4f272399de89..ab784d7a0083 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -947,8 +947,7 @@ has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope)
 #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
 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,
-				int scope)
+static bool is_cpu_meltdown_safe(void)
 {
 	/* List of CPUs that are not vulnerable and don't need KPTI */
 	static const struct midr_range kpti_safe_list[] = {
@@ -962,6 +961,15 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 		MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
 		{ /* sentinel */ }
 	};
+	if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))
+		return true;
+
+	return false;
+}
+
+static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
+				int scope)
+{
 	char const *str = "command line option";
 
 	/*
@@ -985,8 +993,7 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
 		return true;
 
-	/* Don't force KPTI for CPUs that are not vulnerable */
-	if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))
+	if (is_cpu_meltdown_safe())
 		return false;
 
 	/* Defer to CPU feature registers */
-- 
2.17.2

[PATCH v2 3/7] arm64: add sysfs vulnerability show for meltdown

[Jeremy Linton]

Add a simple state machine which will track whether
all the online cores in a machine are vulnerable.

Once that is done we have a fairly authoritative view
of the machine vulnerability, which allows us to make a
judgment about machine safety if it hasn't been mitigated.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/cpufeature.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index ab784d7a0083..1f64cbff8456 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -944,6 +944,9 @@ has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope)
 	return has_cpuid_feature(entry, scope);
 }
 
+/* default value is invalid until unmap_kernel_at_el0() runs */
+static bool __meltdown_safe = true;
+
 #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
 static int __kpti_forced; /* 0: not forced, >0: forced on, <0: forced off */
 
@@ -972,6 +975,12 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 {
 	char const *str = "command line option";
 
+	bool meltdown_safe = is_cpu_meltdown_safe() ||
+		has_cpuid_feature(entry, scope);
+
+	if (!meltdown_safe)
+		__meltdown_safe = false;
+
 	/*
 	 * For reasons that aren't entirely clear, enabling KPTI on Cavium
 	 * ThunderX leads to apparent I-cache corruption of kernel text, which
@@ -993,11 +1002,7 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
 		return true;
 
-	if (is_cpu_meltdown_safe())
-		return false;
-
-	/* Defer to CPU feature registers */
-	return !has_cpuid_feature(entry, scope);
+	return !meltdown_safe;
 }
 
 static void
@@ -2065,3 +2070,17 @@ static int __init enable_mrs_emulation(void)
 }
 
 core_initcall(enable_mrs_emulation);
+
+#ifdef CONFIG_GENERIC_CPU_VULNERABILITIES
+ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	if (arm64_kernel_unmapped_at_el0())
+		return sprintf(buf, "Mitigation: KPTI\n");
+
+	if (IS_ENABLED(CONFIG_UNMAP_KERNEL_AT_EL0) && __meltdown_safe)
+		return sprintf(buf, "Not affected\n");
+
+	return sprintf(buf, "Unknown\n");
+}
+#endif
-- 
2.17.2

[PATCH v2 4/7] arm64: add sysfs vulnerability show for spectre v1

[Jeremy Linton]

From: Mian Yousaf Kaukab <ykaukab@suse.de>

spectre v1, has been mitigated, and the mitigation is
always active.

Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/cpu_errata.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 09ac548c9d44..2352955b1259 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -742,3 +742,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 	{
 	}
 };
+
+#ifdef CONFIG_GENERIC_CPU_VULNERABILITIES
+
+ssize_t cpu_show_spectre_v1(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	return sprintf(buf, "Mitigation: __user pointer sanitization\n");
+}
+
+#endif
-- 
2.17.2

[PATCH v2 5/7] arm64: add sysfs vulnerability show for spectre v2

[Jeremy Linton]

Add code to track whether all the cores in the machine are
vulnerable, and whether all the vulnerable cores have been
mitigated.

Once we have that information we can add the sysfs stub and
provide an accurate view of what is known about the machine.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/cpu_errata.c | 69 +++++++++++++++++++++++++++++++---
 1 file changed, 64 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 2352955b1259..96a55accefa9 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -109,6 +109,11 @@ cpu_enable_trap_ctr_access(const struct arm64_cpu_capabilities *__unused)
 
 atomic_t arm64_el2_vector_last_slot = ATOMIC_INIT(-1);
 
+#if defined(CONFIG_HARDEN_BRANCH_PREDICTOR) || defined(CONFIG_GENERIC_CPU_VULNERABILITIES)
+/* Track overall mitigation state. We are only mitigated if all cores are ok */
+static bool __hardenbp_enab = true;
+#endif
+
 #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
 #include <asm/mmu_context.h>
 #include <asm/cacheflush.h>
@@ -231,15 +236,19 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
 	if (!entry->matches(entry, SCOPE_LOCAL_CPU))
 		return;
 
-	if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
+	if (psci_ops.smccc_version == SMCCC_VERSION_1_0) {
+		__hardenbp_enab = false;
 		return;
+	}
 
 	switch (psci_ops.conduit) {
 	case PSCI_CONDUIT_HVC:
 		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 				  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
-		if ((int)res.a0 < 0)
+		if ((int)res.a0 < 0) {
+			__hardenbp_enab = false;
 			return;
+		}
 		cb = call_hvc_arch_workaround_1;
 		/* This is a guest, no need to patch KVM vectors */
 		smccc_start = NULL;
@@ -249,14 +258,17 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
 	case PSCI_CONDUIT_SMC:
 		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 				  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
-		if ((int)res.a0 < 0)
+		if ((int)res.a0 < 0) {
+			__hardenbp_enab = false;
 			return;
+		}
 		cb = call_smc_arch_workaround_1;
 		smccc_start = __smccc_workaround_1_smc_start;
 		smccc_end = __smccc_workaround_1_smc_end;
 		break;
 
 	default:
+		__hardenbp_enab = false;
 		return;
 	}
 
@@ -507,7 +519,36 @@ cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused)
 	.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,			\
 	CAP_MIDR_RANGE_LIST(midr_list)
 
-#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+#if defined(CONFIG_HARDEN_BRANCH_PREDICTOR) || \
+	defined(CONFIG_GENERIC_CPU_VULNERABILITIES)
+
+static enum { A64_SV2_UNSET, A64_SV2_SAFE, A64_SV2_UNSAFE } __spectrev2_safe = A64_SV2_UNSET;
+
+/*
+ * Track overall bp hardening for all heterogeneous cores in the machine.
+ * We are only considered "safe" if all booted cores are known safe.
+ */
+static bool __maybe_unused
+check_branch_predictor(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	bool is_vul;
+	bool has_csv2;
+	u64 pfr0;
+
+	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
+
+	is_vul = is_midr_in_range_list(read_cpuid_id(), entry->midr_range_list);
+
+	pfr0 = read_cpuid(ID_AA64PFR0_EL1);
+	has_csv2 = cpuid_feature_extract_unsigned_field(pfr0, ID_AA64PFR0_CSV2_SHIFT);
+
+	if (is_vul)
+		__spectrev2_safe = A64_SV2_UNSAFE;
+	else if (__spectrev2_safe == A64_SV2_UNSET && has_csv2)
+		__spectrev2_safe = A64_SV2_SAFE;
+
+	return is_vul;
+}
 
 /*
  * List of CPUs where we need to issue a psci call to
@@ -705,7 +746,9 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 	{
 		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,
 		.cpu_enable = enable_smccc_arch_workaround_1,
-		ERRATA_MIDR_RANGE_LIST(arm64_bp_harden_smccc_cpus),
+		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
+		.matches = check_branch_predictor,
+		.midr_range_list = arm64_bp_harden_smccc_cpus,
 	},
 #endif
 #ifdef CONFIG_HARDEN_EL2_VECTORS
@@ -751,4 +794,20 @@ ssize_t cpu_show_spectre_v1(struct device *dev, struct device_attribute *attr,
 	return sprintf(buf, "Mitigation: __user pointer sanitization\n");
 }
 
+ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	switch (__spectrev2_safe) {
+	case A64_SV2_SAFE:
+		return sprintf(buf, "Not affected\n");
+	case A64_SV2_UNSAFE:
+		if (__hardenbp_enab)
+			return sprintf(buf,
+				"Mitigation: Branch predictor hardening\n");
+		return sprintf(buf, "Vulnerable\n");
+	default:
+		return sprintf(buf, "Unknown\n");
+	}
+}
+
 #endif
-- 
2.17.2

[PATCH v2 6/7] arm64: add sysfs vulnerability show for speculative store bypass

[Jeremy Linton]

From: Mian Yousaf Kaukab <ykaukab@suse.de>

Return status based on ssbd_state and the arm64 SSBS feature.
Return string "Unknown" in case CONFIG_ARM64_SSBD is disabled
or arch workaround2 is not available in the firmware.

Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>
[Added SSBS logic]
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/cpu_errata.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 96a55accefa9..c962b5856c87 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -420,6 +420,7 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
 		ssbd_state = ARM64_SSBD_UNKNOWN;
 		return false;
 
+	/* machines with mixed mitigation requirements must not return this */
 	case SMCCC_RET_NOT_REQUIRED:
 		pr_info_once("%s mitigation not required\n", entry->desc);
 		ssbd_state = ARM64_SSBD_MITIGATED;
@@ -810,4 +811,31 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr,
 	}
 }
 
+ssize_t cpu_show_spec_store_bypass(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	/*
+	 *  Two assumptions: First, get_ssbd_state() reflects the worse case
+	 *  for hetrogenous machines, and that if SSBS is supported its
+	 *  supported by all cores.
+	 */
+	switch (arm64_get_ssbd_state()) {
+	case ARM64_SSBD_MITIGATED:
+		return sprintf(buf, "Not affected\n");
+
+	case ARM64_SSBD_KERNEL:
+	case ARM64_SSBD_FORCE_ENABLE:
+		if (cpus_have_cap(ARM64_SSBS))
+			return sprintf(buf, "Not affected\n");
+		return sprintf(buf,
+			"Mitigation: Speculative Store Bypass disabled\n");
+
+	case ARM64_SSBD_FORCE_DISABLE:
+		return sprintf(buf, "Vulnerable\n");
+
+	default: /* ARM64_SSBD_UNKNOWN*/
+		return sprintf(buf, "Unknown\n");
+	}
+}
+
 #endif
-- 
2.17.2

[PATCH v2 7/7] arm64: enable generic CPU vulnerabilites support

[Jeremy Linton]

From: Mian Yousaf Kaukab <ykaukab@suse.de>

Enable CPU vulnerabilty show functions for spectre_v1, spectre_v2,
meltdown and store-bypass.

Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a4168d366127..be9872ee1d61 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -88,6 +88,7 @@ config ARM64
 	select GENERIC_CLOCKEVENTS
 	select GENERIC_CLOCKEVENTS_BROADCAST
 	select GENERIC_CPU_AUTOPROBE
+	select GENERIC_CPU_VULNERABILITIES
 	select GENERIC_EARLY_IOREMAP
 	select GENERIC_IDLE_POLL_SETUP
 	select GENERIC_IRQ_MULTI_HANDLER
-- 
2.17.2

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Greg Kroah-Hartman]

On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
> There is a lot of variation in the Arm ecosystem. Because of this,
> there exist possible cases where the kernel cannot authoritatively
> determine if a machine is vulnerable.

Really?  Why not?  What keeps you from "knowing" this?  Can't the
developer of the chip tell you?

> Rather than guess the vulnerability status in cases where
> the mitigation is disabled or the firmware isn't responding
> correctly, we need to display an "Unknown" state.

Shouldn't "Unknown" really be the same thing as "Vulnerable"?  A user
should treat it the same way, "Unknown" makes it feel like "maybe I can
just ignore this and hope I really am safe", which is not a good idea at
all.

thanks,

greg k-h

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Dave Martin]

On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
> There is a lot of variation in the Arm ecosystem. Because of this,
> there exist possible cases where the kernel cannot authoritatively
> determine if a machine is vulnerable.
> 
> Rather than guess the vulnerability status in cases where
> the mitigation is disabled or the firmware isn't responding
> correctly, we need to display an "Unknown" state.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  Documentation/ABI/testing/sysfs-devices-system-cpu | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
> index 9605dbd4b5b5..876103fddfa4 100644
> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
> @@ -495,6 +495,7 @@ Description:	Information about CPU vulnerabilities
>  		"Not affected"	  CPU is not affected by the vulnerability
>  		"Vulnerable"	  CPU is affected and no mitigation in effect
>  		"Mitigation: $M"  CPU is affected and mitigation $M is in effect
> +		"Unknown"    	  The kernel is unable to make a determination

Do some of the "Unknown" cases arise from the vulnerability detection
code being compiled out of the kernel?

I wonder whether at least the detection support should be mandatory.
sysfs is not very useful as a standard vulnerability reporting interface
unless we make best efforts to always populate it with real information.


Also, does "Unknown" convey anything beyond what is indicated by the
sysfs entry being omitted altogether?

Cheers
---Dave

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Jeremy Linton]

On 01/03/2019 03:38 AM, Greg Kroah-Hartman wrote:
> On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
>> There is a lot of variation in the Arm ecosystem. Because of this,
>> there exist possible cases where the kernel cannot authoritatively
>> determine if a machine is vulnerable.
> 
> Really?  Why not?  What keeps you from "knowing" this?  Can't the
> developer of the chip tell you?

There tends to be a few cases, possibly incomplete white/black lists, 
firmware that isn't responding correctly, or the user didn't build in 
the code to check the mitigation (possibly because its an embedded 
system and they know its not vulnerable?).

I would hope that it is an exceptional case.

> 
>> Rather than guess the vulnerability status in cases where
>> the mitigation is disabled or the firmware isn't responding
>> correctly, we need to display an "Unknown" state.
> 
> Shouldn't "Unknown" really be the same thing as "Vulnerable"?  A user
> should treat it the same way, "Unknown" makes it feel like "maybe I can
> just ignore this and hope I really am safe", which is not a good idea at
> all.

I tend to agree its not clear what to do with "unknown".

OTOH, I think there is a hesitation to declare something vulnerable when 
it isn't. Meltdown for example, is fairly rare given that it currently 
only affects a few arm parts, so declaring someone vulnerable when they 
likely aren't is going to be just as difficult to explain.

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Jeremy Linton]

Hi,

On 01/03/2019 10:37 AM, Dave Martin wrote:
> On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
>> There is a lot of variation in the Arm ecosystem. Because of this,
>> there exist possible cases where the kernel cannot authoritatively
>> determine if a machine is vulnerable.
>>
>> Rather than guess the vulnerability status in cases where
>> the mitigation is disabled or the firmware isn't responding
>> correctly, we need to display an "Unknown" state.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Dave Hansen <dave.hansen@intel.com>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: David Woodhouse <dwmw@amazon.co.uk>
>> ---
>>   Documentation/ABI/testing/sysfs-devices-system-cpu | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
>> index 9605dbd4b5b5..876103fddfa4 100644
>> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
>> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
>> @@ -495,6 +495,7 @@ Description:	Information about CPU vulnerabilities
>>   		"Not affected"	  CPU is not affected by the vulnerability
>>   		"Vulnerable"	  CPU is affected and no mitigation in effect
>>   		"Mitigation: $M"  CPU is affected and mitigation $M is in effect
>> +		"Unknown"    	  The kernel is unable to make a determination
> 
> Do some of the "Unknown" cases arise from the vulnerability detection
> code being compiled out of the kernel?
>

Yes,

> I wonder whether at least the detection support should be mandatory.
> sysfs is not very useful as a standard vulnerability reporting interface
> unless we make best efforts to always populate it with real information. >
> 
> Also, does "Unknown" convey anything beyond what is indicated by the
> sysfs entry being omitted altogether?

I'm not sure about this one. I tend to think the "unknown" case 
encourages users that really want an answer to dig deeper and call their 
hardware/os/whoever to get an answer. I would tend to think that if the 
entry is missing it would tend to encourage the behavior that Greg KH 
mentions where the user assumes "hey the system doesn't have a sysfs 
entry for $VUNLERABILITY, that probably means that its not possible on 
the architecture".

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Greg Kroah-Hartman]

On Thu, Jan 03, 2019 at 10:38:16AM -0600, Jeremy Linton wrote:
> On 01/03/2019 03:38 AM, Greg Kroah-Hartman wrote:
> > On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
> > > There is a lot of variation in the Arm ecosystem. Because of this,
> > > there exist possible cases where the kernel cannot authoritatively
> > > determine if a machine is vulnerable.
> > 
> > Really?  Why not?  What keeps you from "knowing" this?  Can't the
> > developer of the chip tell you?
> 
> There tends to be a few cases, possibly incomplete white/black lists,

Then fix the lists :)

> firmware that isn't responding correctly, or the user didn't build in the
> code to check the mitigation (possibly because its an embedded system and
> they know its not vulnerable?).

If the firmware doesn't respond, that would imply it is vulnerable :)

And if the code isn't built in, again, it's vulnerable.

> I would hope that it is an exceptional case.

Then have the default be vulnerable, don't give people false hope.

> > > Rather than guess the vulnerability status in cases where
> > > the mitigation is disabled or the firmware isn't responding
> > > correctly, we need to display an "Unknown" state.
> > 
> > Shouldn't "Unknown" really be the same thing as "Vulnerable"?  A user
> > should treat it the same way, "Unknown" makes it feel like "maybe I can
> > just ignore this and hope I really am safe", which is not a good idea at
> > all.
> 
> I tend to agree its not clear what to do with "unknown".
> 
> OTOH, I think there is a hesitation to declare something vulnerable when it
> isn't. Meltdown for example, is fairly rare given that it currently only
> affects a few arm parts, so declaring someone vulnerable when they likely
> aren't is going to be just as difficult to explain.

If you know it is rare, then you know how to properly detect it so
"unknown" is not needed, correct?

Again, "unknown" is not going to help anyone out here, please don't do
it.

thanks,

greg k-h

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Stefan Wahren]

Hi Jeremy,

> Jeremy Linton <jeremy.linton@arm.com> hat am 3. Januar 2019 um 17:46 geschrieben:
> 
> 
> Hi,
> 
> On 01/03/2019 10:37 AM, Dave Martin wrote:
> > On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
> >> There is a lot of variation in the Arm ecosystem. Because of this,
> >> there exist possible cases where the kernel cannot authoritatively
> >> determine if a machine is vulnerable.
> >>
> >> Rather than guess the vulnerability status in cases where
> >> the mitigation is disabled or the firmware isn't responding
> >> correctly, we need to display an "Unknown" state.
> >>

i applied your patch series on linux-next-20190103. On my Raspberry Pi 3B+ (defconfig) i'm getting this from sysfs:

l1tf:Not affected
meltdown:Not affected
spec_store_bypass:Unknown
spectre_v1:Mitigation: __user pointer sanitization
spectre_v2:Unknown

AFAIK it has 4 Cortex-A53 cores (no PSCI firmware), so shouldn't be affected.

How can this be fixed?

Thanks
Stefan

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Jeremy Linton]

Hi,

On 01/03/2019 01:30 PM, Stefan Wahren wrote:
> Hi Jeremy,
> 
>> Jeremy Linton <jeremy.linton@arm.com> hat am 3. Januar 2019 um 17:46 geschrieben:
>>
>>
>> Hi,
>>
>> On 01/03/2019 10:37 AM, Dave Martin wrote:
>>> On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
>>>> There is a lot of variation in the Arm ecosystem. Because of this,
>>>> there exist possible cases where the kernel cannot authoritatively
>>>> determine if a machine is vulnerable.
>>>>
>>>> Rather than guess the vulnerability status in cases where
>>>> the mitigation is disabled or the firmware isn't responding
>>>> correctly, we need to display an "Unknown" state.
>>>>
> 
> i applied your patch series on linux-next-20190103. On my Raspberry Pi 3B+ (defconfig) i'm getting this from sysfs:
> 
> l1tf:Not affected
> meltdown:Not affected
> spec_store_bypass:Unknown
> spectre_v1:Mitigation: __user pointer sanitization
> spectre_v2:Unknown
> 
> AFAIK it has 4 Cortex-A53 cores (no PSCI firmware), so shouldn't be affected.

So, for spec_store_bypass, as you noted your getting hit by the lack of 
psci/smccc to report the ssb state, and this patch is just reflecting that.

In the case of spectrev2 it may be correct to blame this patch set 
because its displaying "unknown" since your core isn't in the black 
list, and your core isn't new enough to have the csv2 bit indicating its 
not vulnerable. In this case if we do away with the unknown state, we 
should probably depend entirely on the black list and simply display 
"Not affected" if the core isn't listed. (meaning we may report cores 
not affected when they are missing from the blacklist).


> How can this be fixed?

For ssb, the correct answer is probably fix the firmware, but given the 
situation, its likely this kind of machine is going to force an 
additional MIDR list to report the state correctly. Maybe Will or 
someone can chime in here?

For spectrev2, wait for another version of this patch.

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Will Deacon]

On Thu, Jan 03, 2019 at 02:32:44PM -0600, Jeremy Linton wrote:
> On 01/03/2019 01:30 PM, Stefan Wahren wrote:
> > > Jeremy Linton <jeremy.linton@arm.com> hat am 3. Januar 2019 um 17:46 geschrieben:
> > > On 01/03/2019 10:37 AM, Dave Martin wrote:
> > > > On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
> > > > > There is a lot of variation in the Arm ecosystem. Because of this,
> > > > > there exist possible cases where the kernel cannot authoritatively
> > > > > determine if a machine is vulnerable.
> > > > > 
> > > > > Rather than guess the vulnerability status in cases where
> > > > > the mitigation is disabled or the firmware isn't responding
> > > > > correctly, we need to display an "Unknown" state.
> > > > > 
> > 
> > i applied your patch series on linux-next-20190103. On my Raspberry Pi 3B+ (defconfig) i'm getting this from sysfs:
> > 
> > l1tf:Not affected
> > meltdown:Not affected
> > spec_store_bypass:Unknown
> > spectre_v1:Mitigation: __user pointer sanitization
> > spectre_v2:Unknown
> > 
> > AFAIK it has 4 Cortex-A53 cores (no PSCI firmware), so shouldn't be affected.
> 
> So, for spec_store_bypass, as you noted your getting hit by the lack of
> psci/smccc to report the ssb state, and this patch is just reflecting that.
> 
> In the case of spectrev2 it may be correct to blame this patch set because
> its displaying "unknown" since your core isn't in the black list, and your
> core isn't new enough to have the csv2 bit indicating its not vulnerable. In
> this case if we do away with the unknown state, we should probably depend
> entirely on the black list and simply display "Not affected" if the core
> isn't listed. (meaning we may report cores not affected when they are
> missing from the blacklist).
> 
> 
> > How can this be fixed?
> 
> For ssb, the correct answer is probably fix the firmware, but given the
> situation, its likely this kind of machine is going to force an additional
> MIDR list to report the state correctly. Maybe Will or someone can chime in
> here?

Marc Z is already working on this iirc, since we need it to fix the message
printed to dmesg about the mitigation status anyway.

Will

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Dave Martin]

On Thu, Jan 03, 2019 at 05:48:31PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Jan 03, 2019 at 10:38:16AM -0600, Jeremy Linton wrote:
> > On 01/03/2019 03:38 AM, Greg Kroah-Hartman wrote:
> > > On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
> > > > There is a lot of variation in the Arm ecosystem. Because of this,
> > > > there exist possible cases where the kernel cannot authoritatively
> > > > determine if a machine is vulnerable.
> > > 
> > > Really?  Why not?  What keeps you from "knowing" this?  Can't the
> > > developer of the chip tell you?
> > 
> > There tends to be a few cases, possibly incomplete white/black lists,
> 
> Then fix the lists :)
> 
> > firmware that isn't responding correctly, or the user didn't build in the
> > code to check the mitigation (possibly because its an embedded system and
> > they know its not vulnerable?).
> 
> If the firmware doesn't respond, that would imply it is vulnerable :)
> 
> And if the code isn't built in, again, it's vulnerable.
> 
> > I would hope that it is an exceptional case.
> 
> Then have the default be vulnerable, don't give people false hope.
> 
> > > > Rather than guess the vulnerability status in cases where
> > > > the mitigation is disabled or the firmware isn't responding
> > > > correctly, we need to display an "Unknown" state.
> > > 
> > > Shouldn't "Unknown" really be the same thing as "Vulnerable"?  A user
> > > should treat it the same way, "Unknown" makes it feel like "maybe I can
> > > just ignore this and hope I really am safe", which is not a good idea at
> > > all.
> > 
> > I tend to agree its not clear what to do with "unknown".
> > 
> > OTOH, I think there is a hesitation to declare something vulnerable when it
> > isn't. Meltdown for example, is fairly rare given that it currently only
> > affects a few arm parts, so declaring someone vulnerable when they likely
> > aren't is going to be just as difficult to explain.
> 
> If you know it is rare, then you know how to properly detect it so
> "unknown" is not needed, correct?
> 
> Again, "unknown" is not going to help anyone out here, please don't do
> it.

Thinking about it, "unknown" is actually the common case.

Kernels that predate the sysfs vulnerabilities interface effectively
report this for all vulnerabilities by omitting the sysfs entries
entirely.

Current kernels also don't know anything about future vulnerabilities
that may be added in sysfs later on (but which may nevertheless be
discovered subsequently to affect current hardware).

So, can we simply omit the sysfs entries for which we can't provide a
good answer?

IMHO the kernel should make best efforts to provide answers for every
vulnerability that it knows about, so the checks should not be Kconfig-
dependent without a good reason.

There will be cases where whitelists/blacklists are the only source of
answers, and we are ultimately reliant on vendors to provide that
information.  Upstream Linux is likely to lag, there's not much we can
do about that.

Cheers
---Dave

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Greg Kroah-Hartman]

On Fri, Jan 04, 2019 at 02:08:32PM +0000, Dave Martin wrote:
> On Thu, Jan 03, 2019 at 05:48:31PM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Jan 03, 2019 at 10:38:16AM -0600, Jeremy Linton wrote:
> > > On 01/03/2019 03:38 AM, Greg Kroah-Hartman wrote:
> > > > On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
> > > > > There is a lot of variation in the Arm ecosystem. Because of this,
> > > > > there exist possible cases where the kernel cannot authoritatively
> > > > > determine if a machine is vulnerable.
> > > > 
> > > > Really?  Why not?  What keeps you from "knowing" this?  Can't the
> > > > developer of the chip tell you?
> > > 
> > > There tends to be a few cases, possibly incomplete white/black lists,
> > 
> > Then fix the lists :)
> > 
> > > firmware that isn't responding correctly, or the user didn't build in the
> > > code to check the mitigation (possibly because its an embedded system and
> > > they know its not vulnerable?).
> > 
> > If the firmware doesn't respond, that would imply it is vulnerable :)
> > 
> > And if the code isn't built in, again, it's vulnerable.
> > 
> > > I would hope that it is an exceptional case.
> > 
> > Then have the default be vulnerable, don't give people false hope.
> > 
> > > > > Rather than guess the vulnerability status in cases where
> > > > > the mitigation is disabled or the firmware isn't responding
> > > > > correctly, we need to display an "Unknown" state.
> > > > 
> > > > Shouldn't "Unknown" really be the same thing as "Vulnerable"?  A user
> > > > should treat it the same way, "Unknown" makes it feel like "maybe I can
> > > > just ignore this and hope I really am safe", which is not a good idea at
> > > > all.
> > > 
> > > I tend to agree its not clear what to do with "unknown".
> > > 
> > > OTOH, I think there is a hesitation to declare something vulnerable when it
> > > isn't. Meltdown for example, is fairly rare given that it currently only
> > > affects a few arm parts, so declaring someone vulnerable when they likely
> > > aren't is going to be just as difficult to explain.
> > 
> > If you know it is rare, then you know how to properly detect it so
> > "unknown" is not needed, correct?
> > 
> > Again, "unknown" is not going to help anyone out here, please don't do
> > it.
> 
> Thinking about it, "unknown" is actually the common case.
> 
> Kernels that predate the sysfs vulnerabilities interface effectively
> report this for all vulnerabilities by omitting the sysfs entries
> entirely.
> 
> Current kernels also don't know anything about future vulnerabilities
> that may be added in sysfs later on (but which may nevertheless be
> discovered subsequently to affect current hardware).
> 
> So, can we simply omit the sysfs entries for which we can't provide a
> good answer?

As you say, we already do this for older systems.

But don't add new logic to explicitly not create the files just because
we "can not figure it out".  For those systems, I would default to
"vulnerable" as I think that's what we do today, right?

thanks,
g
reg k-h

Re: [PATCH v2 1/7] sysfs/cpu: Add "Unknown" vulnerability state

[Will Deacon]

On Fri, Jan 04, 2019 at 03:18:05PM +0100, Greg Kroah-Hartman wrote:
> On Fri, Jan 04, 2019 at 02:08:32PM +0000, Dave Martin wrote:
> > On Thu, Jan 03, 2019 at 05:48:31PM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Jan 03, 2019 at 10:38:16AM -0600, Jeremy Linton wrote:
> > > > On 01/03/2019 03:38 AM, Greg Kroah-Hartman wrote:
> > > > > On Wed, Jan 02, 2019 at 06:49:15PM -0600, Jeremy Linton wrote:
> > > > > > There is a lot of variation in the Arm ecosystem. Because of this,
> > > > > > there exist possible cases where the kernel cannot authoritatively
> > > > > > determine if a machine is vulnerable.
> > > > > 
> > > > > Really?  Why not?  What keeps you from "knowing" this?  Can't the
> > > > > developer of the chip tell you?
> > > > 
> > > > There tends to be a few cases, possibly incomplete white/black lists,
> > > 
> > > Then fix the lists :)
> > > 
> > > > firmware that isn't responding correctly, or the user didn't build in the
> > > > code to check the mitigation (possibly because its an embedded system and
> > > > they know its not vulnerable?).
> > > 
> > > If the firmware doesn't respond, that would imply it is vulnerable :)
> > > 
> > > And if the code isn't built in, again, it's vulnerable.
> > > 
> > > > I would hope that it is an exceptional case.
> > > 
> > > Then have the default be vulnerable, don't give people false hope.
> > > 
> > > > > > Rather than guess the vulnerability status in cases where
> > > > > > the mitigation is disabled or the firmware isn't responding
> > > > > > correctly, we need to display an "Unknown" state.
> > > > > 
> > > > > Shouldn't "Unknown" really be the same thing as "Vulnerable"?  A user
> > > > > should treat it the same way, "Unknown" makes it feel like "maybe I can
> > > > > just ignore this and hope I really am safe", which is not a good idea at
> > > > > all.
> > > > 
> > > > I tend to agree its not clear what to do with "unknown".
> > > > 
> > > > OTOH, I think there is a hesitation to declare something vulnerable when it
> > > > isn't. Meltdown for example, is fairly rare given that it currently only
> > > > affects a few arm parts, so declaring someone vulnerable when they likely
> > > > aren't is going to be just as difficult to explain.
> > > 
> > > If you know it is rare, then you know how to properly detect it so
> > > "unknown" is not needed, correct?
> > > 
> > > Again, "unknown" is not going to help anyone out here, please don't do
> > > it.
> > 
> > Thinking about it, "unknown" is actually the common case.
> > 
> > Kernels that predate the sysfs vulnerabilities interface effectively
> > report this for all vulnerabilities by omitting the sysfs entries
> > entirely.
> > 
> > Current kernels also don't know anything about future vulnerabilities
> > that may be added in sysfs later on (but which may nevertheless be
> > discovered subsequently to affect current hardware).
> > 
> > So, can we simply omit the sysfs entries for which we can't provide a
> > good answer?
> 
> As you say, we already do this for older systems.
> 
> But don't add new logic to explicitly not create the files just because
> we "can not figure it out".  For those systems, I would default to
> "vulnerable" as I think that's what we do today, right?

Nope: currently the vulnerabilities directory doesn't even exist for arm64
because we don't select GENERIC_CPU_VULNERABILITIES.

There are also a few other things to consider here:

  1. The action to take as an end-user is slightly different in the case
     that you know for sure that your system is vulnerable, as opposed to
     the case that you don't know whether your system is vulnerable or not.
     The former needs a firmware update; the second needs a statement about
     the CPU, which could result in a simple whitelist update in Linux.

  2. There's an unfortunate political angle to this. Whilst the Arm website
     [1] provides information for all of the Arm-designed CPUs (i.e.
     Cortex-A*), it doesn't comment on partner implementations. I'm not at
     all keen to be seen as branding them all as vulnerable in the Linux
     kernel, as this is likely to cause more problems than it solves.
     If we had complete whitelist information available in public, that
     would be ideal, but it's not the case.

  3. The architecture has added some ID registers to determine if a CPU
     is affected by Spectre and Meltdown, so a whitelist only needs to
     cover existing CPUs.

So I agree with Dave that continuing to omit the files when we don't know
whether or not the system is affected is the right thing to do.

Will

[1] https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability