All of lore.kernel.org
 help / color / mirror / Atom feed
* + syscallh-add-syscall_set_arguments-fix.patch added to mm-new branch
@ 2025-04-11  1:43 Andrew Morton
  2025-04-11  2:47 ` Nathan Chancellor
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2025-04-11  1:43 UTC (permalink / raw)
  To: mm-commits, paul.walmsley, palmer, ldv, charlie, aou, alex,
	nathan, akpm


The patch titled
     Subject: riscv: avoid fortify warning in syscall_get_arguments()
has been added to the -mm mm-new branch.  Its filename is
     syscallh-add-syscall_set_arguments-fix.patch

This patch will shortly appear at
     https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/syscallh-add-syscall_set_arguments-fix.patch

This patch will later appear in the mm-new branch at
    git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days

------------------------------------------------------
From: Nathan Chancellor <nathan@kernel.org>
Subject: riscv: avoid fortify warning in syscall_get_arguments()
Date: Wed, 09 Apr 2025 14:24:46 -0700

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.

Link: https://lkml.kernel.org/r/20250409-riscv-avoid-fortify-warning-syscall_get_arguments-v1-1-7853436d4755@kernel.org
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Dmitry V. Levin <ldv@strace.io>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: Alexandre Ghiti <alex@ghiti.fr>
Cc: Charlie Jenkins <charlie@rivosinc.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/riscv/include/asm/syscall.h |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

--- a/arch/riscv/include/asm/syscall.h~syscallh-add-syscall_set_arguments-fix
+++ a/arch/riscv/include/asm/syscall.h
@@ -62,8 +62,11 @@ static inline void syscall_get_arguments
 					 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,
_

Patches currently in -mm which might be from nathan@kernel.org are

syscallh-add-syscall_set_arguments-fix.patch


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: + syscallh-add-syscall_set_arguments-fix.patch added to mm-new branch
  2025-04-11  1:43 + syscallh-add-syscall_set_arguments-fix.patch added to mm-new branch Andrew Morton
@ 2025-04-11  2:47 ` Nathan Chancellor
  2025-04-11  4:06   ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Nathan Chancellor @ 2025-04-11  2:47 UTC (permalink / raw)
  To: Andrew Morton; +Cc: mm-commits, paul.walmsley, palmer, ldv, charlie, aou, alex

Hi Andrew,

On Thu, Apr 10, 2025 at 06:43:03PM -0700, Andrew Morton wrote:
> 
> The patch titled
>      Subject: riscv: avoid fortify warning in syscall_get_arguments()
> has been added to the -mm mm-new branch.  Its filename is
>      syscallh-add-syscall_set_arguments-fix.patch
...
> ------------------------------------------------------
> From: Nathan Chancellor <nathan@kernel.org>
> Subject: riscv: avoid fortify warning in syscall_get_arguments()

I do not think I made myself clear enough in my request when reading it
back, so I do apologize for that. I need the diff at the end of [1]
applied to syscallh-add-syscall_set_arguments.patch, not [2], which is a
standalone patch for the existing syscall_get_arguments() that should go
via the RISC-V tree because it is an existing issue (or at least be kept
in standalone patch form).

[1]: https://lore.kernel.org/20250408213131.GA2872426@ax162/
[2]: https://lore.kernel.org/20250409-riscv-avoid-fortify-warning-syscall_get_arguments-v1-1-7853436d4755@kernel.org/

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* + syscallh-add-syscall_set_arguments-fix.patch added to mm-new branch
@ 2025-04-11  3:56 Andrew Morton
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2025-04-11  3:56 UTC (permalink / raw)
  To: mm-commits, macro, deller, charlie, nathan, akpm


The patch titled
     Subject: syscallh-add-syscall_set_arguments-fix
has been added to the -mm mm-new branch.  Its filename is
     syscallh-add-syscall_set_arguments-fix.patch

This patch will shortly appear at
     https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/syscallh-add-syscall_set_arguments-fix.patch

This patch will later appear in the mm-new branch at
    git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days

------------------------------------------------------
From: Nathan Chancellor <nathan@kernel.org>
Subject: syscallh-add-syscall_set_arguments-fix
Date: Tue, 8 Apr 2025 14:31:31 -0700

fix compile time fortify checks

Link: https://lkml.kernel.org/r/20250408213131.GA2872426@ax162
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Cc: Charlie Jenkins <charlie@rivosinc.com>
Cc: Helge Deller <deller@gmx.de> # parisc
Cc: Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/riscv/include/asm/syscall.h |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

--- a/arch/riscv/include/asm/syscall.h~syscallh-add-syscall_set_arguments-fix
+++ a/arch/riscv/include/asm/syscall.h
@@ -71,8 +71,11 @@ static inline void syscall_set_arguments
 					 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)
_

Patches currently in -mm which might be from nathan@kernel.org are

syscallh-add-syscall_set_arguments-fix.patch
riscv-avoid-fortify-warning-in-syscall_get_arguments.patch


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: + syscallh-add-syscall_set_arguments-fix.patch added to mm-new branch
  2025-04-11  2:47 ` Nathan Chancellor
@ 2025-04-11  4:06   ` Andrew Morton
  2025-04-11 14:47     ` Nathan Chancellor
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2025-04-11  4:06 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: mm-commits, paul.walmsley, palmer, ldv, charlie, aou, alex

On Thu, 10 Apr 2025 19:47:26 -0700 Nathan Chancellor <nathan@kernel.org> wrote:

> Hi Andrew,
> 
> On Thu, Apr 10, 2025 at 06:43:03PM -0700, Andrew Morton wrote:
> > 
> > The patch titled
> >      Subject: riscv: avoid fortify warning in syscall_get_arguments()
> > has been added to the -mm mm-new branch.  Its filename is
> >      syscallh-add-syscall_set_arguments-fix.patch
> ...
> > ------------------------------------------------------
> > From: Nathan Chancellor <nathan@kernel.org>
> > Subject: riscv: avoid fortify warning in syscall_get_arguments()
> 
> I do not think I made myself clear enough in my request when reading it
> back, so I do apologize for that. I need the diff at the end of [1]
> applied to syscallh-add-syscall_set_arguments.patch, not [2], which is a
> standalone patch for the existing syscall_get_arguments() that should go
> via the RISC-V tree because it is an existing issue (or at least be kept
> in standalone patch form).
> 
> [1]: https://lore.kernel.org/20250408213131.GA2872426@ax162/

This fixed syscall_set_arguments()_ only.

> [2]: https://lore.kernel.org/20250409-riscv-avoid-fortify-warning-syscall_get_arguments-v1-1-7853436d4755@kernel.org/

Whereas https://lkml.kernel.org/r/20250409003803.GA2876360@ax162 fixed
syscall_get_arguments() also.

I queued both fixes as separate patches.  And I reverted your mail
client's conversion of tabs to spaces and I added signoffs and all the
usual whatever.

Please check mm-new once I get it pushed out.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: + syscallh-add-syscall_set_arguments-fix.patch added to mm-new branch
  2025-04-11  4:06   ` Andrew Morton
@ 2025-04-11 14:47     ` Nathan Chancellor
  0 siblings, 0 replies; 5+ messages in thread
From: Nathan Chancellor @ 2025-04-11 14:47 UTC (permalink / raw)
  To: Andrew Morton; +Cc: mm-commits, paul.walmsley, palmer, ldv, charlie, aou, alex

On Thu, Apr 10, 2025 at 09:06:34PM -0700, Andrew Morton wrote:
> On Thu, 10 Apr 2025 19:47:26 -0700 Nathan Chancellor <nathan@kernel.org> wrote:
> 
> > Hi Andrew,
> > 
> > On Thu, Apr 10, 2025 at 06:43:03PM -0700, Andrew Morton wrote:
> > > 
> > > The patch titled
> > >      Subject: riscv: avoid fortify warning in syscall_get_arguments()
> > > has been added to the -mm mm-new branch.  Its filename is
> > >      syscallh-add-syscall_set_arguments-fix.patch
> > ...
> > > ------------------------------------------------------
> > > From: Nathan Chancellor <nathan@kernel.org>
> > > Subject: riscv: avoid fortify warning in syscall_get_arguments()
> > 
> > I do not think I made myself clear enough in my request when reading it
> > back, so I do apologize for that. I need the diff at the end of [1]
> > applied to syscallh-add-syscall_set_arguments.patch, not [2], which is a
> > standalone patch for the existing syscall_get_arguments() that should go
> > via the RISC-V tree because it is an existing issue (or at least be kept
> > in standalone patch form).
> > 
> > [1]: https://lore.kernel.org/20250408213131.GA2872426@ax162/
> 
> This fixed syscall_set_arguments()_ only.
> 
> > [2]: https://lore.kernel.org/20250409-riscv-avoid-fortify-warning-syscall_get_arguments-v1-1-7853436d4755@kernel.org/
> 
> Whereas https://lkml.kernel.org/r/20250409003803.GA2876360@ax162 fixed
> syscall_get_arguments() also.

Yes, I merely meant for that to be a demonstration of what resolves both
fortify warnings.

> I queued both fixes as separate patches.  And I reverted your mail
> client's conversion of tabs to spaces and I added signoffs and all the
> usual whatever.
> 
> Please check mm-new once I get it pushed out.

The fix for syscall_set_arguments() looks properly done in mm-unstable
but I do not understand why the fix for syscall_get_arguments() is
marked as another fix to be squashed into the patch that adds
syscall_set_arguments()? It is an issue that is present in Linus's tree
currently, not one that appears after Dmitry's patch. Just drop the
patch that fixes syscall_get_arguments(), it is my understanding that
the RISC-V folks will handle it (or if you are adamant about carrying
it, keep it as a standalone patch, not one to be squashed). I just need
syscall_set_arguments() fixed by you because you are carrying the patch
that adds it per the plan at
https://lore.kernel.org/20250409155207.GA1506425@ax162/

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-04-11 14:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-11  1:43 + syscallh-add-syscall_set_arguments-fix.patch added to mm-new branch Andrew Morton
2025-04-11  2:47 ` Nathan Chancellor
2025-04-11  4:06   ` Andrew Morton
2025-04-11 14:47     ` Nathan Chancellor
  -- strict thread matches above, loose matches on Subject: below --
2025-04-11  3:56 Andrew Morton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.