[v7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API

[PATCH v7 2/6] syscall.h: add syscall_set_arguments()

Posted by Dmitry V. Levin 9 months, 2 weeks ago

This function is going to be needed on all HAVE_ARCH_TRACEHOOK
architectures to implement PTRACE_SET_SYSCALL_INFO API.

This partially reverts commit 7962c2eddbfe ("arch: remove unused
function syscall_set_arguments()") by reusing some of old
syscall_set_arguments() implementations.

Signed-off-by: Dmitry V. Levin <ldv@strace.io>
Tested-by: Charlie Jenkins <charlie@rivosinc.com>
Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
Acked-by: Helge Deller <deller@gmx.de> # parisc
Reviewed-by: Maciej W. Rozycki <macro@orcam.me.uk> # mips
---
 arch/arc/include/asm/syscall.h        | 14 +++++++++++
 arch/arm/include/asm/syscall.h        | 13 ++++++++++
 arch/arm64/include/asm/syscall.h      | 13 ++++++++++
 arch/csky/include/asm/syscall.h       | 13 ++++++++++
 arch/hexagon/include/asm/syscall.h    |  7 ++++++
 arch/loongarch/include/asm/syscall.h  |  8 ++++++
 arch/mips/include/asm/syscall.h       | 28 +++++++++++++++++++++
 arch/nios2/include/asm/syscall.h      | 11 ++++++++
 arch/openrisc/include/asm/syscall.h   |  7 ++++++
 arch/parisc/include/asm/syscall.h     | 12 +++++++++
 arch/powerpc/include/asm/syscall.h    | 10 ++++++++
 arch/riscv/include/asm/syscall.h      |  9 +++++++
 arch/s390/include/asm/syscall.h       |  9 +++++++
 arch/sh/include/asm/syscall_32.h      | 12 +++++++++
 arch/sparc/include/asm/syscall.h      | 10 ++++++++
 arch/um/include/asm/syscall-generic.h | 14 +++++++++++
 arch/x86/include/asm/syscall.h        | 36 +++++++++++++++++++++++++++
 arch/xtensa/include/asm/syscall.h     | 11 ++++++++
 include/asm-generic/syscall.h         | 16 ++++++++++++
 19 files changed, 253 insertions(+)

diff --git a/arch/arc/include/asm/syscall.h b/arch/arc/include/asm/syscall.h
index 9709256e31c8..89c1e1736356 100644
--- a/arch/arc/include/asm/syscall.h
+++ b/arch/arc/include/asm/syscall.h
@@ -67,6 +67,20 @@ syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
 	}
 }
 
+static inline void
+syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
+		      unsigned long *args)
+{
+	unsigned long *inside_ptregs = &regs->r0;
+	unsigned int n = 6;
+	unsigned int i = 0;
+
+	while (n--) {
+		*inside_ptregs = args[i++];
+		inside_ptregs--;
+	}
+}
+
 static inline int
 syscall_get_arch(struct task_struct *task)
 {
diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
index fe4326d938c1..21927fa0ae2b 100644
--- a/arch/arm/include/asm/syscall.h
+++ b/arch/arm/include/asm/syscall.h
@@ -80,6 +80,19 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	memcpy(args, &regs->ARM_r0 + 1, 5 * sizeof(args[0]));
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	memcpy(&regs->ARM_r0, args, 6 * sizeof(args[0]));
+	/*
+	 * Also copy the first argument into ARM_ORIG_r0
+	 * so that syscall_get_arguments() would return it
+	 * instead of the previous value.
+	 */
+	regs->ARM_ORIG_r0 = regs->ARM_r0;
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 	/* ARM tasks don't change audit architectures on the fly. */
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index ab8e14b96f68..76020b66286b 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -73,6 +73,19 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	memcpy(args, &regs->regs[1], 5 * sizeof(args[0]));
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	memcpy(&regs->regs[0], args, 6 * sizeof(args[0]));
+	/*
+	 * Also copy the first argument into orig_x0
+	 * so that syscall_get_arguments() would return it
+	 * instead of the previous value.
+	 */
+	regs->orig_x0 = regs->regs[0];
+}
+
 /*
  * We don't care about endianness (__AUDIT_ARCH_LE bit) here because
  * AArch64 has the same system calls both on little- and big- endian.
diff --git a/arch/csky/include/asm/syscall.h b/arch/csky/include/asm/syscall.h
index 0de5734950bf..717f44b4d26f 100644
--- a/arch/csky/include/asm/syscall.h
+++ b/arch/csky/include/asm/syscall.h
@@ -59,6 +59,19 @@ syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
 	memcpy(args, &regs->a1, 5 * sizeof(args[0]));
 }
 
+static inline void
+syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
+		      const unsigned long *args)
+{
+	memcpy(&regs->a0, args, 6 * sizeof(regs->a0));
+	/*
+	 * Also copy the first argument into orig_a0
+	 * so that syscall_get_arguments() would return it
+	 * instead of the previous value.
+	 */
+	regs->orig_a0 = regs->a0;
+}
+
 static inline int
 syscall_get_arch(struct task_struct *task)
 {
diff --git a/arch/hexagon/include/asm/syscall.h b/arch/hexagon/include/asm/syscall.h
index 951ca0ed8376..1024a6548d78 100644
--- a/arch/hexagon/include/asm/syscall.h
+++ b/arch/hexagon/include/asm/syscall.h
@@ -33,6 +33,13 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	memcpy(args, &(&regs->r00)[0], 6 * sizeof(args[0]));
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 unsigned long *args)
+{
+	memcpy(&(&regs->r00)[0], args, 6 * sizeof(args[0]));
+}
+
 static inline long syscall_get_error(struct task_struct *task,
 				     struct pt_regs *regs)
 {
diff --git a/arch/loongarch/include/asm/syscall.h b/arch/loongarch/include/asm/syscall.h
index e286dc58476e..ff415b3c0a8e 100644
--- a/arch/loongarch/include/asm/syscall.h
+++ b/arch/loongarch/include/asm/syscall.h
@@ -61,6 +61,14 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	memcpy(&args[1], &regs->regs[5], 5 * sizeof(long));
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 unsigned long *args)
+{
+	regs->orig_a0 = args[0];
+	memcpy(&regs->regs[5], &args[1], 5 * sizeof(long));
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 	return AUDIT_ARCH_LOONGARCH64;
diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
index 056aa1b713e2..f1926ce30d4b 100644
--- a/arch/mips/include/asm/syscall.h
+++ b/arch/mips/include/asm/syscall.h
@@ -74,6 +74,23 @@ static inline void mips_get_syscall_arg(unsigned long *arg,
 #endif
 }
 
+static inline void mips_set_syscall_arg(unsigned long *arg,
+	struct task_struct *task, struct pt_regs *regs, unsigned int n)
+{
+#ifdef CONFIG_32BIT
+	switch (n) {
+	case 0: case 1: case 2: case 3:
+		regs->regs[4 + n] = *arg;
+		return;
+	case 4: case 5: case 6: case 7:
+		*arg = regs->args[n] = *arg;
+		return;
+	}
+#else
+	regs->regs[4 + n] = *arg;
+#endif
+}
+
 static inline long syscall_get_error(struct task_struct *task,
 				     struct pt_regs *regs)
 {
@@ -120,6 +137,17 @@ static inline void syscall_get_arguments(struct task_struct *task,
 		mips_get_syscall_arg(args++, task, regs, i++);
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 unsigned long *args)
+{
+	unsigned int i = 0;
+	unsigned int n = 6;
+
+	while (n--)
+		mips_set_syscall_arg(args++, task, regs, i++);
+}
+
 extern const unsigned long sys_call_table[];
 extern const unsigned long sys32_call_table[];
 extern const unsigned long sysn32_call_table[];
diff --git a/arch/nios2/include/asm/syscall.h b/arch/nios2/include/asm/syscall.h
index fff52205fb65..526449edd768 100644
--- a/arch/nios2/include/asm/syscall.h
+++ b/arch/nios2/include/asm/syscall.h
@@ -58,6 +58,17 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	*args   = regs->r9;
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+	struct pt_regs *regs, const unsigned long *args)
+{
+	regs->r4 = *args++;
+	regs->r5 = *args++;
+	regs->r6 = *args++;
+	regs->r7 = *args++;
+	regs->r8 = *args++;
+	regs->r9 = *args;
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 	return AUDIT_ARCH_NIOS2;
diff --git a/arch/openrisc/include/asm/syscall.h b/arch/openrisc/include/asm/syscall.h
index 903ed882bdec..e6383be2a195 100644
--- a/arch/openrisc/include/asm/syscall.h
+++ b/arch/openrisc/include/asm/syscall.h
@@ -57,6 +57,13 @@ syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
 	memcpy(args, &regs->gpr[3], 6 * sizeof(args[0]));
 }
 
+static inline void
+syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
+		      const unsigned long *args)
+{
+	memcpy(&regs->gpr[3], args, 6 * sizeof(args[0]));
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 	return AUDIT_ARCH_OPENRISC;
diff --git a/arch/parisc/include/asm/syscall.h b/arch/parisc/include/asm/syscall.h
index 00b127a5e09b..b146d0ae4c77 100644
--- a/arch/parisc/include/asm/syscall.h
+++ b/arch/parisc/include/asm/syscall.h
@@ -29,6 +29,18 @@ static inline void syscall_get_arguments(struct task_struct *tsk,
 	args[0] = regs->gr[26];
 }
 
+static inline void syscall_set_arguments(struct task_struct *tsk,
+					 struct pt_regs *regs,
+					 unsigned long *args)
+{
+	regs->gr[21] = args[5];
+	regs->gr[22] = args[4];
+	regs->gr[23] = args[3];
+	regs->gr[24] = args[2];
+	regs->gr[25] = args[1];
+	regs->gr[26] = args[0];
+}
+
 static inline long syscall_get_error(struct task_struct *task,
 				     struct pt_regs *regs)
 {
diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 422d7735ace6..521f279e6b33 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -114,6 +114,16 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	}
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	memcpy(&regs->gpr[3], args, 6 * sizeof(args[0]));
+
+	/* Also copy the first argument into orig_gpr3 */
+	regs->orig_gpr3 = args[0];
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 	if (is_tsk_32bit_task(task))
diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
index 121fff429dce..8d389ba995c8 100644
--- a/arch/riscv/include/asm/syscall.h
+++ b/arch/riscv/include/asm/syscall.h
@@ -66,6 +66,15 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	memcpy(args, &regs->a1, 5 * sizeof(args[0]));
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	regs->orig_a0 = args[0];
+	args++;
+	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 #ifdef CONFIG_64BIT
diff --git a/arch/s390/include/asm/syscall.h b/arch/s390/include/asm/syscall.h
index 27e3d804b311..0e3296a32e6a 100644
--- a/arch/s390/include/asm/syscall.h
+++ b/arch/s390/include/asm/syscall.h
@@ -78,6 +78,15 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	args[0] = regs->orig_gpr2 & mask;
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	regs->orig_gpr2 = args[0];
+	for (int n = 1; n < 6; n++)
+		regs->gprs[2 + n] = args[n];
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 #ifdef CONFIG_COMPAT
diff --git a/arch/sh/include/asm/syscall_32.h b/arch/sh/include/asm/syscall_32.h
index d87738eebe30..cb51a7528384 100644
--- a/arch/sh/include/asm/syscall_32.h
+++ b/arch/sh/include/asm/syscall_32.h
@@ -57,6 +57,18 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	args[0] = regs->regs[4];
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	regs->regs[1] = args[5];
+	regs->regs[0] = args[4];
+	regs->regs[7] = args[3];
+	regs->regs[6] = args[2];
+	regs->regs[5] = args[1];
+	regs->regs[4] = args[0];
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 	int arch = AUDIT_ARCH_SH;
diff --git a/arch/sparc/include/asm/syscall.h b/arch/sparc/include/asm/syscall.h
index 20c109ac8cc9..62a5a78804c4 100644
--- a/arch/sparc/include/asm/syscall.h
+++ b/arch/sparc/include/asm/syscall.h
@@ -117,6 +117,16 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	}
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	unsigned int i;
+
+	for (i = 0; i < 6; i++)
+		regs->u_regs[UREG_I0 + i] = args[i];
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 #if defined(CONFIG_SPARC64) && defined(CONFIG_COMPAT)
diff --git a/arch/um/include/asm/syscall-generic.h b/arch/um/include/asm/syscall-generic.h
index 172b74143c4b..2984feb9d576 100644
--- a/arch/um/include/asm/syscall-generic.h
+++ b/arch/um/include/asm/syscall-generic.h
@@ -62,6 +62,20 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	*args   = UPT_SYSCALL_ARG6(r);
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	struct uml_pt_regs *r = &regs->regs;
+
+	UPT_SYSCALL_ARG1(r) = *args++;
+	UPT_SYSCALL_ARG2(r) = *args++;
+	UPT_SYSCALL_ARG3(r) = *args++;
+	UPT_SYSCALL_ARG4(r) = *args++;
+	UPT_SYSCALL_ARG5(r) = *args++;
+	UPT_SYSCALL_ARG6(r) = *args;
+}
+
 /* See arch/x86/um/asm/syscall.h for syscall_get_arch() definition. */
 
 #endif	/* __UM_SYSCALL_GENERIC_H */
diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h
index 7c488ff0c764..b9c249dd9e3d 100644
--- a/arch/x86/include/asm/syscall.h
+++ b/arch/x86/include/asm/syscall.h
@@ -90,6 +90,18 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	args[5] = regs->bp;
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	regs->bx = args[0];
+	regs->cx = args[1];
+	regs->dx = args[2];
+	regs->si = args[3];
+	regs->di = args[4];
+	regs->bp = args[5];
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 	return AUDIT_ARCH_I386;
@@ -121,6 +133,30 @@ static inline void syscall_get_arguments(struct task_struct *task,
 	}
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+# ifdef CONFIG_IA32_EMULATION
+	if (task->thread_info.status & TS_COMPAT) {
+		regs->bx = *args++;
+		regs->cx = *args++;
+		regs->dx = *args++;
+		regs->si = *args++;
+		regs->di = *args++;
+		regs->bp = *args;
+	} else
+# endif
+	{
+		regs->di = *args++;
+		regs->si = *args++;
+		regs->dx = *args++;
+		regs->r10 = *args++;
+		regs->r8 = *args++;
+		regs->r9 = *args;
+	}
+}
+
 static inline int syscall_get_arch(struct task_struct *task)
 {
 	/* x32 tasks should be considered AUDIT_ARCH_X86_64. */
diff --git a/arch/xtensa/include/asm/syscall.h b/arch/xtensa/include/asm/syscall.h
index 5ee974bf8330..f9a671cbf933 100644
--- a/arch/xtensa/include/asm/syscall.h
+++ b/arch/xtensa/include/asm/syscall.h
@@ -68,6 +68,17 @@ static inline void syscall_get_arguments(struct task_struct *task,
 		args[i] = regs->areg[reg[i]];
 }
 
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 const unsigned long *args)
+{
+	static const unsigned int reg[] = XTENSA_SYSCALL_ARGUMENT_REGS;
+	unsigned int i;
+
+	for (i = 0; i < 6; ++i)
+		regs->areg[reg[i]] = args[i];
+}
+
 asmlinkage long xtensa_rt_sigreturn(void);
 asmlinkage long xtensa_shmat(int, char __user *, int);
 asmlinkage long xtensa_fadvise64_64(int, int,
diff --git a/include/asm-generic/syscall.h b/include/asm-generic/syscall.h
index 182b039ce5fa..292b412f4e9a 100644
--- a/include/asm-generic/syscall.h
+++ b/include/asm-generic/syscall.h
@@ -117,6 +117,22 @@ void syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
 void syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
 			   unsigned long *args);
 
+/**
+ * syscall_set_arguments - change system call parameter value
+ * @task:	task of interest, must be in system call entry tracing
+ * @regs:	task_pt_regs() of @task
+ * @args:	array of argument values to store
+ *
+ * Changes 6 arguments to the system call.
+ * The first argument gets value @args[0], and so on.
+ *
+ * It's only valid to call this when @task is stopped for tracing on
+ * entry to a system call, due to %SYSCALL_WORK_SYSCALL_TRACE or
+ * %SYSCALL_WORK_SYSCALL_AUDIT.
+ */
+void syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
+			   const unsigned long *args);
+
 /**
  * syscall_get_arch - return the AUDIT_ARCH for the current system call
  * @task:	task of interest, must be blocked
-- 
ldv

Re: [PATCH v7 2/6] syscall.h: add syscall_set_arguments()

Posted by Nathan Chancellor 8 months, 1 week ago

Hi Dmitry,

[dropping majority of folks since this seems irrelevant to them]

On Mon, Mar 03, 2025 at 01:20:09PM +0200, Dmitry V. Levin wrote:
> This function is going to be needed on all HAVE_ARCH_TRACEHOOK
> architectures to implement PTRACE_SET_SYSCALL_INFO API.
> 
> This partially reverts commit 7962c2eddbfe ("arch: remove unused
> function syscall_set_arguments()") by reusing some of old
> syscall_set_arguments() implementations.
> 
> Signed-off-by: Dmitry V. Levin <ldv@strace.io>
> Tested-by: Charlie Jenkins <charlie@rivosinc.com>
> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
> Acked-by: Helge Deller <deller@gmx.de> # parisc
> Reviewed-by: Maciej W. Rozycki <macro@orcam.me.uk> # mips
...
> diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> index 121fff429dce..8d389ba995c8 100644
> --- a/arch/riscv/include/asm/syscall.h
> +++ b/arch/riscv/include/asm/syscall.h
> @@ -66,6 +66,15 @@ static inline void syscall_get_arguments(struct task_struct *task,
>  	memcpy(args, &regs->a1, 5 * sizeof(args[0]));
>  }
>  
> +static inline void syscall_set_arguments(struct task_struct *task,
> +					 struct pt_regs *regs,
> +					 const unsigned long *args)
> +{
> +	regs->orig_a0 = args[0];
> +	args++;
> +	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> +}

This upsets the compiletime fortify checks, as I see a warning after
syscall_set_arguments() starts being used in kernel/ptrace.c later in
the series.

  $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- allmodconfig kernel/ptrace.o
  In file included from include/linux/string.h:392,
                   from include/linux/bitmap.h:13,
                   from include/linux/cpumask.h:12,
                   from arch/riscv/include/asm/processor.h:55,
                   from include/linux/sched.h:13,
                   from kernel/ptrace.c:13:
  In function 'fortify_memcpy_chk',
      inlined from 'syscall_set_arguments.isra' at arch/riscv/include/asm/syscall.h:82:2:
  include/linux/fortify-string.h:571:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
    571 |                         __write_overflow_field(p_size_field, size);
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors

The compiler knows the size of the destination and the size to be copied
so it knows there will be an (intentional) overwrite here.
struct_group() would normally work but I think this structure already
has a struct_group() around some of the members that would be needed. I
build tested eliminating the memcpy() altogether, which would appear to
work, but I am not sure if there is a better solution, hence just the
report.

Cheers,
Nathan

diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
index a5281cdf2b10..70ec19dc8506 100644
--- a/arch/riscv/include/asm/syscall.h
+++ b/arch/riscv/include/asm/syscall.h
@@ -78,8 +78,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
                                         const unsigned long *args)
 {
        regs->orig_a0 = args[0];
-       args++;
-       memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
+       regs->a1 = args[1];
+       regs->a2 = args[2];
+       regs->a3 = args[3];
+       regs->a4 = args[4];
+       regs->a5 = args[5];
 }

 static inline int syscall_get_arch(struct task_struct *task)

Re: [PATCH v7 2/6] syscall.h: add syscall_set_arguments()

Posted by Nathan Chancellor 8 months, 1 week ago

Hi Andrew,

Can you please fold the diff at the end of my message into
syscallh-add-syscall_set_arguments.patch? It sounds like Palmer will
apply the same diff to syscall_get_arguments():
https://lore.kernel.org/mhng-cf999eb1-59c4-4784-8c01-44b1e2482a50@palmer-ri-x1c9a/

Thanks!
Nathan

On Tue, Apr 08, 2025 at 02:31:31PM -0700, Nathan Chancellor wrote:
> Hi Dmitry,
> 
> [dropping majority of folks since this seems irrelevant to them]
> 
> On Mon, Mar 03, 2025 at 01:20:09PM +0200, Dmitry V. Levin wrote:
> > This function is going to be needed on all HAVE_ARCH_TRACEHOOK
> > architectures to implement PTRACE_SET_SYSCALL_INFO API.
> > 
> > This partially reverts commit 7962c2eddbfe ("arch: remove unused
> > function syscall_set_arguments()") by reusing some of old
> > syscall_set_arguments() implementations.
> > 
> > Signed-off-by: Dmitry V. Levin <ldv@strace.io>
> > Tested-by: Charlie Jenkins <charlie@rivosinc.com>
> > Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
> > Acked-by: Helge Deller <deller@gmx.de> # parisc
> > Reviewed-by: Maciej W. Rozycki <macro@orcam.me.uk> # mips
> ...
> > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> > index 121fff429dce..8d389ba995c8 100644
> > --- a/arch/riscv/include/asm/syscall.h
> > +++ b/arch/riscv/include/asm/syscall.h
> > @@ -66,6 +66,15 @@ static inline void syscall_get_arguments(struct task_struct *task,
> >  	memcpy(args, &regs->a1, 5 * sizeof(args[0]));
> >  }
> >  
> > +static inline void syscall_set_arguments(struct task_struct *task,
> > +					 struct pt_regs *regs,
> > +					 const unsigned long *args)
> > +{
> > +	regs->orig_a0 = args[0];
> > +	args++;
> > +	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> > +}
> 
> This upsets the compiletime fortify checks, as I see a warning after
> syscall_set_arguments() starts being used in kernel/ptrace.c later in
> the series.
> 
>   $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- allmodconfig kernel/ptrace.o
>   In file included from include/linux/string.h:392,
>                    from include/linux/bitmap.h:13,
>                    from include/linux/cpumask.h:12,
>                    from arch/riscv/include/asm/processor.h:55,
>                    from include/linux/sched.h:13,
>                    from kernel/ptrace.c:13:
>   In function 'fortify_memcpy_chk',
>       inlined from 'syscall_set_arguments.isra' at arch/riscv/include/asm/syscall.h:82:2:
>   include/linux/fortify-string.h:571:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>     571 |                         __write_overflow_field(p_size_field, size);
>         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   cc1: all warnings being treated as errors
> 
> The compiler knows the size of the destination and the size to be copied
> so it knows there will be an (intentional) overwrite here.
> struct_group() would normally work but I think this structure already
> has a struct_group() around some of the members that would be needed. I
> build tested eliminating the memcpy() altogether, which would appear to
> work, but I am not sure if there is a better solution, hence just the
> report.
> 
> Cheers,
> Nathan
> 
> diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> index a5281cdf2b10..70ec19dc8506 100644
> --- a/arch/riscv/include/asm/syscall.h
> +++ b/arch/riscv/include/asm/syscall.h
> @@ -78,8 +78,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
>                                          const unsigned long *args)
>  {
>         regs->orig_a0 = args[0];
> -       args++;
> -       memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> +       regs->a1 = args[1];
> +       regs->a2 = args[2];
> +       regs->a3 = args[3];
> +       regs->a4 = args[4];
> +       regs->a5 = args[5];
>  }
> 
>  static inline int syscall_get_arch(struct task_struct *task)

Re: [PATCH v7 2/6] syscall.h: add syscall_set_arguments()

Posted by Dmitry V. Levin 8 months, 1 week ago

Hi Nathan,

On Tue, Apr 08, 2025 at 02:31:31PM -0700, Nathan Chancellor wrote:
> Hi Dmitry,
> 
> [dropping majority of folks since this seems irrelevant to them]
> 
> On Mon, Mar 03, 2025 at 01:20:09PM +0200, Dmitry V. Levin wrote:
> > This function is going to be needed on all HAVE_ARCH_TRACEHOOK
> > architectures to implement PTRACE_SET_SYSCALL_INFO API.
> > 
> > This partially reverts commit 7962c2eddbfe ("arch: remove unused
> > function syscall_set_arguments()") by reusing some of old
> > syscall_set_arguments() implementations.
> > 
> > Signed-off-by: Dmitry V. Levin <ldv@strace.io>
> > Tested-by: Charlie Jenkins <charlie@rivosinc.com>
> > Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
> > Acked-by: Helge Deller <deller@gmx.de> # parisc
> > Reviewed-by: Maciej W. Rozycki <macro@orcam.me.uk> # mips
> ...
> > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> > index 121fff429dce..8d389ba995c8 100644
> > --- a/arch/riscv/include/asm/syscall.h
> > +++ b/arch/riscv/include/asm/syscall.h
> > @@ -66,6 +66,15 @@ static inline void syscall_get_arguments(struct task_struct *task,
> >  	memcpy(args, &regs->a1, 5 * sizeof(args[0]));
> >  }
> >  
> > +static inline void syscall_set_arguments(struct task_struct *task,
> > +					 struct pt_regs *regs,
> > +					 const unsigned long *args)
> > +{
> > +	regs->orig_a0 = args[0];
> > +	args++;
> > +	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> > +}
> 
> This upsets the compiletime fortify checks, as I see a warning after
> syscall_set_arguments() starts being used in kernel/ptrace.c later in
> the series.
> 
>   $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- allmodconfig kernel/ptrace.o
>   In file included from include/linux/string.h:392,
>                    from include/linux/bitmap.h:13,
>                    from include/linux/cpumask.h:12,
>                    from arch/riscv/include/asm/processor.h:55,
>                    from include/linux/sched.h:13,
>                    from kernel/ptrace.c:13:
>   In function 'fortify_memcpy_chk',
>       inlined from 'syscall_set_arguments.isra' at arch/riscv/include/asm/syscall.h:82:2:
>   include/linux/fortify-string.h:571:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>     571 |                         __write_overflow_field(p_size_field, size);
>         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   cc1: all warnings being treated as errors

I certainly tested the series on riscv64, but somehow I haven't seen this
compiler diagnostics before.

> The compiler knows the size of the destination and the size to be copied
> so it knows there will be an (intentional) overwrite here.
> struct_group() would normally work but I think this structure already
> has a struct_group() around some of the members that would be needed. I
> build tested eliminating the memcpy() altogether, which would appear to
> work, but I am not sure if there is a better solution, hence just the
> report.
> 
> Cheers,
> Nathan
> 
> diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> index a5281cdf2b10..70ec19dc8506 100644
> --- a/arch/riscv/include/asm/syscall.h
> +++ b/arch/riscv/include/asm/syscall.h
> @@ -78,8 +78,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
>                                          const unsigned long *args)
>  {
>         regs->orig_a0 = args[0];
> -       args++;
> -       memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> +       regs->a1 = args[1];
> +       regs->a2 = args[2];
> +       regs->a3 = args[3];
> +       regs->a4 = args[4];
> +       regs->a5 = args[5];
>  }

I don't mind eliminating the memcpy() altogether, but
I'd like to note that syscall_set_arguments() is an exact mirror
of syscall_get_arguments(), so if the intentional overwrite in
syscall_set_arguments() is not acceptable, then the intentional
overread in syscall_get_arguments() shouldn't be acceptable either.


-- 
ldv

Re: [PATCH v7 2/6] syscall.h: add syscall_set_arguments()

Posted by Nathan Chancellor 8 months, 1 week ago

On Wed, Apr 09, 2025 at 01:36:11AM +0300, Dmitry V. Levin wrote:
> On Tue, Apr 08, 2025 at 02:31:31PM -0700, Nathan Chancellor wrote:
> > On Mon, Mar 03, 2025 at 01:20:09PM +0200, Dmitry V. Levin wrote:
> > > +static inline void syscall_set_arguments(struct task_struct *task,
> > > +					 struct pt_regs *regs,
> > > +					 const unsigned long *args)
> > > +{
> > > +	regs->orig_a0 = args[0];
> > > +	args++;
> > > +	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> > > +}
> > 
> > This upsets the compiletime fortify checks, as I see a warning after
> > syscall_set_arguments() starts being used in kernel/ptrace.c later in
> > the series.
> > 
> >   $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- allmodconfig kernel/ptrace.o
> >   In file included from include/linux/string.h:392,
> >                    from include/linux/bitmap.h:13,
> >                    from include/linux/cpumask.h:12,
> >                    from arch/riscv/include/asm/processor.h:55,
> >                    from include/linux/sched.h:13,
> >                    from kernel/ptrace.c:13:
> >   In function 'fortify_memcpy_chk',
> >       inlined from 'syscall_set_arguments.isra' at arch/riscv/include/asm/syscall.h:82:2:
> >   include/linux/fortify-string.h:571:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
> >     571 |                         __write_overflow_field(p_size_field, size);
> >         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   cc1: all warnings being treated as errors
> 
> I certainly tested the series on riscv64, but somehow I haven't seen this
> compiler diagnostics before.

Maybe CONFIG_FORTIFY_SOURCE was not enabled? This comes from the
kernel's fortified memcpy checking function, fortify_memcpy_chk(), not
necessarily the compiler itself.

> > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> > index a5281cdf2b10..70ec19dc8506 100644
> > --- a/arch/riscv/include/asm/syscall.h
> > +++ b/arch/riscv/include/asm/syscall.h
> > @@ -78,8 +78,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
> >                                          const unsigned long *args)
> >  {
> >         regs->orig_a0 = args[0];
> > -       args++;
> > -       memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> > +       regs->a1 = args[1];
> > +       regs->a2 = args[2];
> > +       regs->a3 = args[3];
> > +       regs->a4 = args[4];
> > +       regs->a5 = args[5];
> >  }
> 
> I don't mind eliminating the memcpy() altogether, but
> I'd like to note that syscall_set_arguments() is an exact mirror
> of syscall_get_arguments(), so if the intentional overwrite in
> syscall_set_arguments() is not acceptable, then the intentional
> overread in syscall_get_arguments() shouldn't be acceptable either.

Yes, I noticed the symmetry too but I was only looking at it from the
overwrite perspective, not the overread one. That reminded me to double
check what fortify_memcpy_chk() actually checks for and I remembered
that the overread version of this warning is hidden under W=1 (I guess
because it happens more frequently).

  $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- W=1 allmodconfig kernel/ptrace.o
  In file included from include/linux/string.h:392,
                   from include/linux/bitmap.h:13,
                   from include/linux/cpumask.h:12,
                   from arch/riscv/include/asm/processor.h:55,
                   from include/linux/sched.h:13,
                   from kernel/ptrace.c:13:
  In function 'fortify_memcpy_chk',
      inlined from 'syscall_get_arguments.isra' at arch/riscv/include/asm/syscall.h:73:2:
  include/linux/fortify-string.h:580:25: error: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
    580 |                         __read_overflow2_field(q_size_field, size);
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors

So memcpy() should indeed be eliminated from both, which obviously
clears up the warnings.

Cheers,
Nathan

diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
index a5281cdf2b10..34313387f977 100644
--- a/arch/riscv/include/asm/syscall.h
+++ b/arch/riscv/include/asm/syscall.h
@@ -69,8 +69,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
 					 unsigned long *args)
 {
 	args[0] = regs->orig_a0;
-	args++;
-	memcpy(args, &regs->a1, 5 * sizeof(args[0]));
+	args[1] = regs->a1;
+	args[2] = regs->a2;
+	args[3] = regs->a3;
+	args[4] = regs->a4;
+	args[5] = regs->a5;
 }
 
 static inline void syscall_set_arguments(struct task_struct *task,
@@ -78,8 +81,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
 					 const unsigned long *args)
 {
 	regs->orig_a0 = args[0];
-	args++;
-	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
+	regs->a1 = args[1];
+	regs->a2 = args[2];
+	regs->a3 = args[3];
+	regs->a4 = args[4];
+	regs->a5 = args[5];
 }
 
 static inline int syscall_get_arch(struct task_struct *task)

Re: [PATCH v7 2/6] syscall.h: add syscall_set_arguments()

Posted by Dmitry V. Levin 8 months, 1 week ago

On Tue, Apr 08, 2025 at 05:38:03PM -0700, Nathan Chancellor wrote:
> On Wed, Apr 09, 2025 at 01:36:11AM +0300, Dmitry V. Levin wrote:
> > On Tue, Apr 08, 2025 at 02:31:31PM -0700, Nathan Chancellor wrote:
> > > On Mon, Mar 03, 2025 at 01:20:09PM +0200, Dmitry V. Levin wrote:
> > > > +static inline void syscall_set_arguments(struct task_struct *task,
> > > > +					 struct pt_regs *regs,
> > > > +					 const unsigned long *args)
> > > > +{
> > > > +	regs->orig_a0 = args[0];
> > > > +	args++;
> > > > +	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> > > > +}
> > > 
> > > This upsets the compiletime fortify checks, as I see a warning after
> > > syscall_set_arguments() starts being used in kernel/ptrace.c later in
> > > the series.
> > > 
> > >   $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- allmodconfig kernel/ptrace.o
> > >   In file included from include/linux/string.h:392,
> > >                    from include/linux/bitmap.h:13,
> > >                    from include/linux/cpumask.h:12,
> > >                    from arch/riscv/include/asm/processor.h:55,
> > >                    from include/linux/sched.h:13,
> > >                    from kernel/ptrace.c:13:
> > >   In function 'fortify_memcpy_chk',
> > >       inlined from 'syscall_set_arguments.isra' at arch/riscv/include/asm/syscall.h:82:2:
> > >   include/linux/fortify-string.h:571:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
> > >     571 |                         __write_overflow_field(p_size_field, size);
> > >         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >   cc1: all warnings being treated as errors
> > 
> > I certainly tested the series on riscv64, but somehow I haven't seen this
> > compiler diagnostics before.
> 
> Maybe CONFIG_FORTIFY_SOURCE was not enabled? This comes from the
> kernel's fortified memcpy checking function, fortify_memcpy_chk(), not
> necessarily the compiler itself.
> 
> > > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> > > index a5281cdf2b10..70ec19dc8506 100644
> > > --- a/arch/riscv/include/asm/syscall.h
> > > +++ b/arch/riscv/include/asm/syscall.h
> > > @@ -78,8 +78,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
> > >                                          const unsigned long *args)
> > >  {
> > >         regs->orig_a0 = args[0];
> > > -       args++;
> > > -       memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> > > +       regs->a1 = args[1];
> > > +       regs->a2 = args[2];
> > > +       regs->a3 = args[3];
> > > +       regs->a4 = args[4];
> > > +       regs->a5 = args[5];
> > >  }
> > 
> > I don't mind eliminating the memcpy() altogether, but
> > I'd like to note that syscall_set_arguments() is an exact mirror
> > of syscall_get_arguments(), so if the intentional overwrite in
> > syscall_set_arguments() is not acceptable, then the intentional
> > overread in syscall_get_arguments() shouldn't be acceptable either.
> 
> Yes, I noticed the symmetry too but I was only looking at it from the
> overwrite perspective, not the overread one. That reminded me to double
> check what fortify_memcpy_chk() actually checks for and I remembered
> that the overread version of this warning is hidden under W=1 (I guess
> because it happens more frequently).
> 
>   $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- W=1 allmodconfig kernel/ptrace.o
>   In file included from include/linux/string.h:392,
>                    from include/linux/bitmap.h:13,
>                    from include/linux/cpumask.h:12,
>                    from arch/riscv/include/asm/processor.h:55,
>                    from include/linux/sched.h:13,
>                    from kernel/ptrace.c:13:
>   In function 'fortify_memcpy_chk',
>       inlined from 'syscall_get_arguments.isra' at arch/riscv/include/asm/syscall.h:73:2:
>   include/linux/fortify-string.h:580:25: error: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
>     580 |                         __read_overflow2_field(q_size_field, size);
>         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   cc1: all warnings being treated as errors
> 
> So memcpy() should indeed be eliminated from both, which obviously
> clears up the warnings.
> 
> Cheers,
> Nathan
> 
> diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> index a5281cdf2b10..34313387f977 100644
> --- a/arch/riscv/include/asm/syscall.h
> +++ b/arch/riscv/include/asm/syscall.h
> @@ -69,8 +69,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
>  					 unsigned long *args)
>  {
>  	args[0] = regs->orig_a0;
> -	args++;
> -	memcpy(args, &regs->a1, 5 * sizeof(args[0]));
> +	args[1] = regs->a1;
> +	args[2] = regs->a2;
> +	args[3] = regs->a3;
> +	args[4] = regs->a4;
> +	args[5] = regs->a5;
>  }
>  
>  static inline void syscall_set_arguments(struct task_struct *task,
> @@ -78,8 +81,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
>  					 const unsigned long *args)
>  {
>  	regs->orig_a0 = args[0];
> -	args++;
> -	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> +	regs->a1 = args[1];
> +	regs->a2 = args[2];
> +	regs->a3 = args[3];
> +	regs->a4 = args[4];
> +	regs->a5 = args[5];
>  }
>  
>  static inline int syscall_get_arch(struct task_struct *task)

Looks good, thanks.  How do we proceed from this point?


-- 
ldv

Re: [PATCH v7 2/6] syscall.h: add syscall_set_arguments()

Posted by Nathan Chancellor 8 months, 1 week ago

On Wed, Apr 09, 2025 at 09:40:18AM +0300, Dmitry V. Levin wrote:
> On Tue, Apr 08, 2025 at 05:38:03PM -0700, Nathan Chancellor wrote:
> > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> > index a5281cdf2b10..34313387f977 100644
> > --- a/arch/riscv/include/asm/syscall.h
> > +++ b/arch/riscv/include/asm/syscall.h
> > @@ -69,8 +69,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
> >  					 unsigned long *args)
> >  {
> >  	args[0] = regs->orig_a0;
> > -	args++;
> > -	memcpy(args, &regs->a1, 5 * sizeof(args[0]));
> > +	args[1] = regs->a1;
> > +	args[2] = regs->a2;
> > +	args[3] = regs->a3;
> > +	args[4] = regs->a4;
> > +	args[5] = regs->a5;
> >  }
> >  
> >  static inline void syscall_set_arguments(struct task_struct *task,
> > @@ -78,8 +81,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
> >  					 const unsigned long *args)
> >  {
> >  	regs->orig_a0 = args[0];
> > -	args++;
> > -	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> > +	regs->a1 = args[1];
> > +	regs->a2 = args[2];
> > +	regs->a3 = args[3];
> > +	regs->a4 = args[4];
> > +	regs->a5 = args[5];
> >  }
> >  
> >  static inline int syscall_get_arch(struct task_struct *task)
> 
> Looks good, thanks.  How do we proceed from this point?

I can send a standalone patch for syscall_get_arguments() since that is
an issue present before your series then Andrew could fold in the same
change for syscall_set_arguments() into your change that introduces it
so there is no bisect problem?

Cheers,
Nathan

Re: [PATCH v7 2/6] syscall.h: add syscall_set_arguments()

Posted by Dmitry V. Levin 8 months, 1 week ago

On Wed, Apr 09, 2025 at 08:52:07AM -0700, Nathan Chancellor wrote:
> On Wed, Apr 09, 2025 at 09:40:18AM +0300, Dmitry V. Levin wrote:
> > On Tue, Apr 08, 2025 at 05:38:03PM -0700, Nathan Chancellor wrote:
> > > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> > > index a5281cdf2b10..34313387f977 100644
> > > --- a/arch/riscv/include/asm/syscall.h
> > > +++ b/arch/riscv/include/asm/syscall.h
> > > @@ -69,8 +69,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
> > >  					 unsigned long *args)
> > >  {
> > >  	args[0] = regs->orig_a0;
> > > -	args++;
> > > -	memcpy(args, &regs->a1, 5 * sizeof(args[0]));
> > > +	args[1] = regs->a1;
> > > +	args[2] = regs->a2;
> > > +	args[3] = regs->a3;
> > > +	args[4] = regs->a4;
> > > +	args[5] = regs->a5;
> > >  }
> > >  
> > >  static inline void syscall_set_arguments(struct task_struct *task,
> > > @@ -78,8 +81,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
> > >  					 const unsigned long *args)
> > >  {
> > >  	regs->orig_a0 = args[0];
> > > -	args++;
> > > -	memcpy(&regs->a1, args, 5 * sizeof(regs->a1));
> > > +	regs->a1 = args[1];
> > > +	regs->a2 = args[2];
> > > +	regs->a3 = args[3];
> > > +	regs->a4 = args[4];
> > > +	regs->a5 = args[5];
> > >  }
> > >  
> > >  static inline int syscall_get_arch(struct task_struct *task)
> > 
> > Looks good, thanks.  How do we proceed from this point?
> 
> I can send a standalone patch for syscall_get_arguments() since that is
> an issue present before your series then Andrew could fold in the same
> change for syscall_set_arguments() into your change that introduces it
> so there is no bisect problem?

Fine with me.  Thanks for taking care of this issue.


-- 
ldv

[PATCH v7 1/6] hexagon: add syscall_set_return_value()
[PATCH v7 2/6] syscall.h: add syscall_set_arguments()
[PATCH v7 3/6] syscall.h: introduce syscall_set_nr()
[PATCH v7 4/6] ptrace_get_syscall_info: factor out ptrace_get_syscall_info_op
[PATCH v7 5/6] ptrace: introduce PTRACE_SET_SYSCALL_INFO request
[PATCH v7 6/6] selftests/ptrace: add a test case for PTRACE_SET_SYSCALL_INFO