Re: [PATCH 04/21] KVM: s390: vsie: Add SCAO read and write helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] read_scao() incorrectly truncates the 64-bit Host Physical Address (HPA) when unpinning the shadow SCB if the guest lacks the 64BSCAO feature, leading to host memory corruption and leaked pages.
--

--- Patch [4]: [PATCH 04/21] KVM: s390: vsie: Add SCAO read and write helpers ---
commit ef50a3af344aad608f5c9779f347c6f56923a4f5
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>

KVM: s390: vsie: Add SCAO read and write helpers

This commit introduces helper functions to get and set the system control
area origin address from the SIE control block.

> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 937f9c99bc002..aa0cebece2d50 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -72,6 +72,22 @@ struct vsie_page {
>  
>  static_assert(sizeof(struct vsie_page) == PAGE_SIZE);
>  
> +static unsigned long read_scao(struct kvm *kvm, struct kvm_s390_sie_block *scb)
> +{
> +	unsigned long vsie_sca = READ_ONCE(scb->scaol) & ~0xfUL;
> +
> +	if (test_kvm_cpu_feat(kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
> +		vsie_sca |= (u64)READ_ONCE(scb->scaoh) << 32;
> +
> +	return vsie_sca;
> +}
> +
> +static void write_scao(struct kvm_s390_sie_block *scb, unsigned long hpa)
> +{
> +	scb->scaoh = (u32)((u64)hpa >> 32);
> +	scb->scaol = (u32)(u64)hpa;
> +}
> +

[ ... ]

> @@ -712,12 +728,11 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  	struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
>  	hpa_t hpa;
>  
> -	hpa = (u64) scb_s->scaoh << 32 | scb_s->scaol;
> +	hpa = read_scao(vcpu->kvm, scb_s);

[Severity: Critical]
Does this truncate the 64-bit Host Physical Address (HPA) when unpinning
the shadow SCB if the guest lacks the 64BSCAO feature?

When a guest is instantiated without KVM_S390_VM_CPU_FEAT_64BSCAO, KVM will
still pin the guest's SCA page to an HPA that can be > 4GB, writing the full
64-bit address to the shadow SCB via write_scao().

During unpin_blocks(), read_scao() will conditionally mask off the upper 32
bits because the guest lacks the CPU feature.

Could this cause unpin_guest_page() to receive a truncated, invalid address,
causing the host to decrement the reference count of the wrong host page and
permanently leak the originally pinned page?

>  	if (hpa) {
>  		unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
>  		vsie_page->sca_gpa = 0;
> -		scb_s->scaol = 0;
> -		scb_s->scaoh = 0;
> +		write_scao(scb_s, 0);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=4