Re: [PATCH v4 07/13] KVM: s390: Return -EINTR if a signal was pending while faulting-in

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Nested virtualization (vSIE) skips the SIE instruction and corrupts guest state when a signal interrupts the shadow page fault handler.
- [High] Instruction handlers using read_guest/write_guest (e.g., STSI, LPSW) silently skip execution if a signal is pending during page fault-in.
--

--- Patch [7]: [PATCH v4 07/13] KVM: s390: Return -EINTR if a signal was pending while faulting-in ---
commit 1e3636aa03c57bfbcde97da61dcda6802d420d61
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Return -EINTR if a signal was pending while faulting-in

If a signal is pending while trying to fault-in a page, return -EINTR
instead of -EAGAIN. Also fix unpack_one() and handle_mvpg_pei() so
that they can handle -EINTR properly.

> diff --git a/arch/s390/kvm/faultin.c b/arch/s390/kvm/faultin.c
> index fee80047bd94f..4ff4c59709566 100644
> --- a/arch/s390/kvm/faultin.c
> +++ b/arch/s390/kvm/faultin.c
> @@ -92,8 +92,8 @@ int kvm_s390_faultin_gfn(struct kvm_vcpu *vcpu, struct kvm *kvm, struct guest_fa
>  		if (is_noslot_pfn(f->pfn))
>  			return PGM_ADDRESSING;
>  		/* Signal pending: try again. */
> -		if (f->pfn == KVM_PFN_ERR_SIGPENDING)
> -			return -EAGAIN;
> +		if (is_sigpending_pfn(f->pfn))
> +			return -EINTR;

[Severity: High]
This is a pre-existing issue, but does propagating this error cause
nested virtualization (vSIE) to skip the SIE instruction when a signal
interrupts the shadow page fault handler?

If a signal is sent to the vCPU thread while handling a nested fault,
gaccess_shadow_fault() calls kvm_s390_get_guest_page(), which returns -EINTR.

This error propagates without rewinding the L2 guest PSW via
retry_vsie_icpt() or kvm_s390_rewind_psw():

arch/s390/kvm/vsie.c:vsie_handle_mvpg() {
    ...
    if (rc_dest < 0)
        return rc_dest;
    ...
}

arch/s390/kvm/vsie.c:vsie_run() {
    ...
    if (rc || scb_s->icptcode)
        break;
    ...
}

When userspace resumes KVM_RUN, does execution continue at the advanced PSW,
skipping the SIE instruction and corrupting L2 guest state? Does vSIE need
to be addressed similarly to how handle_mvpg_pei() is fixed in this patch?

>  		/* Check if it's read-only memory; don't try to actually handle that case. */
>  		if (f->pfn == KVM_PFN_ERR_RO_FAULT)
>  			return -EOPNOTSUPP;
> diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
> index 1980df61ef304..a40fe2a10cbfe 100644
> --- a/arch/s390/kvm/intercept.c
> +++ b/arch/s390/kvm/intercept.c
[ ... ]
> @@ -369,27 +369,30 @@ static int handle_mvpg_pei(struct kvm_vcpu *vcpu)
>  	if (rc)
>  		return kvm_s390_inject_prog_cond(vcpu, rc);
>  
> -	do {
> -		rc = kvm_s390_faultin_gfn_simple(vcpu, NULL, gpa_to_gfn(srcaddr), false);
> -	} while (rc == -EAGAIN);
> -	if (rc)
> -		return rc;
> -
> -	/* Ensure that the source is paged-in, no actual access -> no key checking */
> +	/* Ensure that the destination is paged-in, no actual access -> no key checking */
>  	rc = guest_translate_address_with_key(vcpu, vcpu->run->s.regs.gprs[reg1],
>  					      reg1, &dstaddr, GACC_STORE, 0);
>  	if (rc)
>  		return kvm_s390_inject_prog_cond(vcpu, rc);
>  
> +	/*
> +	 * If an error happens below, the instruction needs to be retried.
> +	 * Positive error codes are impossible; addressing exceptions would
> +	 * have been discovered and reported above.
> +	 */
> +	kvm_s390_retry_instr(vcpu);

[Severity: High]
This isn't a bug introduced by this patch, but while adding the instruction
retry logic here, do other instruction handlers using read_guest() or
write_guest() in arch/s390/kvm/priv.c also need this fix?

If a signal interrupts kvm_s390_faultin_gfn() during an instruction intercept,
the handler receives -EINTR (or -EAGAIN prior to this patch) and returns it
to userspace without calling kvm_s390_retry_instr().

Does this leave the guest's PSW at the instruction following the intercept,
causing the instruction to be skipped when userspace resumes the vCPU?

arch/s390/kvm/priv.c:handle_stsi() {
    ...
    if (rc) {
        rc = kvm_s390_inject_prog_cond(vcpu, rc);
        goto out;
    }
    ...
}

arch/s390/kvm/priv.c:kvm_s390_handle_lpsw() {
    ...
    rc = read_guest(vcpu, addr, ar, &new_psw, sizeof(new_psw));
    if (rc)
        return kvm_s390_inject_prog_cond(vcpu, rc);
    ...
}

> +
>  	do {
> -		rc = kvm_s390_faultin_gfn_simple(vcpu, NULL, gpa_to_gfn(dstaddr), true);
> +		rc = kvm_s390_faultin_gfn_simple(vcpu, NULL, gpa_to_gfn(srcaddr), false);
>  	} while (rc == -EAGAIN);
>  	if (rc)
>  		return rc;

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