[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z63bI66KShw7Au_s@alpha.franken.de>
Date: Thu, 13 Feb 2025 12:44:35 +0100
From: Thomas Bogendoerfer <tsbogend@...ha.franken.de>
To: "Dmitry V. Levin" <ldv@...ace.io>
Cc: "Maciej W. Rozycki" <macro@...am.me.uk>,
Oleg Nesterov <oleg@...hat.com>, Alexey Gladkov <legion@...nel.org>,
Eugene Syromyatnikov <evgsyr@...il.com>,
Jiaxun Yang <jiaxun.yang@...goat.com>, strace-devel@...ts.strace.io,
linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] MIPS: fix mips_get_syscall_arg() for o32
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@...ace.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 ]
Powered by blists - more mailing lists