Re: [PATCH v4 3/7] KVM: s390: Do not set special large pages dirty

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Missing Special Page Validations in PV / Unmap Paths
- [Critical] Unsafe SSKE Execution on Special Pages
--

commit 2d60ecc18379805e67b3a6ad94ddb8516f4d4167
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Do not set special large pages dirty

This commit prevents setting special large pages dirty. It adds a missing
check in gmap_clear_young_crste() to ensure the large page is not a
special page before setting it dirty.

> diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
> --- a/arch/s390/kvm/gmap.c
> +++ b/arch/s390/kvm/gmap.c
> @@ -327,7 +327,7 @@ static long gmap_clear_young_crste(union crste *crstep, gfn_t gfn, gfn_t end, st
>  		new.h.i = 1;
>  		new.s.fc1.y = 0;
>  		new.s.fc1.prefix_notif = 0;
> -		if (new.s.fc1.d || !new.h.p)
> +		if ((new.s.fc1.d || !new.h.p) && !new.s.fc1.s)
>  			folio_set_dirty(phys_to_folio(crste_origin_large(crste)));

[Severity: Critical]
This is a pre-existing issue, but I noticed there are other places where
special page validation seems to be missing in PV and unmap paths.

If a malicious userspace maps device memory (VM_PFNMAP) to a PV guest,
special shadow page table entries are created. When KVM unmaps these pages,
_gmap_unmap_crste() calls phys_to_folio() on the physical frame number:

arch/s390/kvm/gmap.c:_gmap_unmap_crste() {
    ...
    if (old.s.fc1.pr && test_bit(GMAP_FLAG_EXPORT_ON_UNMAP, &priv->gmap->flags))
        folio = phys_to_folio(crste_origin_large(old));
    ...
}

Could this result in a garbage pointer if the entry is a special page,
leading to a host panic when passed to uv_convert_from_secure_folio()
which immediately calls folio_get()?

A similar pattern appears in _gmap_unmap_pte() without checking ptep->s.s:

arch/s390/kvm/gmap.c:_gmap_unmap_pte() {
    ...
    if (ptep->s.pr && test_bit(GMAP_FLAG_EXPORT_ON_UNMAP, &priv->gmap->flags))
        folio = pfn_folio(ptep->h.pfra);
    ...
}

And in _destroy_pages_crste():

arch/s390/kvm/gmap.c:_destroy_pages_crste() {
    ...
    for ( ; cur < end; cur += PAGE_SIZE)
        __kvm_s390_pv_destroy_page(phys_to_page(cur));
    ...
}

As well as _destroy_pages_pte() and _kvm_s390_pv_make_secure() in
arch/s390/kvm/pv.c. Do these paths also need special bit checks to
prevent panics from malicious userspace or guests?


[Severity: Critical]
This is also a pre-existing issue, but does KVM_S390_RESET_SKEYS safely
handle special pages?

When userspace triggers the KVM_S390_RESET_SKEYS ioctl, dat_reset_skeys_pte()
and dat_reset_skeys_crste() walk the gmap and invoke page_set_storage_key()
without checking if the page is special. For example:

arch/s390/kvm/dat.c:dat_reset_skeys_pte() {
    ...
    if (ptep->s.pr)
        page_set_storage_key(pte_origin(*ptep), PAGE_DEFAULT_KEY, 1);
    ...
}

arch/s390/kvm/dat.c:dat_reset_skeys_crste() {
    ...
    for ( ; addr < end; addr += PAGE_SIZE)
        page_set_storage_key(addr, PAGE_DEFAULT_KEY, 1);
    ...
}

If page_set_storage_key() executes the hardware sske instruction on a physical
address not backed by valid system RAM, could this cause an s390 Addressing
Exception and panic the host?

>  		new.s.fc1.d = 0;
>  		new.h.p = 1;

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