Re: [PATCH v2 2/4] s390/mm: Fix handling of secure storage access exceptions in vmalloc area

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] `do_secure_storage_access` uses `find_vma()` to locate the VMA but fails to verify that the faulting address is within the returned VMA's bounds, allowing an unprivileged user to reliably trigger a `WARN_ON_ONCE` via a race condition.
--

--- Patch [2]: [PATCH v2 2/4] s390/mm: Fix handling of secure storage access exceptions in vmalloc area ---
commit 09ae7bc2c3bf1f10251c33af84acf243e29af836
Author: Heiko Carstens <hca@linux.ibm.com>

s390/mm: Fix handling of secure storage access exceptions in vmalloc area

Secure storage access exceptions in vmalloc area need to be handled
separately since folio_try_get() or virt_to_phys() do not work on such
addresses. Given that there cannot be a valid access let such exceptions
fail immediately. The kernel will either fixup or crash.

> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index 028aeb9c48d6f..392e467420392 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -438,6 +438,8 @@ void do_secure_storage_access(struct pt_regs *regs)
>  		panic("Unexpected PGM 0x3d with TEID bit 61=0");
>  	}
>  	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;

[ ... ]

>  	} else {
>  		if (faulthandler_disabled())
>  			return handle_fault_error_nolock(regs, 0);
>  		mm = current->mm;
>  		mmap_read_lock(mm);
>  		vma = find_vma(mm, addr);
>  		if (!vma)
>  			return handle_fault_error(regs, SEGV_MAPERR);
>  		folio = folio_walk_start(&fw, vma, addr, 0);

[Severity: High]
This is a pre-existing issue, but does using find_vma() here allow a race
condition that could trigger a kernel warning in folio_walk_start()?

find_vma() returns the first VMA where vma->vm_end > addr, but it doesn't
guarantee that vma->vm_start <= addr. If a concurrent unmap happens and the
exact VMA is removed before mmap_read_lock() is acquired, find_vma() might
return the next VMA in the address space.

Passing an out-of-bounds VMA to folio_walk_start() violates its API contract
and triggers a WARN_ON_ONCE().

Would using vma_lookup() instead properly validate that the address falls
within the VMA's bounds?

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