Re: [PATCH v1 5/6] KVM: s390: Add missing srcu in kvm_arch_vcpu_ioctl()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Circular deadlock between KVM SRCU lock and userfaultfd during copy_from_user().
--

--- Patch [5]: [PATCH v1 5/6] KVM: s390: Add missing srcu in kvm_arch_vcpu_ioctl() ---
commit 4558246eff181af6ab8735d60c453a1298252bfe
Author: Claudio Imbrenda <[email protected]>

KVM: s390: Add missing srcu in kvm_arch_vcpu_ioctl()

This commit adds an SRCU read lock around kvm_s390_set_irq_state()
in kvm_arch_vcpu_ioctl().

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 50eb72447fd53..7fb5a566e2d85 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -5697,9 +5697,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>  			break;
>  		}
>  		/* do not use irq_state.flags, it will break old QEMUs */
> -		r = kvm_s390_set_irq_state(vcpu,
> -					   (void __user *) irq_state.buf,
> -					   irq_state.len);
> +		scoped_guard(srcu, &vcpu->kvm->srcu) {
> +			r = kvm_s390_set_irq_state(vcpu, (void __user *)irq_state.buf,
> +						   irq_state.len);
> +		}

[Severity: High]
Does this scoped_guard result in calling copy_from_user() while holding
the SRCU read lock?

Looking at kvm_s390_set_irq_state() in arch/s390/kvm/interrupt.c, it
performs a user copy:

int kvm_s390_set_irq_state(struct kvm_vcpu *vcpu, void __user *irqstate, int len)
{
	...
	buf = vmalloc(len);
	if (!buf)
		return -ENOMEM;

	if (copy_from_user((void *)buf, irqstate, len))
		return -EFAULT;
	...
}

If userspace passes a userfaultfd-managed region as the irq state buffer,
copy_from_user() can block and wait for the userfaultfd manager.

If that manager then attempts an operation that updates KVM memslots
(such as KVM_SET_USER_MEMORY_REGION), it will acquire the slots lock
and wait on synchronize_srcu().

Could this cause a circular deadlock between the SRCU read lock held
here and the slots lock acquired by the userfaultfd manager?

>  		break;
>  	}
>  	case KVM_S390_GET_IRQ_STATE: {

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