Re: [PATCH] KVM: x86: Drop "cache" from user return MSR setter that skips WRMSR

[Sean Christopherson <seanjc@google.com>]

On Thu, Oct 16, 2025, Hou Wenlong wrote:
> On Thu, Oct 16, 2025 at 04:58:17PM +0800, Yan Zhao wrote:
> > On Wed, Oct 15, 2025 at 09:19:00AM -0700, Sean Christopherson wrote:
> > > +	/*
> > > +	 * Leave the user-return notifiers as-is when disabling virtualization
> > > +	 * for reboot, i.e. when disabling via IPI function call, and instead
> > > +	 * pin kvm.ko (if it's a module) to defend against use-after-free (in
> > > +	 * the *very* unlikely scenario module unload is racing with reboot).
> > > +	 * On a forced reboot, tasks aren't frozen before shutdown, and so KVM
> > > +	 * could be actively modifying user-return MSR state when the IPI to
> > > +	 * disable virtualization arrives.  Handle the extreme edge case here
> > > +	 * instead of trying to account for it in the normal flows.
> > > +	 */
> > > +	if (in_task() || WARN_ON_ONCE(!kvm_rebooting))
> > kvm_offline_cpu() may be invoked when irq is enabled.
> > So does it depend on [1]?
> > 
> > [1] https://lore.kernel.org/kvm/aMirvo9Xly5fVmbY@google.com/
> >
> 
> Actually, kvm_offline_cpu() can't be interrupted by kvm_shutdown().
> syscore_shutdown() is always called after
> migrate_to_reboot_cpu(), which internally waits for currently running
> CPU hotplug to complete, as described in [*].
> 
> [*] https://lore.kernel.org/kvm/dd4b8286774df98d58b5048e380b10d4de5836af.camel@intel.com
> 
> 
> > > +		drop_user_return_notifiers();
> > > +	else
> > > +		__module_get(THIS_MODULE);
> > Since vm_vm_fops holds ref of module kvm_intel, and drop_user_return_notifiers()
> > is called in kvm_destroy_vm() or kvm_exit():
> > 
> > kvm_destroy_vm/kvm_exit
> >   kvm_disable_virtualization
> >     kvm_offline_cpu
> >       kvm_disable_virtualization_cpu
> >         drop_user_return_notifiers
> > 
> > also since fire_user_return_notifiers() executes with irq disabled, is it
> > necessary to pin kvm.ko?

Pinning kvm.ko is necessary because kvm_disable_virtualization_cpu() will bail
early due to virtualization_enabled being false (it will have been cleared by
the IPI call from kvm_shutdown()).  We could try figuring out a way around that,
but I don't see an easy solution, and in practice I can't think of any meaningful
downside to pinning kvm.ko.

I don't want to leave virtualization_enabled set because that's completely wrong
for everything except x86's user-return MSRs, which aren't even strictly related
to enabling virtualization.

I considered calling drop_user_return_notifiers() directly from kvm_exit(), but
that would require more special-case code, and it would mean blasting an IPI to
all CPUs, which seems like a bad idea when we know the system is trying to reboot.