Re: [PATCH 1/5] KVM: x86/xen: Restrict hypercall MSR to unofficial synthetic range

[David Woodhouse]

On Wed, 2025-02-05 at 15:26 +0000, David Woodhouse wrote:
> 
> > > @@ -3733,7 +3733,13 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > >         u32 msr = msr_info->index;
> > >         u64 data = msr_info->data;
> > >  
> > > -       if (msr && msr == vcpu->kvm->arch.xen_hvm_config.msr)
> > > +       /*
> > > +        * Do not allow host-initiated writes to trigger the Xen hypercall
> > > +        * page setup; it could incur locking paths which are not expected
> > > +        * if userspace sets the MSR in an unusual location.
> > 
> > That's just as likely to break userspace.  Doing a save/restore on the MSR doesn't
> > make a whole lot of sense since it's effectively a "command" MSR, but IMO it's not
> > any less likely than userspace putting the MSR index outside of the synthetic range.
> 
> Save/restore on the MSR makes no sense. It's a write-only MSR; writing
> to it has no effect *other* than populating the target page. In KVM we
> don't implement reading from it at all; I don't think Xen does either?
> 
> And even if it was readable and would rather pointlessly return the
> last value written to it, save/restore arguably shouldn't actually
> trigger the guest memory to be overwritten again. The hypercall page
> should only be populated when the *guest* writes the MSR.
> 
> With the recent elimination of the hypercall page from Linux Xen
> guests, we've suggested that Linux should still set up the hypercall
> page early (as it *does* have the side-effect of letting Xen know that
> the guest is 64-bit). And then just free the page without ever using
> it. We absolutely would not want a save/restore to scribble on that
> page again.
> 
> I'm absolutely not worried about breaking userspace with such a change
> to make the hypercall page MSR only work when !host_initiated. In fact
> I think it's probably the right thing to do *anyway*.
> 
> If userspace wants to write to guest memory, it can do that anyway; it
> doesn't need to ask the *kernel* to do it.

In the meantime, our VMM actually did start to intercept the MSR write
and then 'replay' it to the kernel with ->host_initiated.

It does so because the MSR writes correlate with kexec/crash in the
guest, and tracking that allows the VMM to deal with resetting e.g. the
blkback/netback rings in order that the crash kernel can actually
complete the crashdump.

I pondered reverting commit 3617c0ee7dec but I do still think it makes
sense for all the reasons we originally discussed. I pondered making
VMM do everything for itself which kvm_xen_write_hypercall_page() does,
but that's a bit icky.

Settled for adding a KVM_XEN_HVM_CONFIG_WRITE_HYPERCALL_PAGE attribute
which basically does the same operation directly, instead of triggering
it through an MSR write. Patch incoming.

Attachment: smime.p7s
Description: S/MIME cryptographic signature