* [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
* 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
* [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 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.