Re: [PATCH] s390/signal: Fix incorrect return address in signal frame

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/signal: patch breaks -EINTR for -ERESTART_RESTARTBLOCK
--

commit 158e48eb13e787b0b5c2f432c9056345d015870e
Author: Heiko Carstens <hca@linux.ibm.com>

s390/signal: Fix incorrect return address in signal frame

This commit fixes an issue in arch_do_signal_or_restart() where a second call
builds a signal frame with the wrong return address if the first call set up a
vDSO system call restart. It addresses this by restoring psw.addr from
arch_data before building the signal frame.

> diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
> index 4874de5edea0a..25781d4a0ad0c 100644
> --- a/arch/s390/kernel/signal.c
> +++ b/arch/s390/kernel/signal.c
> @@ -475,7 +475,10 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
>  		}
>  		/* No longer in a system call */
>  		clear_pt_regs_flag(regs, PIF_SYSCALL);
> -
> +		if (!(current->restart_block.arch_data & 1)) {
> +			regs->psw.addr = current->restart_block.arch_data;
> +			current->restart_block.arch_data = 1;
> +		}

[Severity: High]
Does this code break the -EINTR return value for system calls returning
-ERESTART_RESTARTBLOCK when interrupted by a signal?

In arch_do_signal_or_restart(), the first call without handlers might set up a
vDSO restart (setting regs->gprs[2] = regs->orig_gpr2) and clear PIF_SYSCALL.

If a signal with a handler arrives before returning to user mode, a second
call occurs. Because PIF_SYSCALL was cleared, current->thread.system_call
evaluates to 0:

arch/s390/kernel/signal.c:arch_do_signal_or_restart() {
    ...
	current->thread.system_call =
		test_pt_regs_flag(regs, PIF_SYSCALL) ? regs->int_code : 0;

This bypasses the block that would normally set regs->gprs[2] = -EINTR:

	if (get_signal(&ksig)) {
		/* Whee!  Actually deliver the signal.  */
		if (current->thread.system_call) {
    ...

The newly added code restores regs->psw.addr to the original return address,
bypassing the vDSO stub. Without the vDSO stub executing sys_restart_syscall
(which would return -EINTR), the syscall instead returns orig_gpr2.

Could this lead to system calls like nanosleep returning their first argument
instead of -EINTR, breaking userspace error handling and POSIX compliance?

>  		rseq_signal_deliver(&ksig, regs);
>  		handle_signal(&ksig, oldset, regs);
>  		return;

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