arch/riscv/include/asm/syscall.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
When building with CONFIG_FORTIFY_SOURCE=y and W=1, there is a warning
because of the memcpy() in syscall_get_arguments():
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:66: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
The fortified memcpy() routine enforces that the source is not overread
and the destination is not overwritten if the size of either field and
the size of the copy are known at compile time. The memcpy() in
syscall_get_arguments() intentionally overreads from a1 to a5 in
'struct pt_regs' but this is bigger than the size of a1.
Normally, this could be solved by wrapping a1 through a5 with
struct_group() but there was already a struct_group() applied to these
members in commit bba547810c66 ("riscv: tracing: Fix
__write_overflow_field in ftrace_partial_regs()").
Just avoid memcpy() altogether and write the copying of args from regs
manually, which clears up the warning at the expense of three extra
lines of code.
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
I omitted a Fixes tag because I think this has always been an overread
if I understand correctly but it is only the addition of the checks from
commit f68f2ff91512 ("fortify: Detect struct member overflows in
memcpy() at compile-time") that it becomes a noticeable issue.
This came out of a discussion from the addition of
syscall_set_arguments(), where the same logic causes a more noticeable
fortify warning because it happens without W=1, as it is an overwrite:
https://lore.kernel.org/20250408213131.GA2872426@ax162/
---
arch/riscv/include/asm/syscall.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
index 121fff429dce66b31fe79b691b8edd816c8019e9..eceabf59ae482aa1832b09371ddb3ba8cd65f91d 100644
--- a/arch/riscv/include/asm/syscall.h
+++ b/arch/riscv/include/asm/syscall.h
@@ -62,8 +62,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
unsigned long *args)
{
args[0] = regs->orig_a0;
- args++;
- memcpy(args, ®s->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 int syscall_get_arch(struct task_struct *task)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250409-riscv-avoid-fortify-warning-syscall_get_arguments-19c0495d4ed7
Best regards,
--
Nathan Chancellor <nathan@kernel.org>Hi Nathan,
On 09/04/2025 23:24, Nathan Chancellor wrote:
> When building with CONFIG_FORTIFY_SOURCE=y and W=1, there is a warning
> because of the memcpy() in syscall_get_arguments():
>
> 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:66: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
>
> The fortified memcpy() routine enforces that the source is not overread
> and the destination is not overwritten if the size of either field and
> the size of the copy are known at compile time. The memcpy() in
> syscall_get_arguments() intentionally overreads from a1 to a5 in
> 'struct pt_regs' but this is bigger than the size of a1.
>
> Normally, this could be solved by wrapping a1 through a5 with
> struct_group() but there was already a struct_group() applied to these
> members in commit bba547810c66 ("riscv: tracing: Fix
> __write_overflow_field in ftrace_partial_regs()").
>
> Just avoid memcpy() altogether and write the copying of args from regs
> manually, which clears up the warning at the expense of three extra
> lines of code.
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> I omitted a Fixes tag because I think this has always been an overread
> if I understand correctly but it is only the addition of the checks from
> commit f68f2ff91512 ("fortify: Detect struct member overflows in
> memcpy() at compile-time") that it becomes a noticeable issue.
>
> This came out of a discussion from the addition of
> syscall_set_arguments(), where the same logic causes a more noticeable
> fortify warning because it happens without W=1, as it is an overwrite:
> https://lore.kernel.org/20250408213131.GA2872426@ax162/
> ---
> arch/riscv/include/asm/syscall.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> index 121fff429dce66b31fe79b691b8edd816c8019e9..eceabf59ae482aa1832b09371ddb3ba8cd65f91d 100644
> --- a/arch/riscv/include/asm/syscall.h
> +++ b/arch/riscv/include/asm/syscall.h
> @@ -62,8 +62,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
> unsigned long *args)
> {
> args[0] = regs->orig_a0;
> - args++;
> - memcpy(args, ®s->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 int syscall_get_arch(struct task_struct *task)
>
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250409-riscv-avoid-fortify-warning-syscall_get_arguments-19c0495d4ed7
>
> Best regards,
Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
IIUC, Andrew took this patch, if that changes, please let me know and
I'll merge it through the riscv tree.
Thanks,
Alex
On Tue, Apr 15, 2025 at 07:54:04AM +0200, Alexandre Ghiti wrote:
> Hi Nathan,
>
> On 09/04/2025 23:24, Nathan Chancellor wrote:
> > When building with CONFIG_FORTIFY_SOURCE=y and W=1, there is a warning
> > because of the memcpy() in syscall_get_arguments():
> >
> > 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:66: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
> >
> > The fortified memcpy() routine enforces that the source is not overread
> > and the destination is not overwritten if the size of either field and
> > the size of the copy are known at compile time. The memcpy() in
> > syscall_get_arguments() intentionally overreads from a1 to a5 in
> > 'struct pt_regs' but this is bigger than the size of a1.
> >
> > Normally, this could be solved by wrapping a1 through a5 with
> > struct_group() but there was already a struct_group() applied to these
> > members in commit bba547810c66 ("riscv: tracing: Fix
> > __write_overflow_field in ftrace_partial_regs()").
> >
> > Just avoid memcpy() altogether and write the copying of args from regs
> > manually, which clears up the warning at the expense of three extra
> > lines of code.
> >
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> > I omitted a Fixes tag because I think this has always been an overread
> > if I understand correctly but it is only the addition of the checks from
> > commit f68f2ff91512 ("fortify: Detect struct member overflows in
> > memcpy() at compile-time") that it becomes a noticeable issue.
> >
> > This came out of a discussion from the addition of
> > syscall_set_arguments(), where the same logic causes a more noticeable
> > fortify warning because it happens without W=1, as it is an overwrite:
> > https://lore.kernel.org/20250408213131.GA2872426@ax162/
> > ---
> > arch/riscv/include/asm/syscall.h | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> > index 121fff429dce66b31fe79b691b8edd816c8019e9..eceabf59ae482aa1832b09371ddb3ba8cd65f91d 100644
> > --- a/arch/riscv/include/asm/syscall.h
> > +++ b/arch/riscv/include/asm/syscall.h
> > @@ -62,8 +62,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
> > unsigned long *args)
> > {
> > args[0] = regs->orig_a0;
> > - args++;
> > - memcpy(args, ®s->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 int syscall_get_arch(struct task_struct *task)
> >
> > ---
> > base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> > change-id: 20250409-riscv-avoid-fortify-warning-syscall_get_arguments-19c0495d4ed7
> >
> > Best regards,
>
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>
> IIUC, Andrew took this patch, if that changes, please let me know and I'll
> merge it through the riscv tree.
Thanks, I had Andrew drop it so that it could go via the riscv tree so
please pick it up when you can.
https://lore.kernel.org/20250411211833.E3DD1C4CEE2@smtp.kernel.org/
Cheers,
Nathan
On Wed, 09 Apr 2025 14:24:46 PDT (-0700), nathan@kernel.org wrote:
> When building with CONFIG_FORTIFY_SOURCE=y and W=1, there is a warning
> because of the memcpy() in syscall_get_arguments():
>
> 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:66: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
>
> The fortified memcpy() routine enforces that the source is not overread
> and the destination is not overwritten if the size of either field and
> the size of the copy are known at compile time. The memcpy() in
> syscall_get_arguments() intentionally overreads from a1 to a5 in
> 'struct pt_regs' but this is bigger than the size of a1.
>
> Normally, this could be solved by wrapping a1 through a5 with
> struct_group() but there was already a struct_group() applied to these
> members in commit bba547810c66 ("riscv: tracing: Fix
> __write_overflow_field in ftrace_partial_regs()").
>
> Just avoid memcpy() altogether and write the copying of args from regs
> manually, which clears up the warning at the expense of three extra
> lines of code.
You could still memcpy, but you'd need some sort of
memcpy(args, ®s->aregs + sizeof(args[0]), 5 * sizeof(args[0]));
(or however you index a struct group). I think it's saner to just do it
with the manual copies, though, as I'd have to look up what this code
does every time I run into it.
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> I omitted a Fixes tag because I think this has always been an overread
> if I understand correctly but it is only the addition of the checks from
> commit f68f2ff91512 ("fortify: Detect struct member overflows in
> memcpy() at compile-time") that it becomes a noticeable issue.
I'm going to add the Fixes. It's a suprious warning (there's no actual
crash from the overread), but even spurious warnings are a headache for
peolpe trying to build stuff.
> This came out of a discussion from the addition of
> syscall_set_arguments(), where the same logic causes a more noticeable
> fortify warning because it happens without W=1, as it is an overwrite:
> https://lore.kernel.org/20250408213131.GA2872426@ax162/
> ---
> arch/riscv/include/asm/syscall.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> index 121fff429dce66b31fe79b691b8edd816c8019e9..eceabf59ae482aa1832b09371ddb3ba8cd65f91d 100644
> --- a/arch/riscv/include/asm/syscall.h
> +++ b/arch/riscv/include/asm/syscall.h
> @@ -62,8 +62,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
> unsigned long *args)
> {
> args[0] = regs->orig_a0;
> - args++;
> - memcpy(args, ®s->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 int syscall_get_arch(struct task_struct *task)
>
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250409-riscv-avoid-fortify-warning-syscall_get_arguments-19c0495d4ed7
>
> Best regards,
On Wed, Apr 09, 2025 at 02:24:46PM -0700, Nathan Chancellor wrote:
> When building with CONFIG_FORTIFY_SOURCE=y and W=1, there is a warning
> because of the memcpy() in syscall_get_arguments():
>
> 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:66: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
>
> The fortified memcpy() routine enforces that the source is not overread
> and the destination is not overwritten if the size of either field and
> the size of the copy are known at compile time. The memcpy() in
> syscall_get_arguments() intentionally overreads from a1 to a5 in
> 'struct pt_regs' but this is bigger than the size of a1.
>
> Normally, this could be solved by wrapping a1 through a5 with
> struct_group() but there was already a struct_group() applied to these
> members in commit bba547810c66 ("riscv: tracing: Fix
> __write_overflow_field in ftrace_partial_regs()").
>
> Just avoid memcpy() altogether and write the copying of args from regs
> manually, which clears up the warning at the expense of three extra
> lines of code.
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> I omitted a Fixes tag because I think this has always been an overread
> if I understand correctly but it is only the addition of the checks from
> commit f68f2ff91512 ("fortify: Detect struct member overflows in
> memcpy() at compile-time") that it becomes a noticeable issue.
>
> This came out of a discussion from the addition of
> syscall_set_arguments(), where the same logic causes a more noticeable
> fortify warning because it happens without W=1, as it is an overwrite:
> https://lore.kernel.org/20250408213131.GA2872426@ax162/
> ---
> arch/riscv/include/asm/syscall.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> index 121fff429dce66b31fe79b691b8edd816c8019e9..eceabf59ae482aa1832b09371ddb3ba8cd65f91d 100644
> --- a/arch/riscv/include/asm/syscall.h
> +++ b/arch/riscv/include/asm/syscall.h
> @@ -62,8 +62,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
> unsigned long *args)
> {
> args[0] = regs->orig_a0;
> - args++;
> - memcpy(args, ®s->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 int syscall_get_arch(struct task_struct *task)
Reviewed-by: Dmitry V. Levin <ldv@strace.io>
--
ldv
© 2016 - 2026 Red Hat, Inc.