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

[Michal Suchánek]

On Wed, Jul 15, 2026 at 03:38:29PM +0200, Sven Schnelle wrote:
> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
> 
> Suggested-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Signed-off-by: Sven Schnelle <svens@xxxxxxxxxxxxx>
> ---
>  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);

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

Michal

>  }
> -- 
> 2.53.0
>