Re: [PATCH v3 4/6] s390/mm: Fix handling of vmalloc area in do_secure_storage_access()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Returning from the trap handler without fixing the exception when `folio_try_get()` fails causes an infinite kernel loop.
- [High] Kernel-mode accesses (e.g., `copy_from_user`) that fail to convert a secure folio will infinite loop due to improper signal handling.
--

--- Patch [4]: [PATCH v3 4/6] s390/mm: Fix handling of vmalloc area in do_secure_storage_access() ---
commit 1759378fac324776e9a7cbd8390b8445f3bb360c
Author: Heiko Carstens <hca@linux.ibm.com>

s390/mm: Fix handling of vmalloc area in do_secure_storage_access()

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
> --- 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;

[Severity: High]
This isn't a bug introduced by this patch, but returning directly here in
do_secure_storage_access() without fixing the exception state when
folio_try_get() fails seems problematic.

Since PGM 0x3d is a suppressing exception on s390, the instruction pointer
isn't advanced. Returning without modifying the page state or adjusting the
instruction pointer (e.g., via fixup_exception()) will cause the hardware
to immediately re-execute the faulting instruction upon returning to the
interrupted context.

Will this trigger the exact same exception repeatedly and result in a hard
kernel lockup? Should this fail gracefully by calling
handle_fault_error_nolock(regs, 0) instead, so that exception tables can
be processed or a proper Oops generated?

[ ... ]

> 		rc = arch_make_folio_accessible(folio);
> 		folio_put(folio);
> 		folio_walk_end(&fw, vma);
> 		if (rc)
> 			send_sig(SIGSEGV, current, 0);
> 		mmap_read_unlock(mm);

[Severity: High]
This is also a pre-existing issue, but does this error path in
do_secure_storage_access() correctly handle kernel-mode accesses that fail
to convert a secure folio?

For accesses to user-space memory, is_kernel_fault() returns false even if
the access was performed by the kernel (e.g., copy_from_user() using secondary
space). If arch_make_folio_accessible() fails, the code queues a signal and
returns.

Because the fault happened in kernel mode, returning doesn't immediately
deliver the signal to user space. It resumes kernel execution and re-executes
the faulting copy_from_user() instruction, hitting the secure page again and
triggering another PGM 0x3d exception.

Can this result in an infinite kernel loop repeatedly queuing SIGSEGVs without
making forward progress? Should this path check !user_mode(regs) and call
handle_fault_error_nolock() (or equivalent) to apply fixup_exception() for
kernel-mode accesses?

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