Re: [PATCH v6 3/6] syscall.h: introduce syscall_set_nr()

["Dmitry V. Levin" <ldv@strace.io>]

On Wed, Feb 19, 2025 at 05:16:05PM +0000, Maciej W. Rozycki wrote:
> On Mon, 17 Feb 2025, Dmitry V. Levin wrote:
> 
> > diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
> > index ea050b23d428..b956b015641c 100644
> > --- a/arch/mips/include/asm/syscall.h
> > +++ b/arch/mips/include/asm/syscall.h
> > @@ -41,6 +41,20 @@ static inline long syscall_get_nr(struct task_struct *task,
> >  	return task_thread_info(task)->syscall;
> >  }
> >  
> > +static inline void syscall_set_nr(struct task_struct *task,
> > +				  struct pt_regs *regs,
> > +				  int nr)
> > +{
> > +	/*
> > +	 * New syscall number has to be assigned to regs[2] because
> > +	 * syscall_trace_entry() loads it from there unconditionally.
> 
>  That label is called `trace_a_syscall' in arch/mips/kernel/scall64-o32.S 
> instead.  To bring some order and avoid an inaccuracy here should the odd 
> one be matched to the other three?

Apparently, there are two instances of syscall_trace_entry(), one
n32_syscall_trace_entry(), one trace_a_syscall(), and each of them
is calling syscall_trace_enter(), not to be confused with
syscall_trace_entry():

scall32-o32.S-syscall_trace_entry:
scall32-o32.S-	SAVE_STATIC
scall32-o32.S-	move	a0, sp
scall32-o32.S-
scall32-o32.S:	jal	syscall_trace_enter
scall32-o32.S-
scall32-o32.S-	bltz	v0, 1f			# seccomp failed? Skip syscall
scall32-o32.S-
scall32-o32.S-	RESTORE_STATIC
scall32-o32.S-	lw	v0, PT_R2(sp)		# Restore syscall (maybe modified)
--
scall64-n32.S-n32_syscall_trace_entry:
scall64-n32.S-	SAVE_STATIC
scall64-n32.S-	move	a0, sp
scall64-n32.S:	jal	syscall_trace_enter
scall64-n32.S-
scall64-n32.S-	bltz	v0, 1f			# seccomp failed? Skip syscall
scall64-n32.S-
scall64-n32.S-	RESTORE_STATIC
scall64-n32.S-	ld	v0, PT_R2(sp)		# Restore syscall (maybe modified)
--
scall64-n64.S-syscall_trace_entry:
scall64-n64.S-	SAVE_STATIC
scall64-n64.S-	move	a0, sp
scall64-n64.S:	jal	syscall_trace_enter
scall64-n64.S-
scall64-n64.S-	bltz	v0, 1f			# seccomp failed? Skip syscall
scall64-n64.S-
scall64-n64.S-	RESTORE_STATIC
scall64-n64.S-	ld	v0, PT_R2(sp)		# Restore syscall (maybe modified)
--
scall64-o32.S-trace_a_syscall:
scall64-o32.S-	SAVE_STATIC
scall64-o32.S-	sd	a4, PT_R8(sp)		# Save argument registers
scall64-o32.S-	sd	a5, PT_R9(sp)
scall64-o32.S-	sd	a6, PT_R10(sp)
scall64-o32.S-	sd	a7, PT_R11(sp)		# For indirect syscalls
scall64-o32.S-
scall64-o32.S-	move	a0, sp
scall64-o32.S:	jal	syscall_trace_enter
scall64-o32.S-
scall64-o32.S-	bltz	v0, 1f			# seccomp failed? Skip syscall
scall64-o32.S-
scall64-o32.S-	RESTORE_STATIC
scall64-o32.S-	ld	v0, PT_R2(sp)		# Restore syscall (maybe modified)

I'd change the wording of my comment rather than try to disentangle this.
After all, the most important here is that the new syscall number is
loaded from regs[2] right after the syscall_trace_enter() invocation.

Would you be OK with the following wording:
	/*
	 * New syscall number has to be assigned to regs[2] because it is
	 * loaded from there unconditionally after syscall_trace_enter()
	 * invocation.
	 *
	 * Consequently, if the syscall was indirect and nr != __NR_syscall,
	 * then after this assignment the syscall will cease to be indirect.
	 */
?


-- 
ldv