[PATCH] powerpc/security: Fix Speculation_Store_Bypass reporting on Power10

[PATCH] powerpc/security: Fix Speculation_Store_Bypass reporting on Power10

[Michael Ellerman]

Nageswara reported that /proc/self/status was showing "vulnerable" for
the Speculation_Store_Bypass feature on Power10, eg:

  $ grep Speculation_Store_Bypass: /proc/self/status
  Speculation_Store_Bypass:       vulnerable

But at the same time the sysfs files, and lscpu, were showing "Not
affected".

This turns out to simply be a bug in the reporting of the
Speculation_Store_Bypass, aka. PR_SPEC_STORE_BYPASS, case.

When SEC_FTR_STF_BARRIER was added, so that firmware could communicate
the vulnerability was not present, the code in ssb_prctl_get() was not
updated to check the new flag.

So add the check for SEC_FTR_STF_BARRIER being disabled. Rather than
adding the new check to the existing if block and expanding the comment
to cover both cases, rewrite the three cases to be separate so they can
be commented separately for clarity.

Fixes: 84ed26fd00c5 ("powerpc/security: Add a security feature for STF barrier")
Cc: stable@vger.kernel.org # v5.14+
Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/security.c | 37 +++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 206475e3e0b4..4856e1a5161c 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -364,26 +364,27 @@ ssize_t cpu_show_spec_store_bypass(struct device *dev, struct device_attribute *
 
 static int ssb_prctl_get(struct task_struct *task)
 {
+	/*
+	 * The STF_BARRIER feature is on by default, so if it's off that means
+	 * firmware has explicitly said the CPU is not vulnerable via either
+	 * the hypercall or device tree.
+	 */
+	if (!security_ftr_enabled(SEC_FTR_STF_BARRIER))
+		return PR_SPEC_NOT_AFFECTED;
+
+	/*
+	 * If the system's CPU has no known barrier (see setup_stf_barrier())
+	 * then assume that the CPU is not vulnerable.
+	 */
 	if (stf_enabled_flush_types == STF_BARRIER_NONE)
-		/*
-		 * We don't have an explicit signal from firmware that we're
-		 * vulnerable or not, we only have certain CPU revisions that
-		 * are known to be vulnerable.
-		 *
-		 * We assume that if we're on another CPU, where the barrier is
-		 * NONE, then we are not vulnerable.
-		 */
 		return PR_SPEC_NOT_AFFECTED;
-	else
-		/*
-		 * If we do have a barrier type then we are vulnerable. The
-		 * barrier is not a global or per-process mitigation, so the
-		 * only value we can report here is PR_SPEC_ENABLE, which
-		 * appears as "vulnerable" in /proc.
-		 */
-		return PR_SPEC_ENABLE;
-
-	return -EINVAL;
+
+	/*
+	 * Otherwise the CPU is vulnerable. The barrier is not a global or
+	 * per-process mitigation, so the only value that can be reported here
+	 * is PR_SPEC_ENABLE, which appears as "vulnerable" in /proc.
+	 */
+	return PR_SPEC_ENABLE;
 }
 
 int arch_prctl_spec_ctrl_get(struct task_struct *task, unsigned long which)
-- 
2.40.1

Re: [PATCH] powerpc/security: Fix Speculation_Store_Bypass reporting on Power10

[R Nageswara Sastry]

On 17/05/23 1:19 pm, Michael Ellerman wrote:
> Nageswara reported that /proc/self/status was showing "vulnerable" for
> the Speculation_Store_Bypass feature on Power10, eg:
> 
>    $ grep Speculation_Store_Bypass: /proc/self/status
>    Speculation_Store_Bypass:       vulnerable
> 
> But at the same time the sysfs files, and lscpu, were showing "Not
> affected".
> 
> This turns out to simply be a bug in the reporting of the
> Speculation_Store_Bypass, aka. PR_SPEC_STORE_BYPASS, case.
> 
> When SEC_FTR_STF_BARRIER was added, so that firmware could communicate
> the vulnerability was not present, the code in ssb_prctl_get() was not
> updated to check the new flag.
> 
> So add the check for SEC_FTR_STF_BARRIER being disabled. Rather than
> adding the new check to the existing if block and expanding the comment
> to cover both cases, rewrite the three cases to be separate so they can
> be commented separately for clarity.
> 
> Fixes: 84ed26fd00c5 ("powerpc/security: Add a security feature for STF barrier")
> Cc: stable@vger.kernel.org # v5.14+
> Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Thanks for the patch. Adding tested-by tag along with test results.

With out patch:
# grep Speculation_Store_Bypass: /proc/self/status
Speculation_Store_Bypass:	vulnerable
# uname -r
6.4.0-rc2

With patch:
# grep Speculation_Store_Bypass: /proc/self/status
Speculation_Store_Bypass:	not vulnerable
# uname -r
6.4.0-rc2

Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>


> ---
>   arch/powerpc/kernel/security.c | 37 +++++++++++++++++-----------------
>   1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
> index 206475e3e0b4..4856e1a5161c 100644
> --- a/arch/powerpc/kernel/security.c
> +++ b/arch/powerpc/kernel/security.c
> @@ -364,26 +364,27 @@ ssize_t cpu_show_spec_store_bypass(struct device *dev, struct device_attribute *
>   
>   static int ssb_prctl_get(struct task_struct *task)
>   {
> +	/*
> +	 * The STF_BARRIER feature is on by default, so if it's off that means
> +	 * firmware has explicitly said the CPU is not vulnerable via either
> +	 * the hypercall or device tree.
> +	 */
> +	if (!security_ftr_enabled(SEC_FTR_STF_BARRIER))
> +		return PR_SPEC_NOT_AFFECTED;
> +
> +	/*
> +	 * If the system's CPU has no known barrier (see setup_stf_barrier())
> +	 * then assume that the CPU is not vulnerable.
> +	 */
>   	if (stf_enabled_flush_types == STF_BARRIER_NONE)
> -		/*
> -		 * We don't have an explicit signal from firmware that we're
> -		 * vulnerable or not, we only have certain CPU revisions that
> -		 * are known to be vulnerable.
> -		 *
> -		 * We assume that if we're on another CPU, where the barrier is
> -		 * NONE, then we are not vulnerable.
> -		 */
>   		return PR_SPEC_NOT_AFFECTED;
> -	else
> -		/*
> -		 * If we do have a barrier type then we are vulnerable. The
> -		 * barrier is not a global or per-process mitigation, so the
> -		 * only value we can report here is PR_SPEC_ENABLE, which
> -		 * appears as "vulnerable" in /proc.
> -		 */
> -		return PR_SPEC_ENABLE;
> -
> -	return -EINVAL;
> +
> +	/*
> +	 * Otherwise the CPU is vulnerable. The barrier is not a global or
> +	 * per-process mitigation, so the only value that can be reported here
> +	 * is PR_SPEC_ENABLE, which appears as "vulnerable" in /proc.
> +	 */
> +	return PR_SPEC_ENABLE;
>   }
>   
>   int arch_prctl_spec_ctrl_get(struct task_struct *task, unsigned long which)

-- 
Thanks and Regards
R.Nageswara Sastry

Re: [PATCH] powerpc/security: Fix Speculation_Store_Bypass reporting on Power10

[Russell Currey]

On Wed, 2023-05-17 at 17:49 +1000, Michael Ellerman wrote:
> Nageswara reported that /proc/self/status was showing "vulnerable"
> for
> the Speculation_Store_Bypass feature on Power10, eg:
> 
>   $ grep Speculation_Store_Bypass: /proc/self/status
>   Speculation_Store_Bypass:       vulnerable
> 
> But at the same time the sysfs files, and lscpu, were showing "Not
> affected".
> 
> This turns out to simply be a bug in the reporting of the
> Speculation_Store_Bypass, aka. PR_SPEC_STORE_BYPASS, case.
> 
> When SEC_FTR_STF_BARRIER was added, so that firmware could
> communicate
> the vulnerability was not present, the code in ssb_prctl_get() was
> not
> updated to check the new flag.
> 
> So add the check for SEC_FTR_STF_BARRIER being disabled. Rather than
> adding the new check to the existing if block and expanding the
> comment
> to cover both cases, rewrite the three cases to be separate so they
> can
> be commented separately for clarity.
> 
> Fixes: 84ed26fd00c5 ("powerpc/security: Add a security feature for
> STF barrier")
> Cc: stable@vger.kernel.org # v5.14+
> Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Reviewed-by: Russell Currey <ruscur@russell.cc>

Re: [PATCH] powerpc/security: Fix Speculation_Store_Bypass reporting on Power10

[Michael Ellerman]

On Wed, 17 May 2023 17:49:45 +1000, Michael Ellerman wrote:
> Nageswara reported that /proc/self/status was showing "vulnerable" for
> the Speculation_Store_Bypass feature on Power10, eg:
> 
>   $ grep Speculation_Store_Bypass: /proc/self/status
>   Speculation_Store_Bypass:       vulnerable
> 
> But at the same time the sysfs files, and lscpu, were showing "Not
> affected".
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc/security: Fix Speculation_Store_Bypass reporting on Power10
      https://git.kernel.org/powerpc/c/5bcedc5931e7bd6928a2d8207078d4cb476b3b55

cheers