Add 64-bit LoongArch support #205
Conversation
| * SPDX-License-Identifier: LGPL-2.1-or-later | ||
| */ | ||
|
|
||
| #define PERSONALITY0_AUDIT_ARCH { AUDIT_ARCH_LOONGARCH64, 0 } |
There was a problem hiding this comment.
Please add AUDIT_ARCH_LOONGARCH64 to src/xlat/audit_arch.in, too.
There was a problem hiding this comment.
Sure; I'll do so later (after finishing my $DAY_JOB).
ldv-alt
force-pushed
the
loongarch-master
branch
from
848941c to
3cca56a
Compare
|
I've tested this on gcc401.fsffrance.org: Testsuite summary: FAIL: bpf-success.gen In other words, all tests based on syscall tampering are failing there. Assuming that src/linux/loongarch64/set_scno.c and src/linux/loongarch64/set_error.c are correct, |
Codecov Report
@@ Coverage Diff @@
## master #205 +/- ##
==========================================
- Coverage 89.88% 89.88% -0.01%
==========================================
Files 287 287
Lines 24047 24047
==========================================
- Hits 21615 21614 -1
- Misses 2432 2433 +1
Continue to review full report at Codecov.
|
|
cc @chenhuacai |
Okay... this is the same result I've been seeing all the time (and exactly why I haven't pushed the It indeed seems ptrace functionality is missing, judging from the latest kernel source. Syscall tracing machinery cannot be seen. Maybe @chenhuacai could implement this in next revision of loongarch-next series? |
There is some support for changing the syscall nr in do_syscall, but, apparently, it doesn't work as expected. |
|
Well, the syscall cancellation itself seems to be working, it's the return value handling that is not implemented properly in the kernel: You can see from this example that in case of syscall cancellation the first syscall argument is returned unconditionally as the return value. |
|
The problematic piece of kernel code is Here the |
This piece of code is correct for SECCOMP because the latter calls Unfortunately, this approach is not applicable for ptrace: I'm afraid |
Hi, could you please change do_syscall() to be like this and test ptrace/strace (I have tested seccomp)? } |
This way the first syscall argument won't be available for ptracers because |
Hmm, can we keep do_syscall() as is and solve the ptrace problem by overriding arch_syscall_enter_tracehook() like this? static inline __must_check int arch_syscall_enter_tracehook(struct pt_regs *regs) } |
see strace/strace#205 Suggested-by: Huacai Chen <chenhuacai@kernel.org> Signed-off-by: WANG Xuerui <git@xen0n.name>
Hi @chenhuacai, I tried in this commit and confirmed it's not working; exactly the same failures as before. |
It's better to use
No, this cannot help to solve the problem because syscall tampering doesn't make However, this approach makes sense for a different reason - the lengthy comment before |
I'm puzzled here. I think the original problem is if syscall_enter_from_user_mode() return -1 by ptrace we should make the syscall return -ENOSYS. And we don't need to do anything if tracehook_report_syscall_entry() return 0. Because if tracehook_report_syscall_entry() return 0, then syscall_enter_from_user_mode() also return 0 (there is no SYSCALL_WORK_SYSCALL_EMU on LoongArch), and then do_syscall() will do the real syscall.
|
The problem is that btw, another problem with the current implementation of |
|
With @chenhuacai's latest patch, every test passes except this: |
This looks like a race, how often do you see this? |
Yes, this looks plausible. As I said earlier, another problem with the current implementation of |
Exposing orig_a0 means modifying user_pt_regs, gpr_get(), gpr_set() and all tracers (gdb, strace, etc) and don't need to overriding arch_syscall_enter_tracehook(), right? |
Hmm, I'm back home, and just re-run that failing case multiple times with the command line you provided, and it now passes every time. 🤦 |
|
On Sun, Jan 09, 2022 at 05:58:44AM -0800, Huacai Chen wrote:
> > With @chenhuacai's [latest patch](xen0n/linux@593f8c0), every test passes except this:
>
> Yes, this looks plausible.
>
> As I said earlier, another problem with the current implementation of `do_syscall` is that unlike other architectures, here the ptracer cannot change the first syscall argument because it is currently stored in `regs->orig_a0` which is out of reach of ptracers. What do you think about exposing `regs->orig_a0` to ptracers?
Exposing orig_a0 means modifying user_pt_regs, gpr_get(), gpr_set() and all tracers (gdb, strace, etc) and don't need to overriding arch_syscall_enter_tracehook(), right?
Yes, exposing orig_a0 means modifying user_pt_regs, gpr_get(), gpr_set(),
and all tracers.
This would allow to do
regs->regs[4] = -ENOSYS;
in do_syscall() right before the syscall_enter_from_user_mode()
invocation, and there won't be any need to override
arch_syscall_enter_tracehook() then.
|
ldv-alt
force-pushed
the
loongarch-master
branch
from
79ed461 to
644ac28
Compare
Hmm, there is a more simple solution, overide arch_syscall_enter_tracehook() as below: static inline __must_check int arch_syscall_enter_tracehook(struct pt_regs *regs) } |
|
The latest suggested modification made all tests pass: (BTW please fix your Markdown syntax @chenhuacai) |
|
On Wed, Jan 12, 2022 at 01:13:01AM -0800, WÁNG Xuěruì wrote:
The [latest suggested modification](xen0n/linux@60ef894) made all tests pass:
Yes, this should work indeed, thanks.
|
|
On Wed, Jan 12, 2022 at 04:05:21AM -0800, Dmitry V. Levin wrote:
On Wed, Jan 12, 2022 at 01:13:01AM -0800, WÁNG Xuěruì wrote:
> The [latest suggested modification](xen0n/linux@60ef894) made all tests pass:
Yes, this should work indeed, thanks.
I suppose this PR could be merged as soon as it passes all tests on a
cfarm loongarch host.
|
I've pinged @chenhuacai for arranging the kernel updates for the cfarm hosts; meanwhile the test failures in the commit message could be removed as well. |
kernel has been updated. |
xen0n
force-pushed
the
loongarch-master
branch
from
644ac28 to
351b5fe
Compare
|
I've just rebased to latest master branch and removed the test failure message from the commit message; I'll mark this as ready after re-running the test suite. |
|
Tests pass on my development box:
I think this should be ready now. Thanks for the help and revision to my code! |
xen0n
changed the title
This is based on the WIP Linux port still under review, but the port is re-using asm-generic syscall numbers and parameters, so breakage should be minimal when the port is eventually merged, if at all. Currently only the LP64* ABIs are implemented in the toolchains, so only support the host type "loongarch64". * NEWS: Mention this. * configure.ac [$host_cpu == loongarch64]: Define LOONGARCH64. * src/linux/loongarch64/arch_defs_.h: New file. * src/linux/loongarch64/arch_regs.c: Likewise. * src/linux/loongarch64/get_error.c: Likewise. * src/linux/loongarch64/get_scno.c: Likewise. * src/linux/loongarch64/get_syscall_args.c: Likewise. * src/linux/loongarch64/ioctls_arch0.h: Likewise. * src/linux/loongarch64/ioctls_inc0.h: Likewise. * src/linux/loongarch64/raw_syscall.h: Likewise. * src/linux/loongarch64/set_error.c: Likewise. * src/linux/loongarch64/set_scno.c: Likewise. * src/linux/loongarch64/syscallent.h: Likewise. * src/Makefile.am (EXTRA_DIST): Add them. * src/xlat/elf_em.in: Add EM_LOONGARCH. Link: https://lore.kernel.org/lkml/20211013063656.3084555-1-chenhuacai@loongson.cn/
* src/xlat/audit_arch.in (AUDIT_ARCH_LOONGARCH32, AUDIT_ARCH_LOONGARCH64): New constants. Link: https://lore.kernel.org/lkml/20211013063656.3084555-4-chenhuacai@loongson.cn/
* src/linux/loongarch64/arch_fpregset.c: New file. * src/linux/loongarch64/arch_fpregset.h: Likewise. * src/linux/loongarch64/arch_prstatus_regset.c: Likewise. * src/linux/loongarch64/arch_prstatus_regset.h: Likewise. * src/Makefile.am (EXTRA_DIST): Add them. * tests/ptrace.c [__loongarch__] (TRACEE_REGS_STRUCT): Define. (print_prstatus_regset, print_fpregset) [__loongarch__]: Update expected output.
xen0n
force-pushed
the
loongarch-master
branch
from
351b5fe to
2bf68c3
Compare
|
(For the record: the HEAD before my force-push was 644ac28, and if the missing syscall tampering support is to be called out (as is the case for the linked version of Linux port), the commit message could be recovered from there.) |
There was a problem hiding this comment.
Tested on gcc401.fsffrance.org:
Linux gcc401 5.16.0+ #1266 SMP PREEMPT Thu Jan 13 18:08:10 CST 2022 loongarch64 GNU/Linux
Testsuite summary for strace 5.16.0.5.351b
============================================================================
# TOTAL: 1181
# PASS: 961
# SKIP: 220
# XFAIL: 0
# FAIL: 0
# XPASS: 0
# ERROR: 0
|
I'm sorry that we still have to export orig_a0, because the current solution cannot let ptrace to modify arg0 and return value at the same time. |
Well, full user-space rebuilds are ready, it's only a matter of putting out the new branch and corresponding glibc sources on your part before I can test... |
|
Also I don't think the already merged strace code is affected, but it's nice for you to sync this info with us. 👍 |
|
On Sun, Jan 23, 2022 at 02:49:27AM -0800, Huacai Chen wrote:
I'm sorry that we still have to export orig_a0, because the current solution cannot let ptrace to modify arg0 and return value at the same time.
What ptracers usually do is tampering with the syscall number and/or its
arguments on entering syscall and tampering with the return value on
exiting syscall. Tampering with the return value on entering syscall
is not reliable as it is not supported by all linux architectures.
That is, I don't see why the change is necessary.
|
|
On Sun, Jan 23, 2022 at 02:55:10AM -0800, WÁNG Xuěruì wrote:
Also I don't think the already merged strace code is affected, but it's nice for you to sync this info with us. :+1:
If struct user_pt_regs is changed, then at least its decoder and the test
(src/linux/loongarch64/arch_prstatus_regset.c and tests/ptrace.c)
would have to be updated.
|
Could you please see tools/testing/selftests/seccomp/seccomp_bpf.c in kernel? TRACE_syscall.ptrace.syscall_errno and TRACE_syscall.ptrace.syscall_faked fails with the current solution. |
|
On Sun, Jan 23, 2022 at 05:02:02AM -0800, Huacai Chen wrote:
> On Sun, Jan 23, 2022 at 02:49:27AM -0800, Huacai Chen wrote: I'm sorry that we still have to export orig_a0, because the current solution cannot let ptrace to modify arg0 and return value at the same time.
> What ptracers usually do is tampering with the syscall number and/or its arguments on entering syscall and tampering with the return value on exiting syscall. Tampering with the return value on entering syscall is not reliable as it is not supported by all linux architectures. That is, I don't see why the change is necessary.
Could you please see tools/testing/selftests/seccomp/seccomp_bpf.c in kernel?
TRACE_syscall.ptrace.syscall_errno and TRACE_syscall.ptrace.syscall_faked fails with the current solution.
I see, the fix for ptrace caused a regression in seccomp.
|
Oops, of course; I didn't implement the bits in my first take but the changes are definitely needed. Lucky we still have the chance to do that before the ABI freeze... |
This is based on the WIP Linux port still under review, but the port is
re-using asm-generic syscall numbers and parameters, so breakage should
be minimal when the port is eventually merged, if at all.
Currently only the LP64* ABIs are implemented in the toolchains, so only
support the host type "loongarch64".