Re: [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing earlyclobber constraint on the output register in `load_unaligned_zeropad` allows GCC to use the same register for input and output, leading to a kernel crash during secure storage access exception handling.
--

--- Patch [4]: [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again) ---
commit 48318c86a5b130c4f8df66b5d5c16dda32e7448d
Author: Heiko Carstens <hca@linux.ibm.com>

s390: Add support for DCACHE_WORD_ACCESS (again)

This commit implements load_unaligned_zeropad() and enables
DCACHE_WORD_ACCESS to speed up string operations in fs/dcache.c and
fs/namei.c. It reinstates this support after addressing secure storage
access exception cases.

> diff --git a/arch/s390/include/asm/word-at-a-time.h b/arch/s390/include/asm/word-at-a-time.h
> --- a/arch/s390/include/asm/word-at-a-time.h
> +++ b/arch/s390/include/asm/word-at-a-time.h

[ ... ]

> +static inline unsigned long load_unaligned_zeropad(const void *addr)
> +{
> +	unsigned long data;
> +
> +	asm_inline volatile(
> +		"0:	lg	%[data],0(%[addr])\n"
> +		"1:	nopr	%%r7\n"
> +		EX_TABLE_ZEROPAD(0b, 1b, %[data], %[addr])
> +		EX_TABLE_ZEROPAD(1b, 1b, %[data], %[addr])
> +		: [data] "=d" (data)

[Severity: High]
Could GCC allocate the same register for both the [addr] input and the
[data] output since there is no earlyclobber modifier here?

If a page-crosser triggers a terminating secure storage access exception,
the hardware leaves the result register in an unpredictable state.

The exception handler ex_handler_zeropad() relies on reading the original
address from the register specified by [addr] to compute the valid bytes.
If GCC uses the exact same register for both input and output, the handler
will read a corrupted address from the clobbered register, dereference it,
and cause a kernel panic.

Should this use the earlyclobber constraint "=&d" for the data output to
ensure the address register is preserved during a terminating exception?

> +		: [addr] "a" (addr), "m" (*(unsigned long *)addr));
> +	return data;
> +}
> +
>  #endif /* _ASM_WORD_AT_A_TIME_H */

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