Re: [PATCH v2 2/9] KVM: s390: Use srcu in kvm_arch_vcpu_unlocked_ioctl()

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

Pre-existing issues:
- [High] kvm_s390_inject_vcpu() can sleep while holding a spinlock when handling a KVM_S390_SIGP_STOP interrupt, leading to a kernel panic.
--

--- Patch [2]: [PATCH v2 2/9] KVM: s390: Use srcu in kvm_arch_vcpu_unlocked_ioctl() ---
commit 9a55f0a8064e7c3f8972e52233d190eda6c12400
Author: Claudio Imbrenda <[email protected]>

KVM: s390: Use srcu in kvm_arch_vcpu_unlocked_ioctl()

kvm_arch_vcpu_unlocked_ioctl() is called without further locks held, but
kvm_s390_inject_vcpu(), which is called from there, needs either the
kvm->srcu or the slots lock. Fix by taking the kvm->srcu.

> 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
[ ... ]
> @@ -5463,7 +5464,8 @@ long kvm_arch_vcpu_unlocked_ioctl(struct file *filp, unsigned int ioctl,
>  			return -EFAULT;
>  		if (s390int_to_s390irq(&s390int, &s390irq))
>  			return -EINVAL;
> -		rc = kvm_s390_inject_vcpu(vcpu, &s390irq);
> +		scoped_guard(srcu, &vcpu->kvm->srcu)
> +			rc = kvm_s390_inject_vcpu(vcpu, &s390irq);

[Severity: High]
This is a pre-existing issue, but can this call path sleep while holding
a spinlock?

If userspace calls the KVM_S390_INTERRUPT ioctl with type KVM_S390_SIGP_STOP
and the KVM_S390_STOP_FLAG_STORE_STATUS flag, kvm_s390_inject_vcpu()
acquires a spinlock and disables preemption:

arch/s390/kvm/interrupt.c:kvm_s390_inject_vcpu()
    spin_lock(&li->lock);
    rc = do_inject_vcpu(vcpu, irq);
    spin_unlock(&li->lock);

While under the spinlock, do_inject_vcpu() triggers a guest memory write:

arch/s390/kvm/interrupt.c:__inject_sigp_stop()
    if (irq->u.stop.flags & KVM_S390_STOP_FLAG_STORE_STATUS)
        rc = kvm_s390_store_status_unloaded(vcpu,
                    KVM_S390_STORE_STATUS_NOADDR);

This initiates a write to guest memory that can fault and sleep:

arch/s390/kvm/kvm-s390.c:kvm_s390_store_status_unloaded()
    if (gpa == KVM_S390_STORE_STATUS_NOADDR) {
        if (write_guest_abs(vcpu, 163, &archmode, 1))

Could this underlying __copy_to_user() call (via write_guest_abs) cause a
'scheduling while atomic' kernel panic by sleeping while li->lock is held?

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