* [PATCH RFC 0/2] s390: keep syscall number / return value in separate pt_regs members
@ 2026-07-15 13:38 Sven Schnelle
2026-07-15 13:38 ` [PATCH 1/2] s390/syscall: Keep syscall return in extra ptregs member Sven Schnelle
2026-07-15 13:38 ` [PATCH 2/2] s390/syscall: Keep syscall number " Sven Schnelle
0 siblings, 2 replies; 6+ messages in thread
From: Sven Schnelle @ 2026-07-15 13:38 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev
Cc: H. Peter Anvin, Thomas Gleixner, Christian Borntraeger,
Michal Suchánek, Peter Zijlstra, linux-s390, linux-kernel
After a recent discussion on the ML about syscall semantics wrt. pre-initializing
the return value to -ENOSYS and the special handling s390 has, I made two patches to
clean that up. See the commit messages for more details.
The second patch is just a cleanup and could be dropped if not wanted.
Sven Schnelle (2):
s390/syscall: Keep syscall return in extra ptregs member
s390/syscall: Keep syscall number in extra ptregs member
arch/s390/include/asm/ptrace.h | 9 ++++++---
arch/s390/include/asm/syscall.h | 13 +++++++------
arch/s390/kernel/ptrace.c | 2 +-
arch/s390/kernel/signal.c | 12 +++++-------
arch/s390/kernel/syscall.c | 26 +++++++-------------------
5 files changed, 26 insertions(+), 36 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] s390/syscall: Keep syscall return in extra ptregs member
2026-07-15 13:38 [PATCH RFC 0/2] s390: keep syscall number / return value in separate pt_regs members Sven Schnelle
@ 2026-07-15 13:38 ` Sven Schnelle
2026-07-15 14:04 ` sashiko-bot
2026-07-15 13:38 ` [PATCH 2/2] s390/syscall: Keep syscall number " Sven Schnelle
1 sibling, 1 reply; 6+ messages in thread
From: Sven Schnelle @ 2026-07-15 13:38 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev
Cc: H. Peter Anvin, Thomas Gleixner, Christian Borntraeger,
Michal Suchánek, Peter Zijlstra, linux-s390, linux-kernel
On s390, both syscall number and return value share ptregs::gprs[2].
Before executing the syscall, gpr[2] contains the syscall number,
and after execution it gets the return value assigned.
This is problematic with tracing: When either seccomp or ptrace set
a return value and the syscall number to -1 to skip the syscall, the
return value will be overwritten. If the return value is positive,
it's impossible to determine that the syscall should be skipped.
During conversion to generic entry a PIF_SYSCALL_RET_SET flag was
introduced which is set when syscall_set_return_value() is executed.
Peter Zijlstra proposed in [1] to introduce a new member in ptregs
which keeps the return value and copy this to ptregs::gprs[2] before
exit to userspace instead.
To avoid increasing ptregs size add it to the union which contains
int_parm_long and struct tpi_info as these members are not used during
syscalls.
[1] https://lore.kernel.org/all/20260703105718.GO751831@noisy.programming.kicks-ass.net/
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
arch/s390/include/asm/ptrace.h | 9 ++++++---
arch/s390/include/asm/syscall.h | 5 ++---
arch/s390/kernel/syscall.c | 15 +++------------
3 files changed, 11 insertions(+), 18 deletions(-)
diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
index 495e310c3d6d..f39b349da9a7 100644
--- a/arch/s390/include/asm/ptrace.h
+++ b/arch/s390/include/asm/ptrace.h
@@ -15,9 +15,8 @@
#define PIF_SYSCALL 0 /* inside a system call */
#define PIF_PSW_ADDR_ADJUSTED 1 /* psw address has been adjusted */
-#define PIF_SYSCALL_RET_SET 2 /* return value was set via ptrace */
-#define PIF_GUEST_FAULT 3 /* indicates program check in sie64a */
-#define PIF_FTRACE_FULL_REGS 4 /* all register contents valid (ftrace) */
+#define PIF_GUEST_FAULT 2 /* indicates program check in sie64a */
+#define PIF_FTRACE_FULL_REGS 3 /* all register contents valid (ftrace) */
#define _PIF_SYSCALL BIT(PIF_SYSCALL)
#define _PIF_ADDR_PSW_ADJUSTED BIT(PIF_PSW_ADDR_ADJUSTED)
@@ -130,6 +129,10 @@ struct pt_regs {
unsigned int int_parm;
unsigned long int_parm_long;
};
+ struct {
+ unsigned long __unused;
+ unsigned long syscall_ret;
+ };
struct tpi_info tpi_info;
};
unsigned long flags;
diff --git a/arch/s390/include/asm/syscall.h b/arch/s390/include/asm/syscall.h
index 4271e4169f45..22b6ed20a9ae 100644
--- a/arch/s390/include/asm/syscall.h
+++ b/arch/s390/include/asm/syscall.h
@@ -52,15 +52,14 @@ static inline long syscall_get_error(struct task_struct *task,
static inline long syscall_get_return_value(struct task_struct *task,
struct pt_regs *regs)
{
- return regs->gprs[2];
+ return regs->syscall_ret;
}
static inline void syscall_set_return_value(struct task_struct *task,
struct pt_regs *regs,
int error, long val)
{
- set_pt_regs_flag(regs, PIF_SYSCALL_RET_SET);
- regs->gprs[2] = error ? error : val;
+ regs->syscall_ret = error ? error : val;
}
static inline void syscall_get_arguments(struct task_struct *task,
diff --git a/arch/s390/kernel/syscall.c b/arch/s390/kernel/syscall.c
index 75d5a3cab14e..ce244dceec6d 100644
--- a/arch/s390/kernel/syscall.c
+++ b/arch/s390/kernel/syscall.c
@@ -117,25 +117,16 @@ void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
regs->int_code |= nr;
}
regs->gprs[2] = nr;
+ regs->syscall_ret = -ENOSYS;
if (nr == __NR_restart_syscall && !(current->restart_block.arch_data & 1)) {
regs->psw.addr = current->restart_block.arch_data;
current->restart_block.arch_data = 1;
}
nr = syscall_enter_from_user_mode_work(regs, nr);
- /*
- * In the s390 ptrace ABI, both the syscall number and the return value
- * use gpr2. However, userspace puts the syscall number either in the
- * svc instruction itself, or uses gpr1. To make at least skipping syscalls
- * work, the ptrace code sets PIF_SYSCALL_RET_SET, which is checked here
- * and if set, the syscall will be skipped.
- */
- if (unlikely(test_and_clear_pt_regs_flag(regs, PIF_SYSCALL_RET_SET)))
- goto out;
- regs->gprs[2] = -ENOSYS;
if (likely(nr < NR_syscalls)) {
nr = array_index_nospec(nr, NR_syscalls);
- regs->gprs[2] = sys_call_table[nr](regs);
+ regs->syscall_ret = sys_call_table[nr](regs);
}
-out:
+ regs->gprs[2] = regs->syscall_ret;
syscall_exit_to_user_mode(regs);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] s390/syscall: Keep syscall number in extra ptregs member
2026-07-15 13:38 [PATCH RFC 0/2] s390: keep syscall number / return value in separate pt_regs members Sven Schnelle
2026-07-15 13:38 ` [PATCH 1/2] s390/syscall: Keep syscall return in extra ptregs member Sven Schnelle
@ 2026-07-15 13:38 ` Sven Schnelle
2026-07-15 13:59 ` Heiko Carstens
2026-07-15 14:10 ` sashiko-bot
1 sibling, 2 replies; 6+ messages in thread
From: Sven Schnelle @ 2026-07-15 13:38 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev
Cc: H. Peter Anvin, Thomas Gleixner, Christian Borntraeger,
Michal Suchánek, Peter Zijlstra, linux-s390, linux-kernel
Move the syscall number from ptregs::int_code to another union member
called syscall_nr. This simplifies the code a bit.
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
arch/s390/include/asm/ptrace.h | 2 +-
arch/s390/include/asm/syscall.h | 8 +++++---
arch/s390/kernel/ptrace.c | 2 +-
arch/s390/kernel/signal.c | 12 +++++-------
arch/s390/kernel/syscall.c | 11 ++++-------
5 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
index f39b349da9a7..2edc85ddcba8 100644
--- a/arch/s390/include/asm/ptrace.h
+++ b/arch/s390/include/asm/ptrace.h
@@ -130,7 +130,7 @@ struct pt_regs {
unsigned long int_parm_long;
};
struct {
- unsigned long __unused;
+ unsigned long syscall_nr;
unsigned long syscall_ret;
};
struct tpi_info tpi_info;
diff --git a/arch/s390/include/asm/syscall.h b/arch/s390/include/asm/syscall.h
index 22b6ed20a9ae..abb0d4348912 100644
--- a/arch/s390/include/asm/syscall.h
+++ b/arch/s390/include/asm/syscall.h
@@ -14,13 +14,15 @@
#include <linux/err.h>
#include <asm/ptrace.h>
+#define SVC_LEN 2
extern const sys_call_ptr_t sys_call_table[];
static inline long syscall_get_nr(struct task_struct *task,
struct pt_regs *regs)
{
- return test_pt_regs_flag(regs, PIF_SYSCALL) ?
- (regs->int_code & 0xffff) : -1;
+ if (!test_pt_regs_flag(regs, PIF_SYSCALL))
+ return -1;
+ return regs->syscall_nr;
}
static inline void syscall_set_nr(struct task_struct *task,
@@ -32,7 +34,7 @@ static inline void syscall_set_nr(struct task_struct *task,
* the target task is stopped for tracing on entering syscall, so
* there is no need to have the same check syscall_get_nr() has.
*/
- regs->int_code = (regs->int_code & ~0xffff) | (nr & 0xffff);
+ regs->syscall_nr = nr;
}
static inline void syscall_rollback(struct task_struct *task,
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index 125ca4c4e30c..00e825d01504 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -349,7 +349,7 @@ static int __poke_user(struct task_struct *child, addr_t addr, addr_t data)
addr == offsetof(struct user, regs.gprs[2])) {
struct pt_regs *regs = task_pt_regs(child);
- regs->int_code = 0x20000 | (data & 0xffff);
+ regs->syscall_nr = data;
}
*(addr_t *)((addr_t) ®s->psw + addr) = data;
} else if (addr < offsetof(struct user, regs.orig_gpr2)) {
diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
index 4874de5edea0..c3c69294ff33 100644
--- a/arch/s390/kernel/signal.c
+++ b/arch/s390/kernel/signal.c
@@ -447,12 +447,12 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
* call information.
*/
current->thread.system_call =
- test_pt_regs_flag(regs, PIF_SYSCALL) ? regs->int_code : 0;
+ test_pt_regs_flag(regs, PIF_SYSCALL) ? regs->syscall_nr : 0;
if (get_signal(&ksig)) {
/* Whee! Actually deliver the signal. */
if (current->thread.system_call) {
- regs->int_code = current->thread.system_call;
+ regs->syscall_nr = current->thread.system_call;
/* Check for system call restarting. */
switch (regs->gprs[2]) {
case -ERESTART_RESTARTBLOCK:
@@ -467,9 +467,7 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
fallthrough;
case -ERESTARTNOINTR:
regs->gprs[2] = regs->orig_gpr2;
- regs->psw.addr =
- __rewind_psw(regs->psw,
- regs->int_code >> 16);
+ regs->psw.addr = __rewind_psw(regs->psw, SVC_LEN);
break;
}
}
@@ -484,7 +482,7 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
/* No handlers present - check for system call restart */
clear_pt_regs_flag(regs, PIF_SYSCALL);
if (current->thread.system_call) {
- regs->int_code = current->thread.system_call;
+ regs->syscall_nr = current->thread.system_call;
switch (regs->gprs[2]) {
case -ERESTART_RESTARTBLOCK:
/* Restart with sys_restart_syscall */
@@ -498,7 +496,7 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
case -ERESTARTSYS:
case -ERESTARTNOINTR:
regs->gprs[2] = regs->orig_gpr2;
- regs->psw.addr = __rewind_psw(regs->psw, regs->int_code >> 16);
+ regs->psw.addr = __rewind_psw(regs->psw, SVC_LEN);
if (test_thread_flag(TIF_SINGLE_STEP))
clear_thread_flag(TIF_PER_TRAP);
break;
diff --git a/arch/s390/kernel/syscall.c b/arch/s390/kernel/syscall.c
index ce244dceec6d..62e283ffef37 100644
--- a/arch/s390/kernel/syscall.c
+++ b/arch/s390/kernel/syscall.c
@@ -100,7 +100,10 @@ void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
enter_from_user_mode(regs);
add_random_kstack_offset();
regs->psw = get_lowcore()->svc_old_psw;
- regs->int_code = get_lowcore()->svc_int_code;
+ nr = get_lowcore()->svc_int_code & 0xffff;
+ if (likely(!nr))
+ nr = regs->gprs[1];
+ regs->syscall_nr = nr;
update_timer_sys();
if (cpu_has_bear())
current->thread.last_break = regs->last_break;
@@ -110,12 +113,6 @@ void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
set_thread_flag(TIF_PER_TRAP);
regs->flags = 0;
set_pt_regs_flag(regs, PIF_SYSCALL);
- nr = regs->int_code & 0xffff;
- if (likely(!nr)) {
- nr = regs->gprs[1] & 0xffff;
- regs->int_code &= ~0xffffUL;
- regs->int_code |= nr;
- }
regs->gprs[2] = nr;
regs->syscall_ret = -ENOSYS;
if (nr == __NR_restart_syscall && !(current->restart_block.arch_data & 1)) {
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] s390/syscall: Keep syscall number in extra ptregs member
2026-07-15 13:38 ` [PATCH 2/2] s390/syscall: Keep syscall number " Sven Schnelle
@ 2026-07-15 13:59 ` Heiko Carstens
2026-07-15 14:10 ` sashiko-bot
1 sibling, 0 replies; 6+ messages in thread
From: Heiko Carstens @ 2026-07-15 13:59 UTC (permalink / raw)
To: Sven Schnelle
Cc: Vasily Gorbik, Alexander Gordeev, H. Peter Anvin, Thomas Gleixner,
Christian Borntraeger, Michal Suchánek, Peter Zijlstra,
linux-s390, linux-kernel
On Wed, Jul 15, 2026 at 03:38:30PM +0200, Sven Schnelle wrote:
> Move the syscall number from ptregs::int_code to another union member
> called syscall_nr. This simplifies the code a bit.
>
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
> arch/s390/include/asm/ptrace.h | 2 +-
> arch/s390/include/asm/syscall.h | 8 +++++---
> arch/s390/kernel/ptrace.c | 2 +-
> arch/s390/kernel/signal.c | 12 +++++-------
> arch/s390/kernel/syscall.c | 11 ++++-------
> 5 files changed, 16 insertions(+), 19 deletions(-)
...
> diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
> index 125ca4c4e30c..00e825d01504 100644
> --- a/arch/s390/kernel/ptrace.c
> +++ b/arch/s390/kernel/ptrace.c
> @@ -349,7 +349,7 @@ static int __poke_user(struct task_struct *child, addr_t addr, addr_t data)
> addr == offsetof(struct user, regs.gprs[2])) {
> struct pt_regs *regs = task_pt_regs(child);
>
> - regs->int_code = 0x20000 | (data & 0xffff);
> + regs->syscall_nr = data;
This breaks peek_user(). So handling for that needs to be added too.
> - regs->psw.addr =
> - __rewind_psw(regs->psw,
> - regs->int_code >> 16);
> + regs->psw.addr = __rewind_psw(regs->psw, SVC_LEN);
Why? This is unrelated to what you want to achieve, and it is broken
if the svc instruction is executed via exrl. Plus, let's not use
hardcoded instruction lengths if the CPU provides you the correct
length anyway.
Besides that it _seems_ to be ok. :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] s390/syscall: Keep syscall return in extra ptregs member
2026-07-15 13:38 ` [PATCH 1/2] s390/syscall: Keep syscall return in extra ptregs member Sven Schnelle
@ 2026-07-15 14:04 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-15 14:04 UTC (permalink / raw)
To: Sven Schnelle
Cc: Vasily Gorbik, Alexander Gordeev, linux-s390, Heiko Carstens
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Tracer modifications to syscall return values during exit stops are completely ignored.
- [Low] The patch leaves behind an unused and broken macro `_PIF_SYSCALL_RET_SET`.
Pre-existing issues:
- [High] `syscall_rollback()` register restoration is unconditionally clobbered by `-ENOSYS` assignment.
--
--- Patch [1]: [PATCH 1/2] s390/syscall: Keep syscall return in extra ptregs member ---
commit 0354447a3b4a9b11afa78187a684a62c8037dee7
Author: Sven Schnelle <svens@linux.ibm.com>
s390/syscall: Keep syscall return in extra ptregs member
This patch introduces a new member syscall_ret in struct pt_regs to keep
the syscall return value separate from gprs[2]. It copies this value back
to gprs[2] before exit to userspace, resolving issues where setting both a
return value and skipping the syscall was impossible with seccomp or ptrace.
Link: https://lore.kernel.org/all/20260703105718.GO751831@noisy.programming.kicks-ass.net/
> diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
> index 495e310c3d6d7..f39b349da9a7d 100644
> --- a/arch/s390/include/asm/ptrace.h
> +++ b/arch/s390/include/asm/ptrace.h
> @@ -15,9 +15,8 @@
>
> #define PIF_SYSCALL 0 /* inside a system call */
> #define PIF_PSW_ADDR_ADJUSTED 1 /* psw address has been adjusted */
> -#define PIF_SYSCALL_RET_SET 2 /* return value was set via ptrace */
> -#define PIF_GUEST_FAULT 3 /* indicates program check in sie64a */
> -#define PIF_FTRACE_FULL_REGS 4 /* all register contents valid (ftrace) */
> +#define PIF_GUEST_FAULT 2 /* indicates program check in sie64a */
> +#define PIF_FTRACE_FULL_REGS 3 /* all register contents valid (ftrace) */
[Severity: Low]
Could this leave behind a broken macro? Since PIF_SYSCALL_RET_SET is deleted
here, it looks like _PIF_SYSCALL_RET_SET (which is defined right below this
using BIT(PIF_SYSCALL_RET_SET)) will cause a compilation failure if it is ever
used.
[ ... ]
> diff --git a/arch/s390/kernel/syscall.c b/arch/s390/kernel/syscall.c
> index 75d5a3cab14e6..ce244dceec6dc 100644
> --- a/arch/s390/kernel/syscall.c
> +++ b/arch/s390/kernel/syscall.c
> @@ -117,25 +117,16 @@ void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
[ ... ]
> if (likely(nr < NR_syscalls)) {
> nr = array_index_nospec(nr, NR_syscalls);
> - regs->gprs[2] = sys_call_table[nr](regs);
> + regs->syscall_ret = sys_call_table[nr](regs);
> }
> -out:
> + regs->gprs[2] = regs->syscall_ret;
[Severity: High]
Are tracer modifications to the syscall return value being ignored here?
The assignment regs->gprs[2] = regs->syscall_ret happens before we call
syscall_exit_to_user_mode().
If a tracer stops the process during exit hooks inside
syscall_exit_to_user_mode() and modifies the return value, it updates
regs->syscall_ret via syscall_set_return_value(), but this change is never
propagated to the actual CPU register gprs[2].
Will the process resume using the unmodified original return value, completely
breaking exit-time return value injection?
[Severity: High]
This is a pre-existing issue, but isn't the register restoration from
syscall_rollback() clobbered by this assignment?
If a seccomp filter uses SECCOMP_RET_TRAP to abort the syscall, it triggers a
rollback which restores regs->orig_gpr2 into regs->gprs[2]:
arch/s390/include/asm/syscall.h:syscall_rollback() {
...
regs->gprs[2] = regs->orig_gpr2;
...
}
However, this restored value is then immediately overwritten here by the
assignment regs->gprs[2] = regs->syscall_ret (which holds -ENOSYS for aborted
syscalls).
Does this break userspace seccomp signal handlers that expect the restored
state in gprs[2]?
> syscall_exit_to_user_mode(regs);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715133830.2619853-1-svens@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] s390/syscall: Keep syscall number in extra ptregs member
2026-07-15 13:38 ` [PATCH 2/2] s390/syscall: Keep syscall number " Sven Schnelle
2026-07-15 13:59 ` Heiko Carstens
@ 2026-07-15 14:10 ` sashiko-bot
1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-15 14:10 UTC (permalink / raw)
To: Sven Schnelle
Cc: Heiko Carstens, linux-s390, Alexander Gordeev, Vasily Gorbik
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Syscall restarts for system calls invoked via the `EXECUTE` instruction will rewind the instruction pointer by the wrong amount, causing execution corruption.
- [High] The patch breaks the ptrace `NT_S390_SYSTEM_CALL` regset ABI by omitting the instruction length data.
- [High] The syscall number loaded from `%r1` is no longer truncated to 16 bits, allowing arbitrary garbage in the upper bits to cause valid syscalls to fail.
--
--- Patch [2]: [PATCH 2/2] s390/syscall: Keep syscall number in extra ptregs member ---
commit 1c7e9df362bfdaa075e7114b5570a63dc6c4e073
Author: Sven Schnelle <svens@linux.ibm.com>
s390/syscall: Keep syscall number in extra ptregs member
Move the syscall number from ptregs::int_code to another union member
called syscall_nr. This simplifies the code a bit.
> diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
> index 4874de5edea0a..c3c69294ff330 100644
> --- a/arch/s390/kernel/signal.c
> +++ b/arch/s390/kernel/signal.c
> @@ -447,12 +447,12 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
> * call information.
> */
> current->thread.system_call =
> - test_pt_regs_flag(regs, PIF_SYSCALL) ? regs->int_code : 0;
> + test_pt_regs_flag(regs, PIF_SYSCALL) ? regs->syscall_nr : 0;
[Severity: High]
Does this break the ptrace NT_S390_SYSTEM_CALL regset ABI?
Previously, thread.system_call stored both the syscall number and the
instruction length code (ILC) in the upper 16 bits, which userspace tools
like CRIU depend on to correctly restart syscalls.
By saving only regs->syscall_nr, it appears the instruction length data is
dropped.
>
> if (get_signal(&ksig)) {
> /* Whee! Actually deliver the signal. */
> if (current->thread.system_call) {
> - regs->int_code = current->thread.system_call;
> + regs->syscall_nr = current->thread.system_call;
> /* Check for system call restarting. */
> switch (regs->gprs[2]) {
[ ... ]
> @@ -467,9 +467,7 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
> fallthrough;
> case -ERESTARTNOINTR:
> regs->gprs[2] = regs->orig_gpr2;
> - regs->psw.addr =
> - __rewind_psw(regs->psw,
> - regs->int_code >> 16);
> + regs->psw.addr = __rewind_psw(regs->psw, SVC_LEN);
[Severity: High]
Is it safe to rewind the PSW address by a hardcoded SVC_LEN (2 bytes) here?
If a user-space application invokes a restartable syscall using the EXECUTE
instruction, the dynamic instruction length is 4 or 6 bytes.
Because the hardware ILC is no longer used to determine the rewind length,
could resuming userspace result in execution corruption or a SIGILL by
resuming in the middle of the EXECUTE instruction?
> break;
> }
> }
[ ... ]
> @@ -498,7 +496,7 @@ void arch_do_signal_or_restart(struct pt_regs *regs)
> case -ERESTARTSYS:
> case -ERESTARTNOINTR:
> regs->gprs[2] = regs->orig_gpr2;
> - regs->psw.addr = __rewind_psw(regs->psw, regs->int_code >> 16);
> + regs->psw.addr = __rewind_psw(regs->psw, SVC_LEN);
[Severity: High]
This seems to have the same hardcoded SVC_LEN issue as above when restarting
syscalls on the alternative restart path. Could this also cause execution
corruption if the syscall was invoked via EXECUTE?
> if (test_thread_flag(TIF_SINGLE_STEP))
> clear_thread_flag(TIF_PER_TRAP);
> break;
[ ... ]
> diff --git a/arch/s390/kernel/syscall.c b/arch/s390/kernel/syscall.c
> index ce244dceec6dc..62e283ffef373 100644
> --- a/arch/s390/kernel/syscall.c
> +++ b/arch/s390/kernel/syscall.c
> @@ -100,7 +100,10 @@ void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
> enter_from_user_mode(regs);
> add_random_kstack_offset();
> regs->psw = get_lowcore()->svc_old_psw;
> - regs->int_code = get_lowcore()->svc_int_code;
> + nr = get_lowcore()->svc_int_code & 0xffff;
> + if (likely(!nr))
> + nr = regs->gprs[1];
> + regs->syscall_nr = nr;
[Severity: High]
Does this assignment need to truncate the value read from regs->gprs[1] to
16 bits?
If a 31-bit compat application, or a 64-bit application leaves garbage in
the upper 32 bits of register %r1 and issues a syscall with SVC number 0, nr
will include that garbage.
Could this cause the subsequent bounds check on nr in __do_syscall() to
fail, returning -ENOSYS for an otherwise valid syscall?
> update_timer_sys();
> if (cpu_has_bear())
> current->thread.last_break = regs->last_break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715133830.2619853-1-svens@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-15 14:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 13:38 [PATCH RFC 0/2] s390: keep syscall number / return value in separate pt_regs members Sven Schnelle
2026-07-15 13:38 ` [PATCH 1/2] s390/syscall: Keep syscall return in extra ptregs member Sven Schnelle
2026-07-15 14:04 ` sashiko-bot
2026-07-15 13:38 ` [PATCH 2/2] s390/syscall: Keep syscall number " Sven Schnelle
2026-07-15 13:59 ` Heiko Carstens
2026-07-15 14:10 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.