Re: [GIT PULL v1 01/12] KVM: s390: Fix S390_USER_OPEREXEC enablement without STFLE 74

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] A lockless data race between concurrent KVM_ENABLE_CAP and KVM_CREATE_VCPU can result in a vCPU silently missing the operation exception intercept.

Pre-existing issues:
- [Critical] Missing `vcpu->mutex` in `kvm_arch_vcpu_postcreate` allows a concurrent `KVM_RUN` ioctl to trigger a host kernel panic via a NULL pointer dereference on `vcpu->arch.gmap`.
--

commit 40c6c07119e484ffc2bb8e5fccec2d6c44c1df03
Author: Eric Farman <farman@linux.ibm.com>

KVM: s390: Fix S390_USER_OPEREXEC enablement without STFLE 74

This commit ensures the USER_OPEREXEC capability is properly enabled
in kvm_arch_vcpu_postcreate() even if the guest disables STFLE.74.
It adds a check for vcpu->kvm->arch.user_operexec alongside the
existing checks for STFLE.74 and user_instr0.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -3542,7 +3542,8 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
>  		vcpu->arch.gmap = vcpu->kvm->arch.gmap;

[Severity: Critical]
This is a pre-existing issue, but does this function need to hold
vcpu->mutex while initializing vcpu->arch.gmap?

When kvm_vm_ioctl_create_vcpu() executes, it unlocks vcpu->mutex and
increments kvm->online_vcpus before calling kvm_arch_vcpu_postcreate():

virt/kvm/kvm_main.c:kvm_vm_ioctl_create_vcpu()
    atomic_inc(&kvm->online_vcpus);
    mutex_unlock(&vcpu->mutex);
    mutex_unlock(&kvm->lock);
    kvm_arch_vcpu_postcreate(vcpu);

If a second thread immediately calls the KVM_RUN ioctl on the new vCPU,
it can acquire vcpu->mutex and proceed to __vcpu_run() before
kvm_arch_vcpu_postcreate() finishes.  Could this result in a NULL pointer
dereference when __vcpu_run() accesses vcpu->arch.gmap->asce.val?

arch/s390/kvm/kvm-s390.c:__vcpu_run()
    sie_return = kvm_s390_enter_exit_sie(vcpu->arch.sie_block,
                                         vcpu->run->s.regs.gprs,
                                         vcpu->arch.gmap->asce.val);

>  		sca_add_vcpu(vcpu);
>  	}
> -	if (test_kvm_facility(vcpu->kvm, 74) || vcpu->kvm->arch.user_instr0)
> +	if (test_kvm_facility(vcpu->kvm, 74) || vcpu->kvm->arch.user_instr0 ||
> +	    vcpu->kvm->arch.user_operexec)

[Severity: Medium]
Does the lockless read of user_operexec introduce a data race with
KVM_ENABLE_CAP?

If a thread enables KVM_CAP_S390_USER_OPEREXEC concurrently with vCPU
creation, kvm_vm_ioctl_enable_cap() writes to kvm->arch.user_operexec and
iterates over online vCPUs without a full memory barrier:

arch/s390/kvm/kvm-s390.c:kvm_vm_ioctl_enable_cap()
    kvm->arch.user_operexec = 1;
    icpt_operexc_on_all_vcpus(kvm);

Meanwhile, kvm_vm_ioctl_create_vcpu() increments online_vcpus before
calling kvm_arch_vcpu_postcreate().

Because of this sequence, is it possible for both threads to observe the
older state of the other's variables? If so, could this cause the newly
created vCPU to silently miss setting the ICTL_OPEREXC bit?

>  		vcpu->arch.sie_block->ictl |= ICTL_OPEREXC;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624111928.144283-1-imbrenda@linux.ibm.com?part=1