From: Sean Christopherson <seanjc@google.com>
To: Mahmoud Abumandour <ma.mandourr@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] KVM: nVMX: Don't let L2's LMSW modify CR0 bits owned by L1
Date: Fri, 30 Oct 2026 11:44:32 -0700 [thread overview]
Message-ID: <auTlkNFh5C7nqzoi@google.com> (raw)
In-Reply-To: <auTgh5uP5tpL8BlP@google.com>
On Fri, Oct 30, 2026, Sean Christopherson wrote:
> On Thu, Oct 29, 2026, Mahmoud Abumandour wrote:
> diff --git arch/x86/kvm/vmx/vmx.c arch/x86/kvm/vmx/vmx.c
> index 544b7840897c..2d5feeb8a059 100644
> --- arch/x86/kvm/vmx/vmx.c
> +++ arch/x86/kvm/vmx/vmx.c
> @@ -5743,8 +5743,10 @@ static int handle_cr(struct kvm_vcpu *vcpu)
> break;
> case 3: /* lmsw */
> val = (exit_qualification >> LMSW_SOURCE_DATA_SHIFT) & 0x0f;
> - trace_kvm_cr_write(0, (kvm_read_cr0_bits(vcpu, ~0xful) | val));
> - kvm_lmsw(vcpu, val);
> + val |= kvm_read_cr0_bits(vcpu, ~0x0eul);
> +
> + trace_kvm_cr_write(cr, val);
> + (void)handle_set_cr0(vcpu, val);
>
> return kvm_skip_emulated_instruction(vcpu);
> default:
>
> > + err = handle_set_cr0(vcpu, (cr0 & ~0x0eul) | val);
> > + return kvm_complete_insn_gp(vcpu, err);
>
> This needs to (a) be a separate patch and (b) apply to L1 as well (though I'm not
> even sure if you/AI realize this is a bug fix). Emulation of LMSW originally
> suppressed the return value, because outside of VMX operation, LMSW can't #GP on
> a bad value. LMSW can't set reserved bits (for the CPU models KVM supports), and
> it can't toggle any bits that trigger additional checks, e.g. can't try to clear
> CR0.PG while in 64-bit mode.
>
> But with VMX, must-be-1 and must-be-0 bits become a thing, and so LMSW could #GP
> if it tried to set/clear a bit in violation of those masks.
>
> Ugh, never mind, screw this. KVM doesn't support arbitrary MSR_IA32_VMX_CR0_FIXED1
> values, i.e. doesn't let userspace require that arbitrary bits always be '0', and
> instead relies on common CR0 reserved bit checks. KVM (very, very stupidly) does
> allow userspace to expand the set of must-be-1 (MSR_IA32_VMX_CR0_FIXED0) bits), and
> so *in theory* LMSW could fault when clearing EM, MP, or TS. Of course, no sane
> VMM sets those as must-be-1 bits. And to make this a complete trainwreck, KVM
> doesn't update its own guest/host masks, i.e. it's own intercepts, to ensure it
> enforces the must-be-1 bits.
>
> E.g. if userspace made CR0.TS a must-be-1 bit, then the guest could violate that
> so long as it doesn't toggle any other bits that would trigger a VM-Exit.
>
> I see absolutely zero reason to try and fix KVM. Letting userspace make CR0.TS
> a must-be-1 bit is beyond stupid. KVM already requires CR0 and CR4 must-be-1
> masks to be a superset of KVM's "defaults", so userspace can only make things
> more restrictive.
>
> So, rather than synthesize #GPs, I think we should prevent nVMX MSRs from inducing
> #GPs in the first place, and then WARN.
Argh, that's not going to work. Thanks to KVM's massive "attack" surface, a.k.a.
save/restore APIs, it's possible for userspace to shove a value into vcpu->arch.cr0
that's in violation of must-be-1 bits. I.e. userspace can trigger the WARN.
So as much as I want to WARN, in practice no real world use case will induce a #GP
on LMSW (or hell, even trigger an exit on LMSW), so I'll go with your patch of
synthesizing a #GP (which is better than silently failing).
next prev parent reply other threads:[~2026-10-30 18:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-30 3:29 [PATCH v2 0/2] KVM: nVMX: Don't let L2's LMSW modify CR0 bits owned by L1 Mahmoud Abumandour
2026-10-30 3:29 ` [PATCH v2 1/2] " Mahmoud Abumandour
2026-10-30 18:23 ` Sean Christopherson
2026-10-30 18:44 ` Sean Christopherson [this message]
2026-10-30 3:29 ` [PATCH v2 2/2] KVM: selftests: Verify that L2's LMSW leaves L1-owned CR0 bits alone Mahmoud Abumandour
2026-10-30 18:41 ` Sean Christopherson
2026-10-30 18:50 ` [PATCH v2 0/2] KVM: nVMX: Don't let L2's LMSW modify CR0 bits owned by L1 Sean Christopherson
2026-10-30 18:52 ` Jim Mattson
2026-10-30 18:54 ` Sean Christopherson
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=auTlkNFh5C7nqzoi@google.com \
--to=seanjc@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ma.mandourr@gmail.com \
--cc=pbonzini@redhat.com \
/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.