Re: [PATCH v2 2/7] KVM: s390: Return -EFAULT instead of PGM_ADDRESSING

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch fails to fix the `KVM_S390_KEYOP` ioctl, which silently ignores and leaks positive program exception codes (e.g., `PGM_ADDRESSING`) to userspace in the exact same way as the patched functions.
--

--- Patch [2]: [PATCH v2 2/7] KVM: s390: Return -EFAULT instead of PGM_ADDRESSING ---
commit 4941b5a35b7857c25a5086d12db876b148a9642d
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Return -EFAULT instead of PGM_ADDRESSING

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. Same issue with kvm_s390_{g,s}et_skeys().

Fix by returning -EFAULT whenever the return value would be > 0, which
is consistent with the behaviour before the gmap rewrite.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 374dae6dae81b..53691aaf6534c 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2212,7 +2212,7 @@ static int kvm_s390_get_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
>  	}
>  
>  	kvfree(keys);
> -	return r;
> +	return r <= 0 ? r : -EFAULT;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the same issue exist in
kvm_s390_keyop() and the KVM_S390_KEYOP ioctl handler?

When memory functions like dat_get_storage_key() return PGM_ADDRESSING (5),
kvm_s390_keyop() passes the positive exception code back to its caller without
converting it to -EFAULT:

arch/s390/kvm/kvm-s390.c:kvm_s390_keyop() {
    ...
	case KVM_S390_KEYOP_ISKE:
		r = dat_get_storage_key(asce, gfn, &skey);
		if (!r)
			return skey.skey;
		break;
    ...
	return r;
}

The KVM_S390_KEYOP handler then treats any positive return value from
kvm_s390_keyop() as a success, silently leaking the exception code into
kop.key and returning 0 to userspace:

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

		kop.key = r;
		r = 0;
		if (copy_to_user(argp, &kop, sizeof(kop)))
    ...
}

Since kvm_s390_keyop() conflates valid storage keys with exception codes,
userspace is fundamentally unable to detect that the memory access failed.

Should KVM_S390_KEYOP be updated to handle positive return codes similarly
to the functions fixed in this patch?

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