Linux s390 Architecture development
 help / color / mirror / Atom feed
* [PATCH] s390/signal: Fix incorrect return address in signal frame
@ 2026-10-12 15:33 Heiko Carstens
  2026-10-12 15:50 ` sashiko-bot
  2026-10-13  7:07 ` Sven Schnelle
  0 siblings, 2 replies; 4+ messages in thread
From: Heiko Carstens @ 2026-10-12 15:33 UTC (permalink / raw)
  To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
	Christian Borntraeger
  Cc: linux-kernel, linux-s390

exit_to_user_mode_loop() can call arch_do_signal_or_restart() multiple
times when signals are pending.

If the first call set up a vDSO system call restart by saving the real
user return address and adjusting psw.addr so it points to the vDSO stub,
a second call builds the signal frame with the wrong return address.

Restore psw.addr from arch_data before building the signal frame.

Fixes: df29a7440c4b ("s390/signal: switch to using vdso for sigreturn and syscall restart")
Cc: stable@vger.kernel.org
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/signal.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
index 4874de5edea0..25781d4a0ad0 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;
+		}
 		rseq_signal_deliver(&ksig, regs);
 		handle_signal(&ksig, oldset, regs);
 		return;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] s390/signal: Fix incorrect return address in signal frame
  2026-10-12 15:33 [PATCH] s390/signal: Fix incorrect return address in signal frame Heiko Carstens
@ 2026-10-12 15:50 ` sashiko-bot
  2026-10-12 16:11   ` Heiko Carstens
  2026-10-13  7:07 ` Sven Schnelle
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-10-12 15:50 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Alexander Gordeev, Heiko Carstens, Christian Borntraeger,
	linux-s390, Vasily Gorbik

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] s390/signal: Fix incorrect return address in signal frame
  2026-10-12 15:50 ` sashiko-bot
@ 2026-10-12 16:11   ` Heiko Carstens
  0 siblings, 0 replies; 4+ messages in thread
From: Heiko Carstens @ 2026-10-12 16:11 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Alexander Gordeev, Christian Borntraeger, linux-s390,
	Vasily Gorbik

On Mon, Oct 12, 2026 at 03:50:03PM +0000, sashiko-bot@kernel.org wrote:
> 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;
> > +		}
...
> 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?

Correct. The whole point of this patch was to return -EINTR instead of
(incorrectly) restarting the system call, which is the most likely of the
unlikely scenarios. One could construct more obscure scenarios where user
space would end up with a SIGILL, but that's extremely unlikely.

Anyway, the -EINTR return code handling is broken with this patch.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] s390/signal: Fix incorrect return address in signal frame
  2026-10-12 15:33 [PATCH] s390/signal: Fix incorrect return address in signal frame Heiko Carstens
  2026-10-12 15:50 ` sashiko-bot
@ 2026-10-13  7:07 ` Sven Schnelle
  1 sibling, 0 replies; 4+ messages in thread
From: Sven Schnelle @ 2026-10-13  7:07 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Alexander Gordeev, Vasily Gorbik, Christian Borntraeger,
	linux-kernel, linux-s390

Heiko Carstens <hca@linux.ibm.com> writes:

> exit_to_user_mode_loop() can call arch_do_signal_or_restart() multiple
> times when signals are pending.
>
> If the first call set up a vDSO system call restart by saving the real
> user return address and adjusting psw.addr so it points to the vDSO stub,
> a second call builds the signal frame with the wrong return address.
>
> Restore psw.addr from arch_data before building the signal frame.
>
> Fixes: df29a7440c4b ("s390/signal: switch to using vdso for sigreturn and syscall restart")
> Cc: stable@vger.kernel.org
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/kernel/signal.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
> index 4874de5edea0..25781d4a0ad0 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;
> +		}
>  		rseq_signal_deliver(&ksig, regs);
>  		handle_signal(&ksig, oldset, regs);
>  		return;

Reviewed-by: Sven Schnelle <svens@linux.ibm.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-13  7:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 15:33 [PATCH] s390/signal: Fix incorrect return address in signal frame Heiko Carstens
2026-10-12 15:50 ` sashiko-bot
2026-10-12 16:11   ` Heiko Carstens
2026-10-13  7:07 ` Sven Schnelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox