Re: [PATCH v1 4/8] KVM: s390: Fix return value of kvm_s390_set_cmma_bits()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Leaked positive error codes (e.g., PGM_ADDRESSING) to userspace in KVM storage key ioctls.
--

commit 9e2ad138adcf0c89db7df7095fa7b5199afab279
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix return value of kvm_s390_set_cmma_bits()

If kvm_s390_set_cmma_bits() is asked to set CMMA values outside of a
memslot, PGM_ADDRESSING (5) is returned, instead of a negative error
value. Fix by returning -EFAULT whenever the return value would be > 0.

> 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
> @@ -2384,7 +2384,7 @@ static int kvm_s390_set_cmma_bits(struct kvm *kvm,
>  
>  	set_bit(GMAP_FLAG_USES_CMM, &kvm->arch.gmap->flags);
>  
> -	return r;
> +	return r <= 0 ? r : -EFAULT;

[Severity: High]
This isn't a bug introduced by this patch, but since you are fixing the
leaked PGM_ADDRESSING error code here, should we also fix the same issue
in the storage key ioctls?

If userspace calls KVM_S390_GET_SKEYS or KVM_S390_SET_SKEYS with an
out-of-bounds guest address, do they also return the positive error code
directly?

arch/s390/kvm/kvm-s390.c:kvm_s390_get_skeys() {
    ...
    r = dat_get_storage_key(kvm->arch.gmap->asce, args->start_gfn + i, keys + i);
    if (r)
        break;
    ...
    return r;
}

arch/s390/kvm/kvm-s390.c:kvm_s390_set_skeys() {
    ...
    r = dat_set_storage_key(mc, kvm->arch.gmap->asce, args->start_gfn + i, keys[i], 0);
    if (r)
        break;
    ...
    return r;
}

Furthermore, for KVM_S390_KEYOP, does it falsely report success and assign
the error code to the output key?

arch/s390/kvm/kvm-s390.c:kvm_arch_vm_ioctl() {
    case KVM_S390_KEYOP: {
        ...
        r = kvm_s390_keyop(mc, kvm, kop.operation, kop.guest_addr, skey);
        ...
        if (r < 0)
            break;

        kop.key = r;
        r = 0;
        ...
}

Should these functions also be updated to return -EFAULT for positive errors,
just like kvm_s390_set_cmma_bits() does here?

>  }

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