Re: [PATCH v6 01/14] KVM: s390: Fix unlikely NULL gmap dereference

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch implements an architecture-specific workaround to fix a generic KVM race condition instead of addressing the root cause by deferring fd_install().
--

--- Patch [1]: [PATCH v6 01/14] KVM: s390: Fix unlikely NULL gmap dereference ---
commit 403acc4eca694d221f980de53cd9d5235694b0bf
Author: Claudio Imbrenda <[email protected]>

KVM: s390: Fix unlikely NULL gmap dereference

This commit fixes a race condition where a malicious userspace program
can start a newly created vCPU before kvm_arch_vcpu_postcreate() finishes.
It resolves this by tracking the initialization status with a new flag.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 150b5dd2170e2..f86b4b0b356f4 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
[ ... ]
> @@ -3613,6 +3613,9 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
>  	if (test_kvm_facility(vcpu->kvm, 74) || vcpu->kvm->arch.user_instr0 ||
>  	    vcpu->kvm->arch.user_operexec)
>  		vcpu->arch.sie_block->ictl |= ICTL_OPEREXC;
> +
> +	/* Pairs with smp_load_acquire() in kvm_arch_vcpu_ioctl_run() and kvm_arch_vcpu_ioctl() */
> +	smp_store_release(&vcpu->arch.initialized, true);

[Severity: Medium]
Could this regression be addressed in the generic KVM code rather than
adding an architecture-specific workaround here?

The race occurs because virt/kvm/kvm_main.c:kvm_vm_ioctl_create_vcpu()
exposes the file descriptor to userspace before postcreate completes:

virt/kvm/kvm_main.c:kvm_vm_ioctl_create_vcpu() {
    ...
    r = create_vcpu_fd(vcpu);
    ...
    kvm_arch_vcpu_postcreate(vcpu);
    ...
}

Since create_vcpu_fd() installs the file descriptor, userspace can
immediately guess or observe the fd and race to call KVM_RUN or other
ioctls while kvm_arch_vcpu_postcreate() is still running.

Could we fix this generically by splitting anon_inode_getfd() into
get_unused_fd_flags() and anon_inode_getfile() during fd creation,
and then deferring fd_install() until after kvm_arch_vcpu_postcreate()
completes?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1