Re: [PATCH v3 13/13] KVM: x86/xen: Convert evtchn_ports from IDR to XArray

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Type confusion in evtchnfd delivery union allows a userspace guest to crash the host kernel by assigning an EVTCHNSTAT_ipi event channel to port 0.
--

commit 365d4c87b9f4941190fe8a9a101dce7853ba1e18
Author: Furkan Caliskan <frn1furkan10@gmail.com>

KVM: x86/xen: Convert evtchn_ports from IDR to XArray
    
This patch converts the deprecated IDR used for evtchn_ports into an XArray.
It replaces idr_alloc() with xa_insert() and idr_remove() with xa_erase(),
dropping the explicit rcu_read_lock() in kvm_xen_hcall_evtchn_send() since
xa_load() is internally RCU-safe.

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index b024a09de55a3..b59c6b747c2f6 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -2214,13 +2214,13 @@ static int kvm_xen_eventfd_assign(struct kvm *kvm,
>  	}
>  
>  	mutex_lock(&kvm->arch.xen.xen_lock);
> -	ret = idr_alloc(&kvm->arch.xen.evtchn_ports, evtchnfd, port, port + 1,
> +	ret = xa_insert(&kvm->arch.xen.evtchn_ports, port, evtchnfd,
>  			GFP_KERNEL);

[Severity: Critical]
This isn't a bug introduced by this patch, but could this union type
confusion allow a userspace guest to crash the host kernel?

When a userspace process calls the KVM ioctl to assign an event channel with
type EVTCHNSTAT_ipi and send_port = 0, the setup code just prior to this
insertion does the following in kvm_xen_eventfd_assign():

	evtchnfd->deliver.port.port = data->u.evtchn.deliver.port.port;
	evtchnfd->deliver.port.vcpu_id = data->u.evtchn.deliver.port.vcpu;
	evtchnfd->deliver.port.vcpu_idx = -1;

Because of the union layout in the evtchnfd structure, deliver.port overlaps
with deliver.eventfd. Writing -1 to vcpu_idx corrupts the eventfd_ctx pointer.

> [ ... ]
> @@ -2235,7 +2235,7 @@ static int kvm_xen_eventfd_deassign(struct kvm *kvm, u32 port)
>  	struct evtchnfd *evtchnfd;
>  
>  	mutex_lock(&kvm->arch.xen.xen_lock);
> -	evtchnfd = idr_remove(&kvm->arch.xen.evtchn_ports, port);
> +	evtchnfd = xa_erase(&kvm->arch.xen.evtchn_ports, port);
>  	mutex_unlock(&kvm->arch.xen.xen_lock);
>  
>  	if (!evtchnfd)
> @@ -2251,7 +2251,7 @@ static int kvm_xen_eventfd_deassign(struct kvm *kvm, u32 port)
> [ ... ]
> @@ -2376,23 +2374,23 @@ void kvm_xen_destroy_vcpu(struct kvm_vcpu *vcpu)
>  void kvm_xen_destroy_vm(struct kvm *kvm)
>  {
>  	struct evtchnfd *evtchnfd;
> -	int i;
> +	unsigned long i;
>  
>  	kvm_gpc_deactivate(&kvm->arch.xen.shinfo_cache);
>  
> -	idr_for_each_entry(&kvm->arch.xen.evtchn_ports, evtchnfd, i) {
> +	xa_for_each(&kvm->arch.xen.evtchn_ports, i, evtchnfd) {
>  		if (!evtchnfd->deliver.port.port)
>  			eventfd_ctx_put(evtchnfd->deliver.eventfd.ctx);

[Severity: Critical]
This is a pre-existing issue, but does this check trigger an invalid pointer
dereference due to the type confusion mentioned above?

If an IPI was assigned to port 0, evtchnfd->deliver.port.port evaluates to 0,
making the condition true. The kernel will then mistakenly dereference the
corrupted pointer (overlapping with vcpu_idx = -1) by calling
eventfd_ctx_put().

This exact sequence is present during deassignment in
kvm_xen_eventfd_deassign(), as well as here during VM destruction, which
seems like it could lead to a kernel panic and a host denial of service.

>  		kfree(evtchnfd);
>  	}

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