Re: [PATCH] KVM: VMX: remove LFENCE in vmx_spec_ctrl_restore_host()

[Jon Kohler]

> On May 31, 2023, at 7:18 PM, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> 
> On Wed, May 31, 2023 at 11:01:12AM -0400, Jon Kohler wrote:
>> Remove barrier_nospec(), which translates to LFENCE, in
>> vmx_spec_ctrl_restore_host() as RSB-barriers (as defined by [1])
>> already exist prior to this point.
>> 
>> This LFENCE was added on commit fc02735b14ff ("KVM: VMX: Prevent guest
>> RSB poisoning attacks with eIBRS") in the 5.19 timeframe; however,
>> commit 2b1299322016 ("x86/speculation: Add RSB VM Exit protections") in
>> 6.0 timeframe added a LFENCE for X86_FEATURE_RSB_VMEXIT_LITE was added
>> directly in vmx_vmexit, prior to CALL vmx_spec_ctrl_restore_host.
>> 
>> For posterity, vmx_spec_ctrl_restore_host also will execute WRMSR to
>> IA32_SPEC_CTRL for X86_FEATURE_KERNEL_IBRS or when guest/host MSR value
>> does not match, which serves as an additional RSB-barrier.
>> 
>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__www.intel.com_content_www_us_en_developer_articles_technical_software-2Dsecurity-2Dguidance_advisory-2Dguidance_post-2Dbarrier-2Dreturn-2Dstack-2Dbuffer-2Dpredictions.html&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=mWtdoxDY5cOR_aKAtV9D8kHfeWi380k1kYwGB_RAPTEL1F_AUSstYbevyVn9lhk-&s=IG-gZfjPGO_XI9FkdbrvZFvHPyWMQD8EK9AuBEpVY94&e= 
>> 
>> Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
>> Fixes: 2b1299322016 ("x86/speculation: Add RSB VM Exit protections")
> 
> Sorry, I knew I should have put a comment there.
> 
> The goal of this barrier_nospec() is to prevent speculative execution
> from bypassing the SPEC_CTRL write (due to misprediction of the
> conditional branch, Spectre v1 style).  Otherwise the next indirect
> branch or unbalanced RET could be an attack target.
> 
> So any previous LFENCEs before that conditional branch won't help here.
> 
> -- 
> Josh

Ah interesting. Ok, to be clear, thats a guest -> host attack, correct? And such
an attack would not at all be thwarted by the first CALL retire + LFENCE that
was added on commit 2b1299322016 ("x86/speculation: Add RSB VM Exit 
protections”)? Sorry to be long winded, just wanting to triple check because
the aforementioned commit was added slightly after the original one, and I 
want to make extra sure that they aren’t solving the same thing.

If that is indeed the case, does that commit need to be revisited at all?

Or are we saying that this Intel vulnerability needs *two* LFENCE’s to keep
the host secure?

If that’s the case, that’s fine, and I’m happy to transform this commit into
some comments in this area to capture the issue for future onlookers?