Re: [PATCH v3 5/6] s390/mm: Remove folio handling for kernel faults in do_secure_storage_access()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Kernel accesses with exception fixups (e.g., speculative reads or probe reads) unconditionally destroy active secure guest pages because uv_convert_from_secure() is executed before checking fixup_exception().
--

--- Patch [5]: [PATCH v3 5/6] s390/mm: Remove folio handling for kernel faults in do_secure_storage_access() ---
commit 475be2dbf4868b390a06f3b6bd78b394674a341a
Author: Heiko Carstens <hca@linux.ibm.com>

s390/mm: Remove folio handling for kernel faults in do_secure_storage_access()

This commit removes folio handling and reference counting for secure
storage access exceptions triggered via the kernel mapping. It replaces
the folio_to_phys() conversion with a direct __pa(addr) translation to
avoid data corruption from a known race condition.

> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index e650cdfa25231..776549d55c94a 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -440,13 +440,7 @@ void do_secure_storage_access(struct pt_regs *regs)
>  	if (is_kernel_fault(regs)) {
>  		if (is_vmalloc_addr((void *)addr))
>  			return handle_fault_error_nolock(regs, 0);
> -		folio = virt_to_folio((void *)addr);
> -		if (unlikely(!folio_try_get(folio)))
> -			return;
> -		rc = uv_convert_from_secure(folio_to_phys(folio));
> -		if (!rc)
> -			clear_bit(PG_arch_1, &folio->flags.f);
> -		folio_put(folio);
> +		rc = uv_convert_from_secure(__pa(addr));

[Severity: Critical]
This is a pre-existing issue, but does this unconditionally destroy active
secure guest pages on speculative reads?

Looking at do_secure_storage_access(), uv_convert_from_secure() is called
here before checking fixup_exception(). Because the Ultravisor allows the
host to reclaim active guest memory, uv_convert_from_secure() succeeds for
guest memory, scrubs the page, and returns 0.

Since the conversion succeeds, the code skips calling
handle_fault_error_nolock() where the fixup is actually evaluated:

arch/s390/mm/fault.c:do_secure_storage_access() {
        ...
        rc = uv_convert_from_secure(__pa(addr));
        /*
         * There are some valid fixup types for kernel
         * accesses to donated secure memory. zeropad is one
         * of them.
         */
        if (rc)
                return handle_fault_error_nolock(regs, 0);
        ...
}

If a speculative kernel read (like the mentioned load_unaligned_zeropad)
or an eBPF probe read targets an address in the direct map belonging to a
secure guest, wouldn't this silently destroy the guest's memory instead of
safely executing the fixup?

>  		/*
>  		 * There are some valid fixup types for kernel
>  		 * accesses to donated secure memory. zeropad is one

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719130006.3882764-1-hca@linux.ibm.com?part=5