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

[Nathan Chancellor <nathan@kernel.org>]

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)