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

[Sven Schnelle <svens@linux.ibm.com>]

Michal Suchánek <msuchanek@suse.de> 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.