Re: [PATCH 21/60] kvm: Introduce per-plane VCPU requests

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] TOCTOU race condition bypassing the `max_vcpus` limit due to broken `created_vcpus` accounting, leading to a host stack buffer overflow.
--

commit 621fb80759c0391b7de8533a2106e6298a930411
Author: Joerg Roedel <joerg.roedel@amd.com>

kvm: Introduce per-plane VCPU requests

This commit introduces per-plane VCPU requests by adding a new bitfield
flag to indicate request scope. It adds plane_requests to struct kvm_vcpu
and modifies the request macros to route calls to the appropriate bitmap.

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 7704820986da5..3c72a462ccfa3 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h

[ ... ]

> @@ -371,6 +373,10 @@ struct kvm_vcpu {
>  	int sigset_active;
>  	sigset_t sigset;
>  	unsigned int halt_poll_ns;
> +
> +	u64 plane_requests;
> +

[Severity: Critical]
This isn't a bug introduced by this patch, but while reviewing the VCPU
changes, I noticed a pre-existing TOCTOU race that can bypass the max_vcpus
limit and lead to a host stack buffer overflow.

In virt/kvm/kvm_main.c:kvm_plane_ioctl_create_vcpu(), the limit check
happens under kvm->lock, but the lock is dropped before the vCPU is actually
accounted for:

mutex_lock(&kvm->lock);
if (kvm->created_vcpus >= kvm->max_vcpus) {
        mutex_unlock(&kvm->lock);
        return -EINVAL;
}
r = kvm_arch_vcpu_precreate(kvm, id);
mutex_unlock(&kvm->lock);

The kvm->created_vcpus counter is only incremented much later in
virt/kvm/kvm_main.c:kvm_vcpu_init_common() after re-acquiring the lock:

mutex_lock(&kvm->lock);
kvm->created_vcpus++;
mutex_unlock(&kvm->lock);

Could this allow concurrent KVM_CREATE_VCPU ioctls to bypass the limit and
create up to KVM_MAX_VCPU_IDS vCPUs, resulting in an online_vcpus count
that exceeds KVM_MAX_VCPUS?

If so, multiple code paths allocate fixed-size bitmaps on the host kernel
stack and iterate over all online vCPUs. For example, in
arch/x86/kvm/ioapic.c:ioapic_write_indirect():

DECLARE_BITMAP(vcpu_bitmap, KVM_MAX_VCPUS);
...
kvm_bitmap_or_dest_vcpus(ioapic->kvm->planes[0], &irq, vcpu_bitmap);

And then in arch/x86/kvm/lapic.c:kvm_bitmap_or_dest_vcpus():

plane_for_each_vcpu(i, vcpu, plane) {
        ...
        __set_bit(i, vcpu_bitmap);
}

Does this sequence result in an out-of-bounds write on the stack if a
malicious guest triggers the race and subsequently writes to the IOAPIC?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608144252.351443-1-joro@8bytes.org?part=21