Re: [PATCH v1 02/11] KVM: s390: Use srcu in kvm_arch_vcpu_unlocked_ioctl()

[Christian Borntraeger]

Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> 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 in kvm_arch_vcpu_unlocked_ioctl().
>
> Fixes: ba5c1e9b6cee ("KVM: s390: interrupt subsystem, cpu timer, waitpsw")
> Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
> ---
>   arch/s390/kvm/kvm-s390.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 518a69c55e85..02c5428ba239 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -5444,6 +5444,8 @@ long kvm_arch_vcpu_unlocked_ioctl(struct file *filp, unsigned int ioctl,
>   	void __user *argp = (void __user *)arg;
>   	int rc;
>   
> +	guard(srcu)(&vcpu->kvm->srcu);
> +

not a bug, but just having srcu around both inject would avoid having holding srcu on the
argument usercopy. This is not forbidden but might just take a while when paging.

 scoped_guard(srcu)(&vcpu->kvm->srcu)
                      rc = kvm_s390_inject_vcpu(vcpu, &s390irq);

in any way, no objection to your variant and a real fix.

Reviewed-by: Christian Borntraeger <borntraeger@xxxxxxxxxxxxx>


>   	switch (ioctl) {
>   	case KVM_S390_IRQ: {
>   		struct kvm_s390_irq s390irq;