Re: [PATCH v5 12/21] KVM: s390: vsie: Add helper unshadow_intercept()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] unshadow_intercept() writes uninitialized or stale shadow SCB fields (gbea and icptstatus) back to the guest's original SCB on early validity intercepts, causing guest state corruption.
--

commit 065588b108c97fdd6441ebcc560b6bd343203830
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>

KVM: s390: vsie: Add helper unshadow_intercept()

This commit extracts the unshadow_intercept() helper from unshadow_scb().
The new helper only copies intercept information and avoids clobbering the
timer and guest-state fields, to be used when the rest of the scb is not yet
shadowed.

> diff --git a/arch/s390/kvm/s390/vsie.c b/arch/s390/kvm/s390/vsie.c
> index 5c4f13777178b..988370f491bf5 100644
> --- a/arch/s390/kvm/s390/vsie.c
> +++ b/arch/s390/kvm/s390/vsie.c
> @@ -565,18 +565,27 @@ static void prepare_ibc(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  	}
>  }
>  
> -/* unshadow the scb, copying parameters back to the real scb */
> -static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> +/*
> + * Only write the interception fields back; avoid clobbering the timer and
> + * guest-state fields in scb_o.
> + */
> +static inline void unshadow_intercept(struct kvm_s390_sie_block *scb_o,
> +				      struct kvm_s390_sie_block *scb_s)
>  {
> -	struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
> -	struct kvm_s390_sie_block *scb_o = vsie_page->scb_o;
> -
> -	/* interception */
>  	scb_o->icptcode = scb_s->icptcode;
>  	scb_o->icptstatus = scb_s->icptstatus;
>  	scb_o->ipa = scb_s->ipa;
>  	scb_o->ipb = scb_s->ipb;
>  	scb_o->gbea = scb_s->gbea;
> +}

[ ... ]

> @@ -755,10 +764,13 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  
>  	prepare_ibc(vcpu, vsie_page);
>  	rc = shadow_crycb(vcpu, vsie_page);
> -out:
>  	if (rc)
>  		unshadow_scb(vcpu, vsie_page);
>  	return rc;
> +
> +out_validity:
> +	unshadow_intercept(vsie_page->scb_o, &vsie_page->scb_s);

[Severity: High]
This is a pre-existing issue, but does calling unshadow_intercept() here
corrupt the L1 guest state by writing uninitialized or stale shadow SCB fields
back to the guest's original SCB?

When shadow_scb() fails early in prepare_cpuflags() (e.g., due to an L1 guest
setting an invalid CPU flag), it jumps to out_validity and calls
unshadow_intercept().

However, because gbea and icptstatus are not copied from scb_o to scb_s until
much later in shadow_scb(), unshadow_intercept() will write the stale values
from scb_s back into the guest's scb_o, explicitly clobbering the L1 guest's
changes.

An L1 guest could potentially trigger this trivially by forcing an early validity
intercept.

> +	return rc;
>  }
>  
>  /* unpin the scb provided by guest 2, marking it as dirty */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-vsie-sigpi-v5-0-cf471edb46b2@linux.ibm.com?part=12