[PATCH] MIPS: Export syscall stack arguments properly for remote use

[PATCH] MIPS: Export syscall stack arguments properly for remote use

[Maciej W. Rozycki]

We have several places across the kernel where we want to access another 
task's syscall arguments, such as ptrace(2), seccomp(2), etc., by making 
a call to syscall_get_arguments().

This works for register arguments right away by accessing the task's 
`regs' member of `struct pt_regs', however for stack arguments seen with 
32-bit/o32 kernels things are more complicated.  Technically they ought 
to be obtained from the user stack with calls to an access_remote_vm(), 
but we have an easier way available already.

So as to be able to access syscall stack arguments as regular function 
arguments following the MIPS calling convention we copy them over from 
the user stack to the kernel stack in arch/mips/kernel/scall32-o32.S, in 
handle_sys(), to the current stack frame's outgoing argument space at 
the top of the stack, which is where the handler called expects to see 
its incoming arguments.  This area is also pointed at by the `pt_regs'
pointer obtained by task_pt_regs().

Make the o32 stack argument space a proper member of `struct pt_regs' 
then, by renaming the existing member from `pad0' to `args' and using 
generated offsets to access the space.  No functional change though.

With the change in place the o32 kernel stack frame layout at the entry 
to a syscall handler invoked by handle_sys() is therefore as follows:

$sp + 68 -> |         ...         | <- pt_regs.regs[9]
            +---------------------+
$sp + 64 -> |         $t0         | <- pt_regs.regs[8]
            +---------------------+
$sp + 60 -> |   $a3/argument #4   | <- pt_regs.regs[7]
            +---------------------+
$sp + 56 -> |   $a2/argument #3   | <- pt_regs.regs[6]
            +---------------------+
$sp + 52 -> |   $a1/argument #2   | <- pt_regs.regs[5]
            +---------------------+
$sp + 48 -> |   $a0/argument #1   | <- pt_regs.regs[4]
            +---------------------+
$sp + 44 -> |         $v1         | <- pt_regs.regs[3]
            +---------------------+
$sp + 40 -> |         $v0         | <- pt_regs.regs[2]
            +---------------------+
$sp + 36 -> |         $at         | <- pt_regs.regs[1]
            +---------------------+
$sp + 32 -> |        $zero        | <- pt_regs.regs[0]
            +---------------------+
$sp + 28 -> |  stack argument #8  | <- pt_regs.args[7]
            +---------------------+
$sp + 24 -> |  stack argument #7  | <- pt_regs.args[6]
            +---------------------+
$sp + 20 -> |  stack argument #6  | <- pt_regs.args[5]
            +---------------------+
$sp + 16 -> |  stack argument #5  | <- pt_regs.args[4]
            +---------------------+
$sp + 12 -> | psABI space for $a3 | <- pt_regs.args[3]
            +---------------------+
$sp +  8 -> | psABI space for $a2 | <- pt_regs.args[2]
            +---------------------+
$sp +  4 -> | psABI space for $a1 | <- pt_regs.args[1]
            +---------------------+
$sp +  0 -> | psABI space for $a0 | <- pt_regs.args[0]
            +---------------------+

holding user data received and with the first 4 frame slots reserved by 
the psABI for the compiler to spill the incoming arguments from $a0-$a3 
registers (which it sometimes does according to its needs) and the next 
4 frame slots designated by the psABI for any stack function arguments 
that follow.  This data is also available for other tasks to peek/poke 
at as reqired and where permitted.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
 arch/mips/include/asm/ptrace.h |    4 ++--
 arch/mips/kernel/asm-offsets.c |    6 ++++++
 arch/mips/kernel/scall32-o32.S |    8 ++++----
 3 files changed, 12 insertions(+), 6 deletions(-)

linux-mips-scall32-o32-pt-regs-args.diff
Index: linux-malta/arch/mips/include/asm/ptrace.h
===================================================================
--- linux-malta.orig/arch/mips/include/asm/ptrace.h
+++ linux-malta/arch/mips/include/asm/ptrace.h
@@ -27,8 +27,8 @@
  */
 struct pt_regs {
 #ifdef CONFIG_32BIT
-	/* Pad bytes for argument save space on the stack. */
-	unsigned long pad0[8];
+	/* Saved syscall stack arguments; entries 0-3 unused. */
+	unsigned long args[8];
 #endif
 
 	/* Saved main processor registers. */
Index: linux-malta/arch/mips/kernel/asm-offsets.c
===================================================================
--- linux-malta.orig/arch/mips/kernel/asm-offsets.c
+++ linux-malta/arch/mips/kernel/asm-offsets.c
@@ -27,6 +27,12 @@
 void output_ptreg_defines(void)
 {
 	COMMENT("MIPS pt_regs offsets.");
+#ifdef CONFIG_32BIT
+	OFFSET(PT_ARG4, pt_regs, args[4]);
+	OFFSET(PT_ARG5, pt_regs, args[5]);
+	OFFSET(PT_ARG6, pt_regs, args[6]);
+	OFFSET(PT_ARG7, pt_regs, args[7]);
+#endif
 	OFFSET(PT_R0, pt_regs, regs[0]);
 	OFFSET(PT_R1, pt_regs, regs[1]);
 	OFFSET(PT_R2, pt_regs, regs[2]);
Index: linux-malta/arch/mips/kernel/scall32-o32.S
===================================================================
--- linux-malta.orig/arch/mips/kernel/scall32-o32.S
+++ linux-malta/arch/mips/kernel/scall32-o32.S
@@ -64,10 +64,10 @@ load_a6: user_lw(t7, 24(t0))		# argument
 load_a7: user_lw(t8, 28(t0))		# argument #8 from usp
 loads_done:
 
-	sw	t5, 16(sp)		# argument #5 to ksp
-	sw	t6, 20(sp)		# argument #6 to ksp
-	sw	t7, 24(sp)		# argument #7 to ksp
-	sw	t8, 28(sp)		# argument #8 to ksp
+	sw	t5, PT_ARG4(sp)		# argument #5 to ksp
+	sw	t6, PT_ARG5(sp)		# argument #6 to ksp
+	sw	t7, PT_ARG6(sp)		# argument #7 to ksp
+	sw	t8, PT_ARG7(sp)		# argument #8 to ksp
 	.set	pop
 
 	.section __ex_table,"a"

[PATCH] MIPS: fix mips_get_syscall_arg() for o32

[Dmitry V. Levin]

This makes ptrace/get_syscall_info selftest pass on mips o32 and
mips64 o32 by fixing the following two test assertions:

1. get_syscall_info test assertion on mips o32:
  # get_syscall_info.c:218:get_syscall_info:Expected exp_args[5] (3134521044) == info.entry.args[4] (4911432)
  # get_syscall_info.c:219:get_syscall_info:wait #1: entry stop mismatch

2. get_syscall_info test assertion on mips64 o32:
  # get_syscall_info.c:209:get_syscall_info:Expected exp_args[2] (3134324433) == info.entry.args[1] (18446744072548908753)
  # get_syscall_info.c:210:get_syscall_info:wait #1: entry stop mismatch

The first assertion happens due to mips_get_syscall_arg() trying to access
another task's context but failing to do it properly because get_user() it
calls just peeks at the current task's context.  It usually does not crash
because the default user stack always gets assigned the same VMA, but it
is pure luck which mips_get_syscall_arg() wouldn't have if e.g. the stack
was switched (via setcontext(3) or however) or a non-default process's
thread peeked at, and in any case irrelevant data is obtained just as
observed with the test case.

mips_get_syscall_arg() ought to be using access_remote_vm() instead to
retrieve the other task's stack contents, but given that the data has been
already obtained and saved in `struct pt_regs' it would be an overkill.

The first assertion is fixed for mips o32 by using struct pt_regs.args
instead of get_user() to obtain syscall arguments.  This approach works
due to this piece in arch/mips/kernel/scall32-o32.S:

        /*
         * Ok, copy the args from the luser stack to the kernel stack.
         */

        .set    push
        .set    noreorder
        .set    nomacro

    load_a4: user_lw(t5, 16(t0))		# argument #5 from usp
    load_a5: user_lw(t6, 20(t0))		# argument #6 from usp
    load_a6: user_lw(t7, 24(t0))		# argument #7 from usp
    load_a7: user_lw(t8, 28(t0))		# argument #8 from usp
    loads_done:

        sw	t5, PT_ARG4(sp)		# argument #5 to ksp
        sw	t6, PT_ARG5(sp)		# argument #6 to ksp
        sw	t7, PT_ARG6(sp)		# argument #7 to ksp
        sw	t8, PT_ARG7(sp)		# argument #8 to ksp
        .set	pop

        .section __ex_table,"a"
        PTR_WD	load_a4, bad_stack_a4
        PTR_WD	load_a5, bad_stack_a5
        PTR_WD	load_a6, bad_stack_a6
        PTR_WD	load_a7, bad_stack_a7
        .previous

arch/mips/kernel/scall64-o32.S has analogous code for mips64 o32 that
allows fixing the issue by obtaining syscall arguments from struct
pt_regs.regs[4..11] instead of the erroneous use of get_user().

The second assertion is fixed by truncating 64-bit values to 32-bit
syscall arguments.

Fixes: c0ff3c53d4f9 ("MIPS: Enable HAVE_ARCH_TRACEHOOK.")
Signed-off-by: Dmitry V. Levin <ldv@strace.io>
---

This started as a part of PTRACE_SET_SYSCALL_INFO API series[1].
It has been rebased on top of [2] as suggested by Maciej in [3].

[1] https://lore.kernel.org/all/20250210113336.GA887@strace.io/
[2] https://lore.kernel.org/all/alpine.DEB.2.21.2502101732120.65342@angie.orcam.me.uk/
[3] https://lore.kernel.org/all/alpine.DEB.2.21.2502111530080.65342@angie.orcam.me.uk/

 arch/mips/include/asm/syscall.h | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
index ebdf4d910af2..056aa1b713e2 100644
--- a/arch/mips/include/asm/syscall.h
+++ b/arch/mips/include/asm/syscall.h
@@ -57,37 +57,21 @@ static inline void mips_syscall_update_nr(struct task_struct *task,
 static inline void mips_get_syscall_arg(unsigned long *arg,
 	struct task_struct *task, struct pt_regs *regs, unsigned int n)
 {
-	unsigned long usp __maybe_unused = regs->regs[29];
-
+#ifdef CONFIG_32BIT
 	switch (n) {
 	case 0: case 1: case 2: case 3:
 		*arg = regs->regs[4 + n];
-
-		return;
-
-#ifdef CONFIG_32BIT
-	case 4: case 5: case 6: case 7:
-		get_user(*arg, (int *)usp + n);
 		return;
-#endif
-
-#ifdef CONFIG_64BIT
 	case 4: case 5: case 6: case 7:
-#ifdef CONFIG_MIPS32_O32
-		if (test_tsk_thread_flag(task, TIF_32BIT_REGS))
-			get_user(*arg, (int *)usp + n);
-		else
-#endif
-			*arg = regs->regs[4 + n];
-
+		*arg = regs->args[n];
 		return;
-#endif
-
-	default:
-		BUG();
 	}
-
-	unreachable();
+#else
+	*arg = regs->regs[4 + n];
+	if ((IS_ENABLED(CONFIG_MIPS32_O32) &&
+	     test_tsk_thread_flag(task, TIF_32BIT_REGS)))
+		*arg = (unsigned int)*arg;
+#endif
 }
 
 static inline long syscall_get_error(struct task_struct *task,
-- 
ldv

Re: [PATCH] MIPS: fix mips_get_syscall_arg() for o32

[Thomas Bogendoerfer]

On Wed, Feb 12, 2025 at 01:02:09AM +0200, Dmitry V. Levin wrote:
> This makes ptrace/get_syscall_info selftest pass on mips o32 and
> mips64 o32 by fixing the following two test assertions:
> 
> 1. get_syscall_info test assertion on mips o32:
>   # get_syscall_info.c:218:get_syscall_info:Expected exp_args[5] (3134521044) == info.entry.args[4] (4911432)
>   # get_syscall_info.c:219:get_syscall_info:wait #1: entry stop mismatch
> 
> 2. get_syscall_info test assertion on mips64 o32:
>   # get_syscall_info.c:209:get_syscall_info:Expected exp_args[2] (3134324433) == info.entry.args[1] (18446744072548908753)
>   # get_syscall_info.c:210:get_syscall_info:wait #1: entry stop mismatch
> 
> The first assertion happens due to mips_get_syscall_arg() trying to access
> another task's context but failing to do it properly because get_user() it
> calls just peeks at the current task's context.  It usually does not crash
> because the default user stack always gets assigned the same VMA, but it
> is pure luck which mips_get_syscall_arg() wouldn't have if e.g. the stack
> was switched (via setcontext(3) or however) or a non-default process's
> thread peeked at, and in any case irrelevant data is obtained just as
> observed with the test case.
> 
> mips_get_syscall_arg() ought to be using access_remote_vm() instead to
> retrieve the other task's stack contents, but given that the data has been
> already obtained and saved in `struct pt_regs' it would be an overkill.
> 
> The first assertion is fixed for mips o32 by using struct pt_regs.args
> instead of get_user() to obtain syscall arguments.  This approach works
> due to this piece in arch/mips/kernel/scall32-o32.S:
> 
>         /*
>          * Ok, copy the args from the luser stack to the kernel stack.
>          */
> 
>         .set    push
>         .set    noreorder
>         .set    nomacro
> 
>     load_a4: user_lw(t5, 16(t0))		# argument #5 from usp
>     load_a5: user_lw(t6, 20(t0))		# argument #6 from usp
>     load_a6: user_lw(t7, 24(t0))		# argument #7 from usp
>     load_a7: user_lw(t8, 28(t0))		# argument #8 from usp
>     loads_done:
> 
>         sw	t5, PT_ARG4(sp)		# argument #5 to ksp
>         sw	t6, PT_ARG5(sp)		# argument #6 to ksp
>         sw	t7, PT_ARG6(sp)		# argument #7 to ksp
>         sw	t8, PT_ARG7(sp)		# argument #8 to ksp
>         .set	pop
> 
>         .section __ex_table,"a"
>         PTR_WD	load_a4, bad_stack_a4
>         PTR_WD	load_a5, bad_stack_a5
>         PTR_WD	load_a6, bad_stack_a6
>         PTR_WD	load_a7, bad_stack_a7
>         .previous
> 
> arch/mips/kernel/scall64-o32.S has analogous code for mips64 o32 that
> allows fixing the issue by obtaining syscall arguments from struct
> pt_regs.regs[4..11] instead of the erroneous use of get_user().
> 
> The second assertion is fixed by truncating 64-bit values to 32-bit
> syscall arguments.
> 
> Fixes: c0ff3c53d4f9 ("MIPS: Enable HAVE_ARCH_TRACEHOOK.")
> Signed-off-by: Dmitry V. Levin <ldv@strace.io>
> ---
> 
> This started as a part of PTRACE_SET_SYSCALL_INFO API series[1].
> It has been rebased on top of [2] as suggested by Maciej in [3].
> 
> [1] https://lore.kernel.org/all/20250210113336.GA887@strace.io/
> [2] https://lore.kernel.org/all/alpine.DEB.2.21.2502101732120.65342@angie.orcam.me.uk/
> [3] https://lore.kernel.org/all/alpine.DEB.2.21.2502111530080.65342@angie.orcam.me.uk/
> 
>  arch/mips/include/asm/syscall.h | 32 ++++++++------------------------
>  1 file changed, 8 insertions(+), 24 deletions(-)

applied to mips-fixes

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH] MIPS: Export syscall stack arguments properly for remote use

[Thomas Bogendoerfer]

On Tue, Feb 11, 2025 at 06:22:30PM +0000, Maciej W. Rozycki wrote:
> We have several places across the kernel where we want to access another 
> task's syscall arguments, such as ptrace(2), seccomp(2), etc., by making 
> a call to syscall_get_arguments().
> 
> This works for register arguments right away by accessing the task's 
> `regs' member of `struct pt_regs', however for stack arguments seen with 
> 32-bit/o32 kernels things are more complicated.  Technically they ought 
> to be obtained from the user stack with calls to an access_remote_vm(), 
> but we have an easier way available already.
> 
> So as to be able to access syscall stack arguments as regular function 
> arguments following the MIPS calling convention we copy them over from 
> the user stack to the kernel stack in arch/mips/kernel/scall32-o32.S, in 
> handle_sys(), to the current stack frame's outgoing argument space at 
> the top of the stack, which is where the handler called expects to see 
> its incoming arguments.  This area is also pointed at by the `pt_regs'
> pointer obtained by task_pt_regs().
> 
> Make the o32 stack argument space a proper member of `struct pt_regs' 
> then, by renaming the existing member from `pad0' to `args' and using 
> generated offsets to access the space.  No functional change though.
> 
> With the change in place the o32 kernel stack frame layout at the entry 
> to a syscall handler invoked by handle_sys() is therefore as follows:
> 
> $sp + 68 -> |         ...         | <- pt_regs.regs[9]
>             +---------------------+
> $sp + 64 -> |         $t0         | <- pt_regs.regs[8]
>             +---------------------+
> $sp + 60 -> |   $a3/argument #4   | <- pt_regs.regs[7]
>             +---------------------+
> $sp + 56 -> |   $a2/argument #3   | <- pt_regs.regs[6]
>             +---------------------+
> $sp + 52 -> |   $a1/argument #2   | <- pt_regs.regs[5]
>             +---------------------+
> $sp + 48 -> |   $a0/argument #1   | <- pt_regs.regs[4]
>             +---------------------+
> $sp + 44 -> |         $v1         | <- pt_regs.regs[3]
>             +---------------------+
> $sp + 40 -> |         $v0         | <- pt_regs.regs[2]
>             +---------------------+
> $sp + 36 -> |         $at         | <- pt_regs.regs[1]
>             +---------------------+
> $sp + 32 -> |        $zero        | <- pt_regs.regs[0]
>             +---------------------+
> $sp + 28 -> |  stack argument #8  | <- pt_regs.args[7]
>             +---------------------+
> $sp + 24 -> |  stack argument #7  | <- pt_regs.args[6]
>             +---------------------+
> $sp + 20 -> |  stack argument #6  | <- pt_regs.args[5]
>             +---------------------+
> $sp + 16 -> |  stack argument #5  | <- pt_regs.args[4]
>             +---------------------+
> $sp + 12 -> | psABI space for $a3 | <- pt_regs.args[3]
>             +---------------------+
> $sp +  8 -> | psABI space for $a2 | <- pt_regs.args[2]
>             +---------------------+
> $sp +  4 -> | psABI space for $a1 | <- pt_regs.args[1]
>             +---------------------+
> $sp +  0 -> | psABI space for $a0 | <- pt_regs.args[0]
>             +---------------------+
> 
> holding user data received and with the first 4 frame slots reserved by 
> the psABI for the compiler to spill the incoming arguments from $a0-$a3 
> registers (which it sometimes does according to its needs) and the next 
> 4 frame slots designated by the psABI for any stack function arguments 
> that follow.  This data is also available for other tasks to peek/poke 
> at as reqired and where permitted.
> 
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> ---
>  arch/mips/include/asm/ptrace.h |    4 ++--
>  arch/mips/kernel/asm-offsets.c |    6 ++++++
>  arch/mips/kernel/scall32-o32.S |    8 ++++----
>  3 files changed, 12 insertions(+), 6 deletions(-)

applied to mips-fixes

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]