Re: [PATCH 1/2] s390/syscall: Keep syscall return in extra ptregs member

From: Sven Schnelle

Date: Tue Jul 21 2026 - 08:06:03 EST


Michal Suchánek <msuchanek@xxxxxxx> writes:

> On Wed, Jul 15, 2026 at 03:38:29PM +0200, Sven Schnelle wrote:
>> 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);
>
> Hello,
>
> I think this would break ptrace PTRACE_SETSIGINFO on syscall exit.
>
> It should use syscall_set_return_value but that now sets
> regs->syscall_ret, not regs->gprs[2]. At the same time the return value
> is in regs->gprs[2] at this point.

Thanks, Sashiko reported that already. I was thinking that syscall exit
tracing/filtering sets gpr[2] directly, but that's obviously wrong as it
uses syscall_set_return(). After trying to fix this I think I'll leave
the PIF_SYSCALL_RET_SET flag in place as it is - adding yet another
member to ptregs make it even more confusing.