[PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Alejandro Jimenez]

The bits set in x86_spec_ctrl_mask are used to calculate the
guest's value of SPEC_CTRL that is written to the MSR before
VMENTRY, and control which mitigations the guest can enable.
In the case of SSBD, unless the host has enabled SSBD always
on mode (by passing "spec_store_bypass_disable=on" in the
kernel parameters), the SSBD bit is not set in the mask and
the guest can not properly enable the SSBD always on
mitigation mode.

This is confirmed by running the SSBD PoC on a guest using
the SSBD always on mitigation mode (booted with kernel
parameter "spec_store_bypass_disable=on"), and verifying
that the guest is vulnerable unless the host is also using
SSBD always on mode. In addition, the guest OS incorrectly
reports the SSB vulnerability as mitigated.

Always set the SSBD bit in x86_spec_ctrl_mask when the host
CPU supports it, allowing the guest to use SSBD whether or
not the host has chosen to enable the mitigation in any of
its modes.

Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
Cc: stable@vger.kernel.org
---
 arch/x86/kernel/cpu/bugs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 03b4cc0..66ca906 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -836,6 +836,16 @@ static enum ssb_mitigation __init __ssb_select_mitigation(void)
 	}
 
 	/*
+	 * If SSBD is controlled by the SPEC_CTRL MSR, then set the proper
+	 * bit in the mask to allow guests to use the mitigation even in the
+	 * case where the host does not enable it.
+	 */
+	if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) ||
+	    static_cpu_has(X86_FEATURE_AMD_SSBD)) {
+		x86_spec_ctrl_mask |= SPEC_CTRL_SSBD;
+	}
+
+	/*
 	 * We have three CPU feature flags that are in play here:
 	 *  - X86_BUG_SPEC_STORE_BYPASS - CPU is susceptible.
 	 *  - X86_FEATURE_SSBD - CPU is able to turn off speculative store bypass
@@ -852,7 +862,6 @@ static enum ssb_mitigation __init __ssb_select_mitigation(void)
 			x86_amd_ssb_disable();
 		} else {
 			x86_spec_ctrl_base |= SPEC_CTRL_SSBD;
-			x86_spec_ctrl_mask |= SPEC_CTRL_SSBD;
 			wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
 		}
 	}
-- 
1.8.3.1

Re: [PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Mark Kanda]

On 6/10/2019 12:20 PM, Alejandro Jimenez wrote:
> The bits set in x86_spec_ctrl_mask are used to calculate the
> guest's value of SPEC_CTRL that is written to the MSR before
> VMENTRY, and control which mitigations the guest can enable.
> In the case of SSBD, unless the host has enabled SSBD always
> on mode (by passing "spec_store_bypass_disable=on" in the
> kernel parameters), the SSBD bit is not set in the mask and
> the guest can not properly enable the SSBD always on
> mitigation mode.
> 
> This is confirmed by running the SSBD PoC on a guest using
> the SSBD always on mitigation mode (booted with kernel
> parameter "spec_store_bypass_disable=on"), and verifying
> that the guest is vulnerable unless the host is also using
> SSBD always on mode. In addition, the guest OS incorrectly
> reports the SSB vulnerability as mitigated.
> 
> Always set the SSBD bit in x86_spec_ctrl_mask when the host
> CPU supports it, allowing the guest to use SSBD whether or
> not the host has chosen to enable the mitigation in any of
> its modes.
> 
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> Reviewed-by: Liam Merwick <liam.merwick@oracle.com>

Reviewed-by: Mark Kanda <mark.kanda@oracle.com>

> Cc: stable@vger.kernel.org
> ---
>   arch/x86/kernel/cpu/bugs.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 03b4cc0..66ca906 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -836,6 +836,16 @@ static enum ssb_mitigation __init __ssb_select_mitigation(void)
>   	}
>   
>   	/*
> +	 * If SSBD is controlled by the SPEC_CTRL MSR, then set the proper
> +	 * bit in the mask to allow guests to use the mitigation even in the
> +	 * case where the host does not enable it.
> +	 */
> +	if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) ||
> +	    static_cpu_has(X86_FEATURE_AMD_SSBD)) {
> +		x86_spec_ctrl_mask |= SPEC_CTRL_SSBD;
> +	}
> +
> +	/*
>   	 * We have three CPU feature flags that are in play here:
>   	 *  - X86_BUG_SPEC_STORE_BYPASS - CPU is susceptible.
>   	 *  - X86_FEATURE_SSBD - CPU is able to turn off speculative store bypass
> @@ -852,7 +862,6 @@ static enum ssb_mitigation __init __ssb_select_mitigation(void)
>   			x86_amd_ssb_disable();
>   		} else {
>   			x86_spec_ctrl_base |= SPEC_CTRL_SSBD;
> -			x86_spec_ctrl_mask |= SPEC_CTRL_SSBD;
>   			wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
>   		}
>   	}
> 

Re: [PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Paolo Bonzini]

On 10/06/19 19:20, Alejandro Jimenez wrote:
> The bits set in x86_spec_ctrl_mask are used to calculate the
> guest's value of SPEC_CTRL that is written to the MSR before
> VMENTRY, and control which mitigations the guest can enable.
> In the case of SSBD, unless the host has enabled SSBD always
> on mode (by passing "spec_store_bypass_disable=on" in the
> kernel parameters), the SSBD bit is not set in the mask and
> the guest can not properly enable the SSBD always on
> mitigation mode.
> 
> This is confirmed by running the SSBD PoC on a guest using
> the SSBD always on mitigation mode (booted with kernel
> parameter "spec_store_bypass_disable=on"), and verifying
> that the guest is vulnerable unless the host is also using
> SSBD always on mode. In addition, the guest OS incorrectly
> reports the SSB vulnerability as mitigated.
> 
> Always set the SSBD bit in x86_spec_ctrl_mask when the host
> CPU supports it, allowing the guest to use SSBD whether or
> not the host has chosen to enable the mitigation in any of
> its modes.
> 
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
> Cc: stable@vger.kernel.org

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> ---
>  arch/x86/kernel/cpu/bugs.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 03b4cc0..66ca906 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -836,6 +836,16 @@ static enum ssb_mitigation __init __ssb_select_mitigation(void)
>  	}
>  
>  	/*
> +	 * If SSBD is controlled by the SPEC_CTRL MSR, then set the proper
> +	 * bit in the mask to allow guests to use the mitigation even in the
> +	 * case where the host does not enable it.
> +	 */
> +	if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) ||
> +	    static_cpu_has(X86_FEATURE_AMD_SSBD)) {
> +		x86_spec_ctrl_mask |= SPEC_CTRL_SSBD;
> +	}
> +
> +	/*
>  	 * We have three CPU feature flags that are in play here:
>  	 *  - X86_BUG_SPEC_STORE_BYPASS - CPU is susceptible.
>  	 *  - X86_FEATURE_SSBD - CPU is able to turn off speculative store bypass
> @@ -852,7 +862,6 @@ static enum ssb_mitigation __init __ssb_select_mitigation(void)
>  			x86_amd_ssb_disable();
>  		} else {
>  			x86_spec_ctrl_base |= SPEC_CTRL_SSBD;
> -			x86_spec_ctrl_mask |= SPEC_CTRL_SSBD;
>  			wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
>  		}
>  	}
> 

Re: [PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Thomas Gleixner]

On Tue, 25 Jun 2019, Paolo Bonzini wrote:
> On 10/06/19 19:20, Alejandro Jimenez wrote:

Btw, the proper prefix is: x86/speculation: Allow guests ....

> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > index 03b4cc0..66ca906 100644
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -836,6 +836,16 @@ static enum ssb_mitigation __init __ssb_select_mitigation(void)
> >  	}
> >  
> >  	/*
> > +	 * If SSBD is controlled by the SPEC_CTRL MSR, then set the proper
> > +	 * bit in the mask to allow guests to use the mitigation even in the
> > +	 * case where the host does not enable it.
> > +	 */
> > +	if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) ||
> > +	    static_cpu_has(X86_FEATURE_AMD_SSBD)) {
> > +		x86_spec_ctrl_mask |= SPEC_CTRL_SSBD;

Well, yes. But that also allows the guest to turn off SSBD if the host has
it disabled globally. So this needs to be conditional depending on the host
mode. It affects two places:

  1) If the host has it globally disabled then the mask needs to be clear.

  2) If the host has it specifically disabled for the VCPU thread, then it
     shouldn't be allowed to be cleared by the guest either.

Thanks,

	tglx

Re: [PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Alejandro Jimenez]

On 6/25/2019 12:05 PM, Thomas Gleixner wrote:
> On Tue, 25 Jun 2019, Paolo Bonzini wrote:
>> On 10/06/19 19:20, Alejandro Jimenez wrote:
> Btw, the proper prefix is: x86/speculation: Allow guests ....
I'll correct it on the next iteration of the patch.

>>> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
>>> index 03b4cc0..66ca906 100644
>>> --- a/arch/x86/kernel/cpu/bugs.c
>>> +++ b/arch/x86/kernel/cpu/bugs.c
>>> @@ -836,6 +836,16 @@ static enum ssb_mitigation __init __ssb_select_mitigation(void)
>>>   	}
>>>   
>>>   	/*
>>> +	 * If SSBD is controlled by the SPEC_CTRL MSR, then set the proper
>>> +	 * bit in the mask to allow guests to use the mitigation even in the
>>> +	 * case where the host does not enable it.
>>> +	 */
>>> +	if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) ||
>>> +	    static_cpu_has(X86_FEATURE_AMD_SSBD)) {
>>> +		x86_spec_ctrl_mask |= SPEC_CTRL_SSBD;
> Well, yes. But that also allows the guest to turn off SSBD if the host has
> it disabled globally. So this needs to be conditional depending on the host
> mode. It affects two places:
>
>    1) If the host has it globally disabled then the mask needs to be clear.
>
>    2) If the host has it specifically disabled for the VCPU thread, then it
>       shouldn't be allowed to be cleared by the guest either.
I see the argument that the host must be able to enforce its security 
policies on the guests running on it. The guest OS would still be 
'lying' by reporting that is running with the mitigation turned off, but 
I suppose this is preferable to overriding the host's security policy.

I think that even with that approach there is still an unsolved problem, 
as I believe guests are allowed to write directly to SPEC_CTRL MSR 
without causing a VMEXIT, which bypasses the host masking entirely.  
e.g. a guest using IBRS writes frequently to SPEC_CTRL, and could turn 
off SSBD on the VPCU while is running after the first non-zero write to 
the MSR. Do you agree?

Thank you for the feedback,
Alejandro

>
> Thanks,
>
> 	tglx
>
>
>

Re: [PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]

On Tue, 25 Jun 2019, Alejandro Jimenez wrote:
> On 6/25/2019 12:05 PM, Thomas Gleixner wrote:
> > On Tue, 25 Jun 2019, Paolo Bonzini wrote:
> > > On 10/06/19 19:20, Alejandro Jimenez wrote:
> > > >     	/*
> > > > +	 * If SSBD is controlled by the SPEC_CTRL MSR, then set the proper
> > > > +	 * bit in the mask to allow guests to use the mitigation even in the
> > > > +	 * case where the host does not enable it.
> > > > +	 */
> > > > +	if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) ||
> > > > +	    static_cpu_has(X86_FEATURE_AMD_SSBD)) {
> > > > +		x86_spec_ctrl_mask |= SPEC_CTRL_SSBD;
> >
> > Well, yes. But that also allows the guest to turn off SSBD if the host has
> > it disabled globally. So this needs to be conditional depending on the host
> > mode. It affects two places:
> > 
> >    1) If the host has it globally disabled then the mask needs to be clear.
> > 
> >    2) If the host has it specifically disabled for the VCPU thread, then it
> >       shouldn't be allowed to be cleared by the guest either.
>
> I see the argument that the host must be able to enforce its security policies
> on the guests running on it. The guest OS would still be 'lying' by reporting
> that is running with the mitigation turned off, but I suppose this is
> preferable to overriding the host's security policy.

True.

> I think that even with that approach there is still an unsolved problem, as I
> believe guests are allowed to write directly to SPEC_CTRL MSR without causing
> a VMEXIT, which bypasses the host masking entirely.  e.g. a guest using IBRS
> writes frequently to SPEC_CTRL, and could turn off SSBD on the VPCU while is
> running after the first non-zero write to the MSR. Do you agree?

Indeed. Of course that was a decision we made _before_ all the other fancy
things came around. Looks like we have to reopen that discussion.

Thanks,

	tglx

Re: [PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Paolo Bonzini]

On 25/06/19 20:22, Thomas Gleixner wrote:
>> I think that even with that approach there is still an unsolved problem, as I
>> believe guests are allowed to write directly to SPEC_CTRL MSR without causing
>> a VMEXIT, which bypasses the host masking entirely.  e.g. a guest using IBRS
>> writes frequently to SPEC_CTRL, and could turn off SSBD on the VPCU while is
>> running after the first non-zero write to the MSR. Do you agree?
> Indeed. Of course that was a decision we made _before_ all the other fancy
> things came around. Looks like we have to reopen that discussion.

It's not just that, it's a decision that was made because otherwise
performance is absolutely horrible (like 4-5x slower syscalls if the
guest is using IBRS).

I think it's better to leave the guest in control of SSBD even if it's
globally disabled.  The harm cannot escape the guest and in particular
it cannot escape to the sibling hyperthread.

Paolo

Re: [PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]

On Wed, 26 Jun 2019, Paolo Bonzini wrote:
> On 25/06/19 20:22, Thomas Gleixner wrote:
> >> I think that even with that approach there is still an unsolved problem, as I
> >> believe guests are allowed to write directly to SPEC_CTRL MSR without causing
> >> a VMEXIT, which bypasses the host masking entirely.  e.g. a guest using IBRS
> >> writes frequently to SPEC_CTRL, and could turn off SSBD on the VPCU while is
> >> running after the first non-zero write to the MSR. Do you agree?
> > Indeed. Of course that was a decision we made _before_ all the other fancy
> > things came around. Looks like we have to reopen that discussion.
> 
> It's not just that, it's a decision that was made because otherwise
> performance is absolutely horrible (like 4-5x slower syscalls if the
> guest is using IBRS).
> 
> I think it's better to leave the guest in control of SSBD even if it's
> globally disabled.  The harm cannot escape the guest and in particular
> it cannot escape to the sibling hyperthread.

SSB allows guest to guest attacks IIRC

Thanks,

	tglx

Re: [PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Paolo Bonzini]

On 26/06/19 14:41, Thomas Gleixner wrote:
>> I think it's better to leave the guest in control of SSBD even if it's
>> globally disabled.  The harm cannot escape the guest and in particular
>> it cannot escape to the sibling hyperthread.
>
> SSB allows guest to guest attacks IIRC

SSB requires something like

   p = &foo;
   ...
   p = &bar;
   q = *p;

where "p = &foo;" is executed from one privilege domain and the others
are executed by another process or privilege domain.  Unless two guests
share memory, it is not possible to use it for guest-to-guest attacks.

Paolo

Re: [PATCH 1/1] kvm/speculation: Allow KVM guests to use SSBD even if host does not

[Thomas Gleixner]

On Wed, 26 Jun 2019, Paolo Bonzini wrote:
> On 26/06/19 14:41, Thomas Gleixner wrote:
> >> I think it's better to leave the guest in control of SSBD even if it's
> >> globally disabled.  The harm cannot escape the guest and in particular
> >> it cannot escape to the sibling hyperthread.
> >
> > SSB allows guest to guest attacks IIRC
> 
> SSB requires something like
> 
>    p = &foo;
>    ...
>    p = &bar;
>    q = *p;
> 
> where "p = &foo;" is executed from one privilege domain and the others
> are executed by another process or privilege domain.  Unless two guests
> share memory, it is not possible to use it for guest-to-guest attacks.

Fair enough. It's way too hot to think clearly about these kind of problems
and there are simply way too many of them...

Thanks,

	tglx