Re: [PATCH v2 06/11] KVM: x86/xen: Retry event channel delivery which races a memslot update

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: host DoS via userspace-reachable WARN_ON_ONCE in kvm_xen_set_evtchn
- [Medium] kvm: xen: guest #GP on hypercall page WRMSR racing with memslot update
--

commit 1bad660e92709c34e97084491648f1684e74aada
Author: David Woodhouse <dwmw@amazon.co.uk>

KVM: x86/xen: Retry event channel delivery which races a memslot update

Event channel delivery now returns -EAGAIN when it races a memslot
update. It always failed; now the failure is distinguishable, and
instead of dropping the event each caller reacts according to its
context.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260920215341.5ECE91F000FF@smtp.kernel.org
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM

> Event channel delivery now returns -EAGAIN when it races a memslot
> update. It always failed; now the failure is distinguishable, and
> instead of dropping the event each caller reacts according to its
> context:

[Severity: Medium]
Will this newly distinguishable -EAGAIN error cause a guest #GP on hypercall
page WRMSR racing with a memslot update?

In arch/x86/kvm/xen.c:kvm_xen_write_hypercall_page(), the guest executes
WRMSR to set up the Xen hypercall page, which runs under the vCPU's
kvm->srcu read lock. If the guest's 32/64-bit mode changed, it attempts to
re-initialize the shared_info page:

	if (kvm->arch.xen.long_mode != lm) {
		kvm->arch.xen.long_mode = lm;

		/*
		 * Re-initialize shared_info to put the wallclock in the
		 * correct place.
		 */
		if (kvm->arch.xen.shinfo_cache.active &&
		    kvm_xen_shared_info_init(kvm, false))
			r = 1;
	}

Because wait=false is explicitly passed to avoid deadlocking srcu,
kvm_xen_shared_info_init returns -EAGAIN if it races with a concurrent
host memslot update.

kvm_xen_write_hypercall_page treats any non-zero return (including the
newly distinguishable -EAGAIN transient error) as a failure by setting
r = 1 and returning it. kvm_set_msr_common receives this 1 and incorrectly
injects a #GP into the guest, crashing the guest purely due to a transient
host-side memslot update.

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index f1be9022bc6fe..a88f571e8682b 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c

[ ... ]

> @@ -2076,7 +2088,7 @@ static int evtchn_set_fn(struct kvm_kernel_irq_routing_entry *e, struct kvm *kvm
>  	if (!level)
>  		return -EINVAL;
>  
> -	return kvm_xen_set_evtchn(&e->xen_evtchn, kvm);
> +	return kvm_xen_set_evtchn(&e->xen_evtchn, kvm, true);
>  }

[Severity: High]
Does this expose a host denial of service to unprivileged userspace threads?

An unprivileged userspace process can create a KVM VM and set up an IRQ
routing entry for a Xen event channel using KVM_SET_GSI_ROUTING. The process
then forks, passing the VM file descriptor to the child process, which now
runs with a different mm (current->mm != kvm->mm).

The child process can issue a KVM_IRQ_LINE ioctl on the VM fd, which invokes
evtchn_set_fn() and subsequently kvm_xen_set_evtchn().

In kvm_xen_set_evtchn():

	if (current->mm != kvm->mm) {
		/*
		 * If not on a thread which already belongs to this KVM,
		 * we'd better be in the irqfd workqueue.
		 */
		if (WARN_ON_ONCE(current->mm))
			return -EINVAL;

Because current->mm != kvm->mm and the child process is a userspace thread
(current->mm != NULL), the WARN_ON_ONCE(current->mm) condition evaluates to
true. On systems configured with panic_on_warn, this allows an unprivileged
user to crash the host kernel.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014220508.518772-1-dwmw2@infradead.org?part=6