Currently, x86, Riscv, Loongarch use the Generic Entry which makes maintainers' work easier and codes more elegant. arm64 has already successfully switched to the Generic IRQ Entry in commit b3cf07851b6c ("arm64: entry: Switch to generic IRQ entry"), it is time to completely convert arm64 to Generic Entry.
The goal is to bring arm64 in line with other architectures that already use the generic entry infrastructure, reducing duplicated code and making it easier to share future changes in entry/exit paths, such as "Syscall User Dispatch".
This patch set is rebased on v6.18-rc6.
The performance benchmarks from perf bench basic syscall on real hardware are below:
| Metric | W/O Generic Framework | With Generic Framework | Change | | ---------- | --------------------- | ---------------------- | ------ | | Total time | 2.813 [sec] | 2.930 [sec] | ↑4% | | usecs/op | 0.281349 | 0.293006 | ↑4% | | ops/sec | 3,554,299 | 3,412,894 | ↓4% |
Compared to earlier with arch specific handling, the performance decreased by approximately 4%.
It was tested ok with following test cases on QEMU virt platform: - Perf tests. - Different `dynamic preempt` mode switch. - Pseudo NMI tests. - Stress-ng CPU stress test. - MTE test case in Documentation/arch/arm64/memory-tagging-extension.rst and all test cases in tools/testing/selftests/arm64/mte/*. - "sud" selftest testcase. - get_syscall_info, peeksiginfo in tools/testing/selftests/ptrace.
The test QEMU configuration is as follows:
qemu-system-aarch64 \ -M virt,gic-version=3,virtualization=on,mte=on \ -cpu max,pauth-impdef=on \ -kernel Image \ -smp 8,sockets=1,cores=4,threads=2 \ -m 512m \ -nographic \ -no-reboot \ -device virtio-rng-pci \ -append "root=/dev/vda rw console=ttyAMA0 kgdboc=ttyAMA0,115200 \ earlycon preempt=voluntary irqchip.gicv3_pseudo_nmi=1" \ -drive if=none,file=images/rootfs.ext4,format=raw,id=hd0 \ -device virtio-blk-device,drive=hd0 \
Chanegs in v7: - Support "Syscall User Dispatch" by implementing arch_syscall_is_vdso_sigreturn() as kemal suggested. - Add aarch64 support for "sud" selftest testcase, which tested ok with the patch series. - Fix the kernel test robot warning for arch_ptrace_report_syscall_entry() and arch_ptrace_report_syscall_exit() in asm/entry-common.h. - Add perf syscall performance test. - Link to v6: https://lore.kernel.org/all/20250916082611.2972008-1-ruanjinjie@huawei.com/
Changes in v6: - Rebased on v6.17-rc5-next as arm64 generic irq entry has merged. - Update the commit message. - Link to v5: https://lore.kernel.org/all/20241206101744.4161990-1-ruanjinjie@huawei.com/
Changes in v5: - Not change arm32 and keep inerrupts_enabled() macro for gicv3 driver. - Move irqentry_state definition into arch/arm64/kernel/entry-common.c. - Avoid removing the __enter_from_*() and __exit_to_*() wrappers. - Update "irqentry_state_t ret/irq_state" to "state" to keep it consistently. - Use generic irq entry header for PREEMPT_DYNAMIC after split the generic entry. - Also refactor the ARM64 syscall code. - Introduce arch_ptrace_report_syscall_entry/exit(), instead of arch_pre/post_report_syscall_entry/exit() to simplify code. - Make the syscall patches clear separation. - Update the commit message. - Link to v4: https://lore.kernel.org/all/20241025100700.3714552-1-ruanjinjie@huawei.com/
Changes in v4: - Rework/cleanup split into a few patches as Mark suggested. - Replace interrupts_enabled() macro with regs_irqs_disabled(), instead of left it here. - Remove rcu and lockdep state in pt_regs by using temporary irqentry_state_t as Mark suggested. - Remove some unnecessary intermediate functions to make it clear. - Rework preempt irq and PREEMPT_DYNAMIC code to make the switch more clear. - arch_prepare_*_entry/exit() -> arch_pre_*_entry/exit(). - Expand the arch functions comment. - Make arch functions closer to its caller. - Declare saved_reg in for block. - Remove arch_exit_to_kernel_mode_prepare(), arch_enter_from_kernel_mode(). - Adjust "Add few arch functions to use generic entry" patch to be the penultimate. - Update the commit message. - Add suggested-by. - Link to v3: https://lore.kernel.org/all/20240629085601.470241-1-ruanjinjie@huawei.com/
Changes in v3: - Test the MTE test cases. - Handle forget_syscall() in arch_post_report_syscall_entry() - Make the arch funcs not use __weak as Thomas suggested, so move the arch funcs to entry-common.h, and make arch_forget_syscall() folded in arch_post_report_syscall_entry() as suggested. - Move report_single_step() to thread_info.h for arm64 - Change __always_inline() to inline, add inline for the other arch funcs. - Remove unused signal.h for entry-common.h. - Add Suggested-by. - Update the commit message.
Changes in v2: - Add tested-by. - Fix a bug that not call arch_post_report_syscall_entry() in syscall_trace_enter() if ptrace_report_syscall_entry() return not zero. - Refactor report_syscall(). - Add comment for arch_prepare_report_syscall_exit(). - Adjust entry-common.h header file inclusion to alphabetical order. - Update the commit message.
Jinjie Ruan (10): arm64/ptrace: Split report_syscall() arm64/ptrace: Refactor syscall_trace_enter/exit() arm64/ptrace: Refator el0_svc_common() entry: Add syscall_exit_to_user_mode_prepare() helper arm64/ptrace: Handle ptrace_report_syscall_entry() error arm64/ptrace: Expand secure_computing() in place arm64/ptrace: Use syscall_get_arguments() heleper entry: Add arch_ptrace_report_syscall_entry/exit() entry: Add has_syscall_work() helper arm64: entry: Convert to generic entry
kemal (1): selftests: sud_test: Support aarch64
arch/arm64/Kconfig | 2 +- arch/arm64/include/asm/entry-common.h | 69 ++++++++++++++ arch/arm64/include/asm/syscall.h | 29 +++++- arch/arm64/include/asm/thread_info.h | 22 +---- arch/arm64/kernel/debug-monitors.c | 7 ++ arch/arm64/kernel/ptrace.c | 90 ------------------- arch/arm64/kernel/signal.c | 2 +- arch/arm64/kernel/syscall.c | 31 ++----- include/linux/entry-common.h | 42 ++++++--- kernel/entry/syscall-common.c | 43 ++++++++- .../syscall_user_dispatch/sud_test.c | 4 + 11 files changed, 188 insertions(+), 153 deletions(-)
The generic syscall entry code has the form:
| syscall_trace_enter() | { | ptrace_report_syscall_entry() | } | | syscall_exit_work() | { | ptrace_report_syscall_exit() | }
In preparation for moving arm64 over to the generic entry code, split report_syscall() to two separate enter and exit functions to align the structure of the arm64 code with syscall_trace_enter() and syscall_exit_work() from the generic entry code.
No functional changes.
Suggested-by: Mark Rutland mark.rutland@arm.com Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- arch/arm64/kernel/ptrace.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 4b001121c72d..5534c175ceb7 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2317,7 +2317,7 @@ enum ptrace_syscall_dir { PTRACE_SYSCALL_EXIT, };
-static void report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir) +static void report_syscall_enter(struct pt_regs *regs) { int regno; unsigned long saved_reg; @@ -2340,13 +2340,24 @@ static void report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir) */ regno = (is_compat_task() ? 12 : 7); saved_reg = regs->regs[regno]; - regs->regs[regno] = dir; + regs->regs[regno] = PTRACE_SYSCALL_ENTER;
- if (dir == PTRACE_SYSCALL_ENTER) { - if (ptrace_report_syscall_entry(regs)) - forget_syscall(regs); - regs->regs[regno] = saved_reg; - } else if (!test_thread_flag(TIF_SINGLESTEP)) { + if (ptrace_report_syscall_entry(regs)) + forget_syscall(regs); + regs->regs[regno] = saved_reg; +} + +static void report_syscall_exit(struct pt_regs *regs) +{ + int regno; + unsigned long saved_reg; + + /* See comment for report_syscall_enter() above */ + regno = (is_compat_task() ? 12 : 7); + saved_reg = regs->regs[regno]; + regs->regs[regno] = PTRACE_SYSCALL_EXIT; + + if (!test_thread_flag(TIF_SINGLESTEP)) { ptrace_report_syscall_exit(regs, 0); regs->regs[regno] = saved_reg; } else { @@ -2366,7 +2377,7 @@ int syscall_trace_enter(struct pt_regs *regs) unsigned long flags = read_thread_flags();
if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { - report_syscall(regs, PTRACE_SYSCALL_ENTER); + report_syscall_enter(regs); if (flags & _TIF_SYSCALL_EMU) return NO_SYSCALL; } @@ -2394,7 +2405,7 @@ void syscall_trace_exit(struct pt_regs *regs) trace_sys_exit(regs, syscall_get_return_value(current, regs));
if (flags & (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP)) - report_syscall(regs, PTRACE_SYSCALL_EXIT); + report_syscall_exit(regs);
rseq_syscall(regs); }
On 17/11/2025 14:30, Jinjie Ruan wrote:
The generic syscall entry code has the form:
| syscall_trace_enter() | { | ptrace_report_syscall_entry() | } | | syscall_exit_work() | { | ptrace_report_syscall_exit() | }
In preparation for moving arm64 over to the generic entry code, split report_syscall() to two separate enter and exit functions to align the structure of the arm64 code with syscall_trace_enter() and syscall_exit_work() from the generic entry code.
No functional changes.
Suggested-by: Mark Rutland mark.rutland@arm.com Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/kernel/ptrace.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 4b001121c72d..5534c175ceb7 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2317,7 +2317,7 @@ enum ptrace_syscall_dir { PTRACE_SYSCALL_EXIT, };
This is now unused so it should be removed.
-static void report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir) +static void report_syscall_enter(struct pt_regs *regs)
Nit: better to call it "report_syscall_entry" to match its final name.
{ int regno; unsigned long saved_reg; @@ -2340,13 +2340,24 @@ static void report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir) */ regno = (is_compat_task() ? 12 : 7); saved_reg = regs->regs[regno];
- regs->regs[regno] = dir;
- regs->regs[regno] = PTRACE_SYSCALL_ENTER;
- if (dir == PTRACE_SYSCALL_ENTER) {
if (ptrace_report_syscall_entry(regs))forget_syscall(regs);regs->regs[regno] = saved_reg;- } else if (!test_thread_flag(TIF_SINGLESTEP)) {
- if (ptrace_report_syscall_entry(regs))
forget_syscall(regs);- regs->regs[regno] = saved_reg;
+}
+static void report_syscall_exit(struct pt_regs *regs) +{
- int regno;
- unsigned long saved_reg;
- /* See comment for report_syscall_enter() above */
- regno = (is_compat_task() ? 12 : 7);
Probably best not to duplicate such magic numbers, moving this line to a helper would be good.
- Kevin
- saved_reg = regs->regs[regno];
- regs->regs[regno] = PTRACE_SYSCALL_EXIT;
- if (!test_thread_flag(TIF_SINGLESTEP)) { ptrace_report_syscall_exit(regs, 0); regs->regs[regno] = saved_reg; } else {
@@ -2366,7 +2377,7 @@ int syscall_trace_enter(struct pt_regs *regs) unsigned long flags = read_thread_flags(); if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) {
report_syscall(regs, PTRACE_SYSCALL_ENTER);
if (flags & _TIF_SYSCALL_EMU) return NO_SYSCALL; }report_syscall_enter(regs);@@ -2394,7 +2405,7 @@ void syscall_trace_exit(struct pt_regs *regs) trace_sys_exit(regs, syscall_get_return_value(current, regs)); if (flags & (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP))
report_syscall(regs, PTRACE_SYSCALL_EXIT);
report_syscall_exit(regs);rseq_syscall(regs); }
On 2025/11/19 1:09, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
The generic syscall entry code has the form:
| syscall_trace_enter() | { | ptrace_report_syscall_entry() | } | | syscall_exit_work() | { | ptrace_report_syscall_exit() | }
In preparation for moving arm64 over to the generic entry code, split report_syscall() to two separate enter and exit functions to align the structure of the arm64 code with syscall_trace_enter() and syscall_exit_work() from the generic entry code.
No functional changes.
Suggested-by: Mark Rutland mark.rutland@arm.com Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/kernel/ptrace.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 4b001121c72d..5534c175ceb7 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2317,7 +2317,7 @@ enum ptrace_syscall_dir { PTRACE_SYSCALL_EXIT, };
This is now unused so it should be removed.
Sure.
-static void report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir) +static void report_syscall_enter(struct pt_regs *regs)
Nit: better to call it "report_syscall_entry" to match its final name.
That makes sense, thanks for the review.
{ int regno; unsigned long saved_reg; @@ -2340,13 +2340,24 @@ static void report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir) */ regno = (is_compat_task() ? 12 : 7); saved_reg = regs->regs[regno];
- regs->regs[regno] = dir;
- regs->regs[regno] = PTRACE_SYSCALL_ENTER;
- if (dir == PTRACE_SYSCALL_ENTER) {
if (ptrace_report_syscall_entry(regs))forget_syscall(regs);regs->regs[regno] = saved_reg;- } else if (!test_thread_flag(TIF_SINGLESTEP)) {
- if (ptrace_report_syscall_entry(regs))
forget_syscall(regs);- regs->regs[regno] = saved_reg;
+}
+static void report_syscall_exit(struct pt_regs *regs) +{
- int regno;
- unsigned long saved_reg;
- /* See comment for report_syscall_enter() above */
- regno = (is_compat_task() ? 12 : 7);
Probably best not to duplicate such magic numbers, moving this line to a helper would be good.
That makes sense. A helper will remove the duplicate.
- Kevin
- saved_reg = regs->regs[regno];
- regs->regs[regno] = PTRACE_SYSCALL_EXIT;
- if (!test_thread_flag(TIF_SINGLESTEP)) { ptrace_report_syscall_exit(regs, 0); regs->regs[regno] = saved_reg; } else {
@@ -2366,7 +2377,7 @@ int syscall_trace_enter(struct pt_regs *regs) unsigned long flags = read_thread_flags(); if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) {
report_syscall(regs, PTRACE_SYSCALL_ENTER);
if (flags & _TIF_SYSCALL_EMU) return NO_SYSCALL; }report_syscall_enter(regs);@@ -2394,7 +2405,7 @@ void syscall_trace_exit(struct pt_regs *regs) trace_sys_exit(regs, syscall_get_return_value(current, regs)); if (flags & (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP))
report_syscall(regs, PTRACE_SYSCALL_EXIT);
report_syscall_exit(regs);rseq_syscall(regs); }
The generic syscall entry code has the following form, which use the input syscall work flag and syscall number:
| syscall_trace_enter(struct pt_regs *regs, long syscall, | unsigned long work) | | syscall_exit_work(struct pt_regs *regs, unsigned long work)
In preparation for moving arm64 over to the generic entry code, refactor syscall_trace_enter/exit() to also pass thread flags, and get syscall number by syscall_get_nr() helper.
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- arch/arm64/include/asm/syscall.h | 4 ++-- arch/arm64/kernel/ptrace.c | 24 +++++++++++++++--------- arch/arm64/kernel/syscall.c | 5 +++-- 3 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h index 712daa90e643..d69f590a989b 100644 --- a/arch/arm64/include/asm/syscall.h +++ b/arch/arm64/include/asm/syscall.h @@ -114,7 +114,7 @@ static inline int syscall_get_arch(struct task_struct *task) return AUDIT_ARCH_AARCH64; }
-int syscall_trace_enter(struct pt_regs *regs); -void syscall_trace_exit(struct pt_regs *regs); +int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags); +void syscall_trace_exit(struct pt_regs *regs, unsigned long flags);
#endif /* __ASM_SYSCALL_H */ diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 5534c175ceb7..bbb868f6b292 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2372,10 +2372,8 @@ static void report_syscall_exit(struct pt_regs *regs) } }
-int syscall_trace_enter(struct pt_regs *regs) +int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) { - unsigned long flags = read_thread_flags(); - if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { report_syscall_enter(regs); if (flags & _TIF_SYSCALL_EMU) @@ -2386,19 +2384,27 @@ int syscall_trace_enter(struct pt_regs *regs) if (secure_computing() == -1) return NO_SYSCALL;
- if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) - trace_sys_enter(regs, regs->syscallno); + /* Either of the above might have changed the syscall number */ + syscall = syscall_get_nr(current, regs); + + if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) { + trace_sys_enter(regs, syscall);
- audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1], + /* + * Probes or BPF hooks in the tracepoint may have changed the + * system call number as well. + */ + syscall = syscall_get_nr(current, regs); + } + + audit_syscall_entry(syscall, regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]);
return regs->syscallno; }
-void syscall_trace_exit(struct pt_regs *regs) +void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) { - unsigned long flags = read_thread_flags(); - audit_syscall_exit(regs);
if (flags & _TIF_SYSCALL_TRACEPOINT) diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index aba7ca6bca2d..6e3fe760e0bb 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -124,7 +124,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, */ if (scno == NO_SYSCALL) syscall_set_return_value(current, regs, -ENOSYS, 0); - scno = syscall_trace_enter(regs); + scno = syscall_trace_enter(regs, regs->syscallno, flags); if (scno == NO_SYSCALL) goto trace_exit; } @@ -143,7 +143,8 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, }
trace_exit: - syscall_trace_exit(regs); + flags = read_thread_flags(); + syscall_trace_exit(regs, flags); }
void do_el0_svc(struct pt_regs *regs)
On 17/11/2025 14:30, Jinjie Ruan wrote:
[...]
@@ -2386,19 +2384,27 @@ int syscall_trace_enter(struct pt_regs *regs) if (secure_computing() == -1) return NO_SYSCALL;
- if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
trace_sys_enter(regs, regs->syscallno);
- /* Either of the above might have changed the syscall number */
- syscall = syscall_get_nr(current, regs);
- if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) {
trace_sys_enter(regs, syscall);
- audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1],
/** Probes or BPF hooks in the tracepoint may have changed the* system call number as well.*/syscall = syscall_get_nr(current, regs);- }
- audit_syscall_entry(syscall, regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]);
return regs->syscallno;
It would be good to align the return with the generic version as well.
} -void syscall_trace_exit(struct pt_regs *regs) +void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) {
- unsigned long flags = read_thread_flags();
- audit_syscall_exit(regs);
if (flags & _TIF_SYSCALL_TRACEPOINT) diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index aba7ca6bca2d..6e3fe760e0bb 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -124,7 +124,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, */ if (scno == NO_SYSCALL) syscall_set_return_value(current, regs, -ENOSYS, 0);
scno = syscall_trace_enter(regs);
scno = syscall_trace_enter(regs, regs->syscallno, flags);
Nit: could use scno instead of regs->syscallno.
- Kevin
if (scno == NO_SYSCALL) goto trace_exit;} @@ -143,7 +143,8 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, } trace_exit:
- syscall_trace_exit(regs);
- flags = read_thread_flags();
- syscall_trace_exit(regs, flags);
} void do_el0_svc(struct pt_regs *regs)
On 2025/11/19 1:09, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
[...]
@@ -2386,19 +2384,27 @@ int syscall_trace_enter(struct pt_regs *regs) if (secure_computing() == -1) return NO_SYSCALL;
- if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
trace_sys_enter(regs, regs->syscallno);
- /* Either of the above might have changed the syscall number */
- syscall = syscall_get_nr(current, regs);
- if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) {
trace_sys_enter(regs, syscall);
- audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1],
/** Probes or BPF hooks in the tracepoint may have changed the* system call number as well.*/syscall = syscall_get_nr(current, regs);- }
- audit_syscall_entry(syscall, regs->orig_x0, regs->regs[1], regs->regs[2], regs->regs[3]);
return regs->syscallno;
It would be good to align the return with the generic version as well.
Right, could be aligned with the return with the generic version.
} -void syscall_trace_exit(struct pt_regs *regs) +void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) {
- unsigned long flags = read_thread_flags();
- audit_syscall_exit(regs);
if (flags & _TIF_SYSCALL_TRACEPOINT) diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index aba7ca6bca2d..6e3fe760e0bb 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -124,7 +124,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, */ if (scno == NO_SYSCALL) syscall_set_return_value(current, regs, -ENOSYS, 0);
scno = syscall_trace_enter(regs);
scno = syscall_trace_enter(regs, regs->syscallno, flags);Nit: could use scno instead of regs->syscallno.
That is ok.
- Kevin
if (scno == NO_SYSCALL) goto trace_exit;} @@ -143,7 +143,8 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, } trace_exit:
- syscall_trace_exit(regs);
- flags = read_thread_flags();
- syscall_trace_exit(regs, flags);
} void do_el0_svc(struct pt_regs *regs)
Compared to the generic entry code, arm64 terminate the process after report_syscall_exit() if the syscall is issued within a restartable sequence.
In preparation for moving arm64 over to the generic entry code, refactor el0_svc_common() as below:
- Extract syscall_exit_to_user_mode_prepare() to replace the the combination of read_thread_flags() and syscall_trace_exit(), also move the syscall exit check logic into it.
- Move rseq_syscall() ahead, so the CONFIG_DEBUG_RSEQ check is not needed.
- Move has_syscall_work() helper into asm/syscall.h, which will be reused by ptrace.c.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- arch/arm64/include/asm/syscall.h | 7 ++++++- arch/arm64/kernel/ptrace.c | 10 +++++++++- arch/arm64/kernel/syscall.c | 26 +++++--------------------- 3 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h index d69f590a989b..6225981fbbdb 100644 --- a/arch/arm64/include/asm/syscall.h +++ b/arch/arm64/include/asm/syscall.h @@ -114,7 +114,12 @@ static inline int syscall_get_arch(struct task_struct *task) return AUDIT_ARCH_AARCH64; }
+static inline bool has_syscall_work(unsigned long flags) +{ + return unlikely(flags & _TIF_SYSCALL_WORK); +} + int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags); -void syscall_trace_exit(struct pt_regs *regs, unsigned long flags); +void syscall_exit_to_user_mode_prepare(struct pt_regs *regs);
#endif /* __ASM_SYSCALL_H */ diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index bbb868f6b292..95984bbf53db 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2403,7 +2403,7 @@ int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) return regs->syscallno; }
-void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) +static void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) { audit_syscall_exit(regs);
@@ -2412,8 +2412,16 @@ void syscall_trace_exit(struct pt_regs *regs, unsigned long flags)
if (flags & (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP)) report_syscall_exit(regs); +} + +void syscall_exit_to_user_mode_prepare(struct pt_regs *regs) +{ + unsigned long flags = read_thread_flags();
rseq_syscall(regs); + + if (has_syscall_work(flags) || flags & _TIF_SINGLESTEP) + syscall_trace_exit(regs, flags); }
/* diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index 6e3fe760e0bb..9713b038d750 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -65,11 +65,6 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno, choose_random_kstack_offset(get_random_u16()); }
-static inline bool has_syscall_work(unsigned long flags) -{ - return unlikely(flags & _TIF_SYSCALL_WORK); -} - static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, const syscall_fn_t syscall_table[]) { @@ -125,26 +120,15 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, if (scno == NO_SYSCALL) syscall_set_return_value(current, regs, -ENOSYS, 0); scno = syscall_trace_enter(regs, regs->syscallno, flags); - if (scno == NO_SYSCALL) - goto trace_exit; + if (scno == NO_SYSCALL) { + syscall_exit_to_user_mode_prepare(regs); + return; + } }
invoke_syscall(regs, scno, sc_nr, syscall_table);
- /* - * The tracing status may have changed under our feet, so we have to - * check again. However, if we were tracing entry, then we always trace - * exit regardless, as the old entry assembly did. - */ - if (!has_syscall_work(flags) && !IS_ENABLED(CONFIG_DEBUG_RSEQ)) { - flags = read_thread_flags(); - if (!has_syscall_work(flags) && !(flags & _TIF_SINGLESTEP)) - return; - } - -trace_exit: - flags = read_thread_flags(); - syscall_trace_exit(regs, flags); + syscall_exit_to_user_mode_prepare(regs); }
void do_el0_svc(struct pt_regs *regs)
In subject: s/refator/refactor/
Also nit: this is less about ptrace than about syscall entry, so maybe "arm64/syscall:"?
On 17/11/2025 14:30, Jinjie Ruan wrote:
Compared to the generic entry code, arm64 terminate the process
s/terminate/terminates/ (similar observation for other commit messages)
after report_syscall_exit() if the syscall is issued within a restartable sequence.
Presumably this implies that forcing SIGSEGV before or after calling the tracing handlers makes no difference, if so it should be clarified.
In preparation for moving arm64 over to the generic entry code, refactor el0_svc_common() as below:
Extract syscall_exit_to_user_mode_prepare() to replace the the combination of read_thread_flags() and syscall_trace_exit(), also move the syscall exit check logic into it.
Move rseq_syscall() ahead, so the CONFIG_DEBUG_RSEQ check is not needed.
Move has_syscall_work() helper into asm/syscall.h, which will be reused by ptrace.c.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/include/asm/syscall.h | 7 ++++++- arch/arm64/kernel/ptrace.c | 10 +++++++++- arch/arm64/kernel/syscall.c | 26 +++++--------------------- 3 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h index d69f590a989b..6225981fbbdb 100644 --- a/arch/arm64/include/asm/syscall.h +++ b/arch/arm64/include/asm/syscall.h @@ -114,7 +114,12 @@ static inline int syscall_get_arch(struct task_struct *task) return AUDIT_ARCH_AARCH64; } +static inline bool has_syscall_work(unsigned long flags) +{
- return unlikely(flags & _TIF_SYSCALL_WORK);
+}
int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags); -void syscall_trace_exit(struct pt_regs *regs, unsigned long flags); +void syscall_exit_to_user_mode_prepare(struct pt_regs *regs); #endif /* __ASM_SYSCALL_H */ diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index bbb868f6b292..95984bbf53db 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2403,7 +2403,7 @@ int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) return regs->syscallno; } -void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) +static void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) { audit_syscall_exit(regs); @@ -2412,8 +2412,16 @@ void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) if (flags & (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP)) report_syscall_exit(regs); +}
+void syscall_exit_to_user_mode_prepare(struct pt_regs *regs) +{
- unsigned long flags = read_thread_flags();
rseq_syscall(regs);
- if (has_syscall_work(flags) || flags & _TIF_SINGLESTEP)
syscall_trace_exit(regs, flags);
Since we're trying to align as much with the generic code as possible, we might as well rename this function to syscall_exit_work() as well.
} /* diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index 6e3fe760e0bb..9713b038d750 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -65,11 +65,6 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno, choose_random_kstack_offset(get_random_u16()); } -static inline bool has_syscall_work(unsigned long flags) -{
- return unlikely(flags & _TIF_SYSCALL_WORK);
-}
static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, const syscall_fn_t syscall_table[]) { @@ -125,26 +120,15 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, if (scno == NO_SYSCALL) syscall_set_return_value(current, regs, -ENOSYS, 0); scno = syscall_trace_enter(regs, regs->syscallno, flags);
if (scno == NO_SYSCALL)goto trace_exit;
Why not keep the goto instead of duplicating the call? It could be renamed if necessary.
- Kevin
if (scno == NO_SYSCALL) {syscall_exit_to_user_mode_prepare(regs);return; }}invoke_syscall(regs, scno, sc_nr, syscall_table);
- /*
* The tracing status may have changed under our feet, so we have to* check again. However, if we were tracing entry, then we always trace* exit regardless, as the old entry assembly did.*/- if (!has_syscall_work(flags) && !IS_ENABLED(CONFIG_DEBUG_RSEQ)) {
flags = read_thread_flags();if (!has_syscall_work(flags) && !(flags & _TIF_SINGLESTEP))return;- }
-trace_exit:
- flags = read_thread_flags();
- syscall_trace_exit(regs, flags);
- syscall_exit_to_user_mode_prepare(regs);
} void do_el0_svc(struct pt_regs *regs)
On 2025/11/19 1:10, Kevin Brodsky wrote:
In subject: s/refator/refactor/
Also nit: this is less about ptrace than about syscall entry, so maybe "arm64/syscall:"?
On 17/11/2025 14:30, Jinjie Ruan wrote:
Compared to the generic entry code, arm64 terminate the process
s/terminate/terminates/ (similar observation for other commit messages)
Will update it.
after report_syscall_exit() if the syscall is issued within a restartable sequence.
Presumably this implies that forcing SIGSEGV before or after calling the tracing handlers makes no difference, if so it should be clarified.
It seems that it need a separate patch to clarify.
In preparation for moving arm64 over to the generic entry code, refactor el0_svc_common() as below:
Extract syscall_exit_to_user_mode_prepare() to replace the the combination of read_thread_flags() and syscall_trace_exit(), also move the syscall exit check logic into it.
Move rseq_syscall() ahead, so the CONFIG_DEBUG_RSEQ check is not needed.
Move has_syscall_work() helper into asm/syscall.h, which will be reused by ptrace.c.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/include/asm/syscall.h | 7 ++++++- arch/arm64/kernel/ptrace.c | 10 +++++++++- arch/arm64/kernel/syscall.c | 26 +++++--------------------- 3 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h index d69f590a989b..6225981fbbdb 100644 --- a/arch/arm64/include/asm/syscall.h +++ b/arch/arm64/include/asm/syscall.h @@ -114,7 +114,12 @@ static inline int syscall_get_arch(struct task_struct *task) return AUDIT_ARCH_AARCH64; } +static inline bool has_syscall_work(unsigned long flags) +{
- return unlikely(flags & _TIF_SYSCALL_WORK);
+}
int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags); -void syscall_trace_exit(struct pt_regs *regs, unsigned long flags); +void syscall_exit_to_user_mode_prepare(struct pt_regs *regs); #endif /* __ASM_SYSCALL_H */ diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index bbb868f6b292..95984bbf53db 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2403,7 +2403,7 @@ int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) return regs->syscallno; } -void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) +static void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) { audit_syscall_exit(regs); @@ -2412,8 +2412,16 @@ void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) if (flags & (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP)) report_syscall_exit(regs); +}
+void syscall_exit_to_user_mode_prepare(struct pt_regs *regs) +{
- unsigned long flags = read_thread_flags();
rseq_syscall(regs);
- if (has_syscall_work(flags) || flags & _TIF_SINGLESTEP)
syscall_trace_exit(regs, flags);Since we're trying to align as much with the generic code as possible, we might as well rename this function to syscall_exit_work() as well.
Yes, syscall_trace_exit() is very similar to syscall_exit_work().
} /* diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index 6e3fe760e0bb..9713b038d750 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -65,11 +65,6 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno, choose_random_kstack_offset(get_random_u16()); } -static inline bool has_syscall_work(unsigned long flags) -{
- return unlikely(flags & _TIF_SYSCALL_WORK);
-}
static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, const syscall_fn_t syscall_table[]) { @@ -125,26 +120,15 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, if (scno == NO_SYSCALL) syscall_set_return_value(current, regs, -ENOSYS, 0); scno = syscall_trace_enter(regs, regs->syscallno, flags);
if (scno == NO_SYSCALL)goto trace_exit;Why not keep the goto instead of duplicating the call? It could be renamed if necessary.
That is good.
- Kevin
if (scno == NO_SYSCALL) {syscall_exit_to_user_mode_prepare(regs);return; }}invoke_syscall(regs, scno, sc_nr, syscall_table);
- /*
* The tracing status may have changed under our feet, so we have to* check again. However, if we were tracing entry, then we always trace* exit regardless, as the old entry assembly did.*/- if (!has_syscall_work(flags) && !IS_ENABLED(CONFIG_DEBUG_RSEQ)) {
flags = read_thread_flags();if (!has_syscall_work(flags) && !(flags & _TIF_SINGLESTEP))return;- }
-trace_exit:
- flags = read_thread_flags();
- syscall_trace_exit(regs, flags);
- syscall_exit_to_user_mode_prepare(regs);
} void do_el0_svc(struct pt_regs *regs)
In the generic entry code, the part before syscall_exit_to_user_mode_work() calls syscall_exit_work(), which serves the same purpose as syscall_exit_to_user_mode_prepare() in arm64.
In preparation for moving arm64 over to the generic entry code, extract syscall_exit_to_user_mode_prepare() helper from syscall_exit_to_user_mode_work().
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- include/linux/entry-common.h | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h index 7177436f0f9e..cd6dacb2d8bf 100644 --- a/include/linux/entry-common.h +++ b/include/linux/entry-common.h @@ -137,20 +137,11 @@ static __always_inline long syscall_enter_from_user_mode(struct pt_regs *regs, l */ void syscall_exit_work(struct pt_regs *regs, unsigned long work);
-/** - * syscall_exit_to_user_mode_work - Handle work before returning to user mode - * @regs: Pointer to currents pt_regs - * - * Same as step 1 and 2 of syscall_exit_to_user_mode() but without calling - * exit_to_user_mode() to perform the final transition to user mode. - * - * Calling convention is the same as for syscall_exit_to_user_mode() and it - * returns with all work handled and interrupts disabled. The caller must - * invoke exit_to_user_mode() before actually switching to user mode to - * make the final state transitions. Interrupts must stay disabled between - * return from this function and the invocation of exit_to_user_mode(). +/* + * Syscall specific exit to user mode preparation. Runs with interrupts + * enabled. */ -static __always_inline void syscall_exit_to_user_mode_work(struct pt_regs *regs) +static __always_inline void syscall_exit_to_user_mode_prepare(struct pt_regs *regs) { unsigned long work = READ_ONCE(current_thread_info()->syscall_work); unsigned long nr = syscall_get_nr(current, regs); @@ -171,6 +162,24 @@ static __always_inline void syscall_exit_to_user_mode_work(struct pt_regs *regs) */ if (unlikely(work & SYSCALL_WORK_EXIT)) syscall_exit_work(regs, work); +} + +/** + * syscall_exit_to_user_mode_work - Handle work before returning to user mode + * @regs: Pointer to currents pt_regs + * + * Same as step 1 and 2 of syscall_exit_to_user_mode() but without calling + * exit_to_user_mode() to perform the final transition to user mode. + * + * Calling convention is the same as for syscall_exit_to_user_mode() and it + * returns with all work handled and interrupts disabled. The caller must + * invoke exit_to_user_mode() before actually switching to user mode to + * make the final state transitions. Interrupts must stay disabled between + * return from this function and the invocation of exit_to_user_mode(). + */ +static __always_inline void syscall_exit_to_user_mode_work(struct pt_regs *regs) +{ + syscall_exit_to_user_mode_prepare(regs); local_irq_disable_exit_to_user(); exit_to_user_mode_prepare(regs); }
On Mon, Nov 17 2025 at 21:30, Jinjie Ruan wrote:
In the generic entry code, the part before syscall_exit_to_user_mode_work() calls syscall_exit_work(), which serves the same purpose as syscall_exit_to_user_mode_prepare() in arm64.
In preparation for moving arm64 over to the generic entry code, extract syscall_exit_to_user_mode_prepare() helper from syscall_exit_to_user_mode_work().
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
Reviewed-by: Thomas Gleixner tglx@linutronix.de
On 17/11/2025 14:30, Jinjie Ruan wrote:
In the generic entry code, the part before syscall_exit_to_user_mode_work() calls syscall_exit_work(), which serves the same purpose as syscall_exit_to_user_mode_prepare() in arm64.
This is really hard to parse, I suppose the point is that the beginning of syscall_exit_to_user_mode_work() can be reused on arm64 and therefore it makes sense to split it?
In preparation for moving arm64 over to the generic entry code, extract syscall_exit_to_user_mode_prepare() helper from syscall_exit_to_user_mode_work().
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
The position of this patch in the series seems a little arbitrary (it has no dependency on other patches), it might make more sense to group it with the other generic patches (i.e. move it to patch 7).
Otherwise:
Reviewed-by: Kevin Brodsky kevin.brodsky@arm.com
- Kevin
include/linux/entry-common.h | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h index 7177436f0f9e..cd6dacb2d8bf 100644 --- a/include/linux/entry-common.h +++ b/include/linux/entry-common.h @@ -137,20 +137,11 @@ static __always_inline long syscall_enter_from_user_mode(struct pt_regs *regs, l */ void syscall_exit_work(struct pt_regs *regs, unsigned long work); -/**
- syscall_exit_to_user_mode_work - Handle work before returning to user mode
- @regs: Pointer to currents pt_regs
- Same as step 1 and 2 of syscall_exit_to_user_mode() but without calling
- exit_to_user_mode() to perform the final transition to user mode.
- Calling convention is the same as for syscall_exit_to_user_mode() and it
- returns with all work handled and interrupts disabled. The caller must
- invoke exit_to_user_mode() before actually switching to user mode to
- make the final state transitions. Interrupts must stay disabled between
- return from this function and the invocation of exit_to_user_mode().
+/*
- Syscall specific exit to user mode preparation. Runs with interrupts
*/
- enabled.
-static __always_inline void syscall_exit_to_user_mode_work(struct pt_regs *regs) +static __always_inline void syscall_exit_to_user_mode_prepare(struct pt_regs *regs) { unsigned long work = READ_ONCE(current_thread_info()->syscall_work); unsigned long nr = syscall_get_nr(current, regs); @@ -171,6 +162,24 @@ static __always_inline void syscall_exit_to_user_mode_work(struct pt_regs *regs) */ if (unlikely(work & SYSCALL_WORK_EXIT)) syscall_exit_work(regs, work); +}
+/**
- syscall_exit_to_user_mode_work - Handle work before returning to user mode
- @regs: Pointer to currents pt_regs
- Same as step 1 and 2 of syscall_exit_to_user_mode() but without calling
- exit_to_user_mode() to perform the final transition to user mode.
- Calling convention is the same as for syscall_exit_to_user_mode() and it
- returns with all work handled and interrupts disabled. The caller must
- invoke exit_to_user_mode() before actually switching to user mode to
- make the final state transitions. Interrupts must stay disabled between
- return from this function and the invocation of exit_to_user_mode().
- */
+static __always_inline void syscall_exit_to_user_mode_work(struct pt_regs *regs) +{
- syscall_exit_to_user_mode_prepare(regs); local_irq_disable_exit_to_user(); exit_to_user_mode_prepare(regs);
}
The generic entry handle error of ptrace_report_syscall_entry(), but arm64 not.
As the comment said, the calling arch code should abort the system call and must prevent normal entry so no system call is made if ptrace_report_syscall_entry() return nonzero.
In preparation for moving arm64 over to the generic entry code, return early if ptrace_report_syscall_entry() encounters an error.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- arch/arm64/kernel/ptrace.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 95984bbf53db..707951ad5d24 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2317,10 +2317,10 @@ enum ptrace_syscall_dir { PTRACE_SYSCALL_EXIT, };
-static void report_syscall_enter(struct pt_regs *regs) +static int report_syscall_enter(struct pt_regs *regs) { - int regno; unsigned long saved_reg; + int regno, ret;
/* * We have some ABI weirdness here in the way that we handle syscall @@ -2342,9 +2342,13 @@ static void report_syscall_enter(struct pt_regs *regs) saved_reg = regs->regs[regno]; regs->regs[regno] = PTRACE_SYSCALL_ENTER;
- if (ptrace_report_syscall_entry(regs)) + ret = ptrace_report_syscall_entry(regs); + if (ret) forget_syscall(regs); + regs->regs[regno] = saved_reg; + + return ret; }
static void report_syscall_exit(struct pt_regs *regs) @@ -2374,9 +2378,11 @@ static void report_syscall_exit(struct pt_regs *regs)
int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) { + int ret; + if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { - report_syscall_enter(regs); - if (flags & _TIF_SYSCALL_EMU) + ret = report_syscall_enter(regs); + if (ret || (flags & _TIF_SYSCALL_EMU)) return NO_SYSCALL; }
On 17/11/2025 14:30, Jinjie Ruan wrote:
The generic entry handle error of ptrace_report_syscall_entry(), but arm64 not.
This suggests that arm64 ignores the error completely, which isn't the case: no syscall will be performed, but tracing will still occur as normal.
What this patch seems to be doing is to abort the _enter sequence if ptrace_report_syscall_entry() errors out. The commit title and message should be reworded accordingly.
As the comment said, the calling arch code should abort the system
Which comment?
call and must prevent normal entry so no system call is made if ptrace_report_syscall_entry() return nonzero.
This is already the case since we're calling forget_syscall().
In preparation for moving arm64 over to the generic entry code, return early if ptrace_report_syscall_entry() encounters an error.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/kernel/ptrace.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 95984bbf53db..707951ad5d24 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2317,10 +2317,10 @@ enum ptrace_syscall_dir { PTRACE_SYSCALL_EXIT, }; -static void report_syscall_enter(struct pt_regs *regs) +static int report_syscall_enter(struct pt_regs *regs) {
- int regno; unsigned long saved_reg;
- int regno, ret;
/* * We have some ABI weirdness here in the way that we handle syscall @@ -2342,9 +2342,13 @@ static void report_syscall_enter(struct pt_regs *regs) saved_reg = regs->regs[regno]; regs->regs[regno] = PTRACE_SYSCALL_ENTER;
- if (ptrace_report_syscall_entry(regs))
- ret = ptrace_report_syscall_entry(regs);
- if (ret) forget_syscall(regs);
The generic syscall_trace_enter() doesn't do this (i.e. setting regs->syscallno to NO_SYSCALL). Is that an oversight or do we just not need it? In principle this does have a visible effect (e.g. via REGSET_SYSTEM_CALL).
- Kevin
- regs->regs[regno] = saved_reg;
- return ret;
} static void report_syscall_exit(struct pt_regs *regs) @@ -2374,9 +2378,11 @@ static void report_syscall_exit(struct pt_regs *regs) int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) {
- int ret;
- if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) {
report_syscall_enter(regs);if (flags & _TIF_SYSCALL_EMU)
ret = report_syscall_enter(regs); }if (ret || (flags & _TIF_SYSCALL_EMU)) return NO_SYSCALL;
On 2025/11/19 1:12, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
The generic entry handle error of ptrace_report_syscall_entry(), but arm64 not.
This suggests that arm64 ignores the error completely, which isn't the case: no syscall will be performed, but tracing will still occur as normal.
What this patch seems to be doing is to abort the _enter sequence if ptrace_report_syscall_entry() errors out. The commit title and message should be reworded accordingly.
You are right,the description is unclear .
As the comment said, the calling arch code should abort the system
Which comment?
ptrace_report_syscall_entry()
call and must prevent normal entry so no system call is made if ptrace_report_syscall_entry() return nonzero.
This is already the case since we're calling forget_syscall().
Yes. it is similar with the generic entry returns NO_SYSCALL.
In preparation for moving arm64 over to the generic entry code, return early if ptrace_report_syscall_entry() encounters an error.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/kernel/ptrace.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 95984bbf53db..707951ad5d24 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2317,10 +2317,10 @@ enum ptrace_syscall_dir { PTRACE_SYSCALL_EXIT, }; -static void report_syscall_enter(struct pt_regs *regs) +static int report_syscall_enter(struct pt_regs *regs) {
- int regno; unsigned long saved_reg;
- int regno, ret;
/* * We have some ABI weirdness here in the way that we handle syscall @@ -2342,9 +2342,13 @@ static void report_syscall_enter(struct pt_regs *regs) saved_reg = regs->regs[regno]; regs->regs[regno] = PTRACE_SYSCALL_ENTER;
- if (ptrace_report_syscall_entry(regs))
- ret = ptrace_report_syscall_entry(regs);
- if (ret) forget_syscall(regs);
The generic syscall_trace_enter() doesn't do this (i.e. setting regs->syscallno to NO_SYSCALL). Is that an oversight or do we just not need it? In principle this does have a visible effect (e.g. via REGSET_SYSTEM_CALL).
We just not need it because the original syscall_trace_enter() need use regs->syscallno as the return value, but now we return early by using NO_SYSCALL.
- Kevin
- regs->regs[regno] = saved_reg;
- return ret;
} static void report_syscall_exit(struct pt_regs *regs) @@ -2374,9 +2378,11 @@ static void report_syscall_exit(struct pt_regs *regs) int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) {
- int ret;
- if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) {
report_syscall_enter(regs);if (flags & _TIF_SYSCALL_EMU)
ret = report_syscall_enter(regs); }if (ret || (flags & _TIF_SYSCALL_EMU)) return NO_SYSCALL;
The generic entry expand secure_computing() in place and call __secure_computing() directly.
In order to switch to the generic entry for arm64, refactor secure_computing() for syscall_trace_enter().
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- arch/arm64/kernel/ptrace.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 707951ad5d24..9af3046a2ce9 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2387,8 +2387,11 @@ int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) }
/* Do the secure computing after ptrace; failures should be fast. */ - if (secure_computing() == -1) - return NO_SYSCALL; + if (flags & _TIF_SECCOMP) { + ret = __secure_computing(NULL); + if (ret == -1L) + return NO_SYSCALL; + }
/* Either of the above might have changed the syscall number */ syscall = syscall_get_nr(current, regs);
On 17/11/2025 14:30, Jinjie Ruan wrote:
The generic entry expand secure_computing() in place and call __secure_computing() directly.
In order to switch to the generic entry for arm64, refactor secure_computing() for syscall_trace_enter().
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/kernel/ptrace.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 707951ad5d24..9af3046a2ce9 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2387,8 +2387,11 @@ int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) } /* Do the secure computing after ptrace; failures should be fast. */
- if (secure_computing() == -1)
return NO_SYSCALL;
- if (flags & _TIF_SECCOMP) {
ret = __secure_computing(NULL);
No argument was passed to __secure_computing() in v6, as expected since it was removed in v6.15. Not sure why this NULL reappeared.
if (ret == -1L)
ret is an int, so should be -1, not -1L.
- Kevin
return NO_SYSCALL;- }
/* Either of the above might have changed the syscall number */ syscall = syscall_get_nr(current, regs);
On 2025/11/19 1:12, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
The generic entry expand secure_computing() in place and call __secure_computing() directly.
In order to switch to the generic entry for arm64, refactor secure_computing() for syscall_trace_enter().
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/kernel/ptrace.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 707951ad5d24..9af3046a2ce9 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2387,8 +2387,11 @@ int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) } /* Do the secure computing after ptrace; failures should be fast. */
- if (secure_computing() == -1)
return NO_SYSCALL;
- if (flags & _TIF_SECCOMP) {
ret = __secure_computing(NULL);No argument was passed to __secure_computing() in v6, as expected since it was removed in v6.15. Not sure why this NULL reappeared.
There is a typo here.
if (ret == -1L)ret is an int, so should be -1, not -1L.
Right, not -1L.
- Kevin
return NO_SYSCALL;- }
/* Either of the above might have changed the syscall number */ syscall = syscall_get_nr(current, regs);
The generic entry check audit context first and use syscall_get_arguments() helper.
In order to switch to the generic entry for arm64,
- Also use the helper.
- Extract the syscall_enter_audit() helper to make it clear.
- Check audit context for syscall_enter_audit(), which only adds one additional check without any other differences as audit_syscall_entry() check it first otherwise do nothing.
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- arch/arm64/kernel/ptrace.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 9af3046a2ce9..9455ccf1ecc1 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2376,6 +2376,17 @@ static void report_syscall_exit(struct pt_regs *regs) } }
+static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) +{ + if (unlikely(audit_context())) { + unsigned long args[6]; + + syscall_get_arguments(current, regs, args); + audit_syscall_entry(syscall, args[0], args[1], args[2], args[3]); + } + +} + int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) { int ret; @@ -2406,8 +2417,7 @@ int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) syscall = syscall_get_nr(current, regs); }
- audit_syscall_entry(syscall, regs->orig_x0, regs->regs[1], - regs->regs[2], regs->regs[3]); + syscall_enter_audit(regs, syscall);
return regs->syscallno; }
In subject: s/heleper/helper/
On 17/11/2025 14:30, Jinjie Ruan wrote:
The generic entry check audit context first and use syscall_get_arguments() helper.
In order to switch to the generic entry for arm64,
Also use the helper.
Extract the syscall_enter_audit() helper to make it clear.
Check audit context for syscall_enter_audit(), which only adds one additional check without any other differences as audit_syscall_entry() check it first otherwise do nothing.
Presumably this is to avoid an unnecessary memcpy(). Overall these changes make syscall_enter_audit() exactly equivalent to the generic one, would be good to spell it out.
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/kernel/ptrace.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 9af3046a2ce9..9455ccf1ecc1 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2376,6 +2376,17 @@ static void report_syscall_exit(struct pt_regs *regs) } } +static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) +{
- if (unlikely(audit_context())) {
unsigned long args[6];syscall_get_arguments(current, regs, args);audit_syscall_entry(syscall, args[0], args[1], args[2], args[3]);- }
Nit: empty line.
- Kevin
+}
int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) { int ret; @@ -2406,8 +2417,7 @@ int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) syscall = syscall_get_nr(current, regs); }
- audit_syscall_entry(syscall, regs->orig_x0, regs->regs[1],
regs->regs[2], regs->regs[3]);
- syscall_enter_audit(regs, syscall);
return regs->syscallno; }
Differ from generic entry, due to historical reasons, ARM64 need to save/restore during syscall entry/exit because ARM64 use a scratch register (ip(r12) on AArch32, x7 on AArch64) to denote syscall entry/exit.
In preparation for moving arm64 over to the generic entry code, add arch_ptrace_report_syscall_entry/exit() as the default ptrace_report_syscall_entry/exit() implementation. This allows arm64 to implement the architecture specific version.
Suggested-by: Mark Rutland mark.rutland@arm.com Suggested-by: Kevin Brodsky kevin.brodsky@arm.com Suggested-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- kernel/entry/syscall-common.c | 43 +++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c index 66e6ba7fa80c..27310e611567 100644 --- a/kernel/entry/syscall-common.c +++ b/kernel/entry/syscall-common.c @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) } }
+/** + * arch_ptrace_report_syscall_entry - Architecture specific + * ptrace_report_syscall_entry(). + * + * Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry(). + * Defaults to ptrace_report_syscall_entry. + * + * The main purpose is to support arch-specific ptrace_report_syscall_entry() + * implementation. + */ +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs); + +#ifndef arch_ptrace_report_syscall_entry +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs) +{ + return ptrace_report_syscall_entry(regs); +} +#endif + long syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long work) { @@ -34,7 +53,7 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
/* Handle ptrace */ if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) { - ret = ptrace_report_syscall_entry(regs); + ret = arch_ptrace_report_syscall_entry(regs); if (ret || (work & SYSCALL_WORK_SYSCALL_EMU)) return -1L; } @@ -84,6 +103,26 @@ static inline bool report_single_step(unsigned long work) return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP; }
+/** + * arch_ptrace_report_syscall_exit - Architecture specific + * ptrace_report_syscall_exit. + * + * Invoked from syscall_exit_work() to wrap ptrace_report_syscall_exit(). + * + * The main purpose is to support arch-specific ptrace_report_syscall_exit + * implementation. + */ +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs, + int step); + +#ifndef arch_ptrace_report_syscall_exit +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs, + int step) +{ + ptrace_report_syscall_exit(regs, step); +} +#endif + void syscall_exit_work(struct pt_regs *regs, unsigned long work) { bool step; @@ -108,5 +147,5 @@ void syscall_exit_work(struct pt_regs *regs, unsigned long work)
step = report_single_step(work); if (step || work & SYSCALL_WORK_SYSCALL_TRACE) - ptrace_report_syscall_exit(regs, step); + arch_ptrace_report_syscall_exit(regs, step); }
On 17/11/2025 14:30, Jinjie Ruan wrote:
Differ from generic entry, due to historical reasons, ARM64 need to save/restore during syscall entry/exit because ARM64 use a scratch register (ip(r12) on AArch32, x7 on AArch64) to denote syscall entry/exit.
In preparation for moving arm64 over to the generic entry code, add arch_ptrace_report_syscall_entry/exit() as the default ptrace_report_syscall_entry/exit() implementation. This allows arm64 to implement the architecture specific version.
Suggested-by: Mark Rutland mark.rutland@arm.com Suggested-by: Kevin Brodsky kevin.brodsky@arm.com
I don't think I suggested this patch. I see that I suggested renaming some functions on v3, but I don't think that justifies a Suggested-by tag.
Suggested-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
kernel/entry/syscall-common.c | 43 +++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c index 66e6ba7fa80c..27310e611567 100644 --- a/kernel/entry/syscall-common.c +++ b/kernel/entry/syscall-common.c @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) } } +/**
- arch_ptrace_report_syscall_entry - Architecture specific
ptrace_report_syscall_entry().
- Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
- Defaults to ptrace_report_syscall_entry.
- The main purpose is to support arch-specific ptrace_report_syscall_entry()
- implementation.
- */
+static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
+#ifndef arch_ptrace_report_syscall_entry +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs) +{
- return ptrace_report_syscall_entry(regs);
I saw that Thomas suggested this approach on v4, and it makes sense to me, but I find the naming surprising. If an architecture does need extra handling, then the generic function should never be called from generic code. So it seems to me that the more logical change would be:
* Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry * Introduce ptrace_report_syscall_entry(), defaults to __ptrace_report_syscall_entry()
All this would be done in <linux/ptrace.h>, where it clearly belongs. The __ prefix makes it clear that the generic function is not the main interface. Even better, no need to change any caller with that approach.
- Kevin
+} +#endif
long syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long work) { @@ -34,7 +53,7 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall, /* Handle ptrace */ if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
ret = ptrace_report_syscall_entry(regs);
if (ret || (work & SYSCALL_WORK_SYSCALL_EMU)) return -1L; }ret = arch_ptrace_report_syscall_entry(regs);@@ -84,6 +103,26 @@ static inline bool report_single_step(unsigned long work) return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP; } +/**
- arch_ptrace_report_syscall_exit - Architecture specific
ptrace_report_syscall_exit.
- Invoked from syscall_exit_work() to wrap ptrace_report_syscall_exit().
- The main purpose is to support arch-specific ptrace_report_syscall_exit
- implementation.
- */
+static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
int step);+#ifndef arch_ptrace_report_syscall_exit +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
int step)+{
- ptrace_report_syscall_exit(regs, step);
+} +#endif
void syscall_exit_work(struct pt_regs *regs, unsigned long work) { bool step; @@ -108,5 +147,5 @@ void syscall_exit_work(struct pt_regs *regs, unsigned long work) step = report_single_step(work); if (step || work & SYSCALL_WORK_SYSCALL_TRACE)
ptrace_report_syscall_exit(regs, step);
arch_ptrace_report_syscall_exit(regs, step);}
Add has_syscall_work() helper to facilitate reuse of this function in other places.
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- include/linux/entry-common.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h index cd6dacb2d8bf..e0f94e937e84 100644 --- a/include/linux/entry-common.h +++ b/include/linux/entry-common.h @@ -44,6 +44,11 @@ SYSCALL_WORK_SYSCALL_EXIT_TRAP | \ ARCH_SYSCALL_WORK_EXIT)
+static inline bool has_syscall_work(unsigned long work) +{ + return unlikely(work & SYSCALL_WORK_ENTER); +} + /** * syscall_enter_from_user_mode_prepare - Establish state and enable interrupts * @regs: Pointer to currents pt_regs @@ -91,7 +96,7 @@ static __always_inline long syscall_enter_from_user_mode_work(struct pt_regs *re { unsigned long work = READ_ONCE(current_thread_info()->syscall_work);
- if (work & SYSCALL_WORK_ENTER) + if (has_syscall_work(work)) syscall = syscall_trace_enter(regs, syscall, work);
return syscall;
On 17/11/2025 14:30, Jinjie Ruan wrote:
Add has_syscall_work() helper to facilitate reuse of this function in other places.
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
include/linux/entry-common.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h index cd6dacb2d8bf..e0f94e937e84 100644 --- a/include/linux/entry-common.h +++ b/include/linux/entry-common.h @@ -44,6 +44,11 @@ SYSCALL_WORK_SYSCALL_EXIT_TRAP | \ ARCH_SYSCALL_WORK_EXIT) +static inline bool has_syscall_work(unsigned long work) +{
- return unlikely(work & SYSCALL_WORK_ENTER);
I'm not sure this is a good idea, since the generic syscall machinery has two separate sets of flags (SYSCALL_WORK_ENTER and SYSCALL_WORK_EXIT). Of course we could reflect that in the helper's name, but since it's only used twice after patch 10, maybe we don't need a helper at all.
- Kevin
+}
/**
- syscall_enter_from_user_mode_prepare - Establish state and enable interrupts
- @regs: Pointer to currents pt_regs
@@ -91,7 +96,7 @@ static __always_inline long syscall_enter_from_user_mode_work(struct pt_regs *re { unsigned long work = READ_ONCE(current_thread_info()->syscall_work);
- if (work & SYSCALL_WORK_ENTER)
- if (has_syscall_work(work)) syscall = syscall_trace_enter(regs, syscall, work);
return syscall;
Currently, x86, Riscv, Loongarch use the generic entry which makes maintainers' work easier and codes more elegant. arm64 has already switched to the generic IRQ entry, so completely convert arm64 to use the generic entry infrastructure from kernel/entry/*.
The changes are below: - Remove TIF_SYSCALL_* flag, _TIF_WORK_MASK, _TIF_SYSCALL_WORK.
- Implement arch_ptrace_report_syscall_entry/exit() with report_syscall_enter/exit() to do arm64-specific save/restore during syscall entry/exit.
- Remove arm64 syscall_trace_enter(), syscall_exit_to_user_mode_prepare(), and related sub-functions including syscall_trace_exit() and syscall_enter_audit(), by calling generic entry's functions with similar functionality.
- Implement arch_syscall_is_vdso_sigreturn() to support "Syscall User Dispatch".
Suggested-by: Mark Rutland mark.rutland@arm.com Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com --- arch/arm64/Kconfig | 2 +- arch/arm64/include/asm/entry-common.h | 69 +++++++++++++ arch/arm64/include/asm/syscall.h | 30 +++++- arch/arm64/include/asm/thread_info.h | 22 +---- arch/arm64/kernel/debug-monitors.c | 7 ++ arch/arm64/kernel/ptrace.c | 134 -------------------------- arch/arm64/kernel/signal.c | 2 +- arch/arm64/kernel/syscall.c | 6 +- 8 files changed, 108 insertions(+), 164 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 6663ffd23f25..1463ff15d67a 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -152,9 +152,9 @@ config ARM64 select GENERIC_CPU_DEVICES select GENERIC_CPU_VULNERABILITIES select GENERIC_EARLY_IOREMAP + select GENERIC_ENTRY select GENERIC_IDLE_POLL_SETUP select GENERIC_IOREMAP - select GENERIC_IRQ_ENTRY select GENERIC_IRQ_IPI select GENERIC_IRQ_KEXEC_CLEAR_VM_FORWARD select GENERIC_IRQ_PROBE diff --git a/arch/arm64/include/asm/entry-common.h b/arch/arm64/include/asm/entry-common.h index cab8cd78f693..ee4dda1b01a2 100644 --- a/arch/arm64/include/asm/entry-common.h +++ b/arch/arm64/include/asm/entry-common.h @@ -11,6 +11,11 @@ #include <asm/mte.h> #include <asm/stacktrace.h>
+enum ptrace_syscall_dir { + PTRACE_SYSCALL_ENTER = 0, + PTRACE_SYSCALL_EXIT, +}; + #define ARCH_EXIT_TO_USER_MODE_WORK (_TIF_MTE_ASYNC_FAULT | _TIF_FOREIGN_FPSTATE)
static __always_inline void arch_exit_to_user_mode_work(struct pt_regs *regs, @@ -54,4 +59,68 @@ static inline bool arch_irqentry_exit_need_resched(void)
#define arch_irqentry_exit_need_resched arch_irqentry_exit_need_resched
+static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs) +{ + unsigned long saved_reg; + int regno, ret; + + /* + * We have some ABI weirdness here in the way that we handle syscall + * exit stops because we indicate whether or not the stop has been + * signalled from syscall entry or syscall exit by clobbering a general + * purpose register (ip/r12 for AArch32, x7 for AArch64) in the tracee + * and restoring its old value after the stop. This means that: + * + * - Any writes by the tracer to this register during the stop are + * ignored/discarded. + * + * - The actual value of the register is not available during the stop, + * so the tracer cannot save it and restore it later. + * + * - Syscall stops behave differently to seccomp and pseudo-step traps + * (the latter do not nobble any registers). + */ + regno = (is_compat_task() ? 12 : 7); + saved_reg = regs->regs[regno]; + regs->regs[regno] = PTRACE_SYSCALL_ENTER; + + ret = ptrace_report_syscall_entry(regs); + if (ret) + forget_syscall(regs); + + regs->regs[regno] = saved_reg; + + return ret; +} + +#define arch_ptrace_report_syscall_entry arch_ptrace_report_syscall_entry + +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs, + int step) +{ + unsigned long saved_reg; + int regno; + + /* See comment for arch_ptrace_report_syscall_entry() */ + regno = (is_compat_task() ? 12 : 7); + saved_reg = regs->regs[regno]; + regs->regs[regno] = PTRACE_SYSCALL_EXIT; + + if (!step) { + ptrace_report_syscall_exit(regs, 0); + regs->regs[regno] = saved_reg; + } else { + regs->regs[regno] = saved_reg; + + /* + * Signal a pseudo-step exception since we are stepping but + * tracer modifications to the registers may have rewound the + * state machine. + */ + ptrace_report_syscall_exit(regs, 1); + } +} + +#define arch_ptrace_report_syscall_exit arch_ptrace_report_syscall_exit + #endif /* _ASM_ARM64_ENTRY_COMMON_H */ diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h index 6225981fbbdb..c91938718468 100644 --- a/arch/arm64/include/asm/syscall.h +++ b/arch/arm64/include/asm/syscall.h @@ -9,6 +9,8 @@ #include <linux/compat.h> #include <linux/err.h>
+#include <asm/vdso.h> + typedef long (*syscall_fn_t)(const struct pt_regs *regs);
extern const syscall_fn_t sys_call_table[]; @@ -114,12 +116,30 @@ static inline int syscall_get_arch(struct task_struct *task) return AUDIT_ARCH_AARCH64; }
-static inline bool has_syscall_work(unsigned long flags) +static inline bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs) { - return unlikely(flags & _TIF_SYSCALL_WORK); -} + unsigned long vdso = (unsigned long)current->mm->context.vdso; + unsigned long vdso_pages, vdso_text_len; + unsigned long pc = regs->pc - 4;
-int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags); -void syscall_exit_to_user_mode_prepare(struct pt_regs *regs); +#ifdef CONFIG_COMPAT + if (is_compat_task()) { + vdso = (unsigned long)current->mm->context.sigpage; + if (pc >= vdso && pc < vdso + PAGE_SIZE) + return true; + + return false; + } +#endif + if (regs->syscallno != __NR_rt_sigreturn) + return false; + + vdso_pages = (vdso_end - vdso_start) >> PAGE_SHIFT; + vdso_text_len = vdso_pages << PAGE_SHIFT; + if (pc < vdso || pc >= vdso + vdso_text_len) + return false; + + return true; +}
#endif /* __ASM_SYSCALL_H */ diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h index f241b8601ebd..0c083be23018 100644 --- a/arch/arm64/include/asm/thread_info.h +++ b/arch/arm64/include/asm/thread_info.h @@ -43,6 +43,7 @@ struct thread_info { void *scs_sp; #endif u32 cpu; + unsigned long syscall_work; /* SYSCALL_WORK_ flags */ };
#define thread_saved_pc(tsk) \ @@ -65,11 +66,6 @@ void arch_setup_new_exec(void); #define TIF_UPROBE 5 /* uprobe breakpoint or singlestep */ #define TIF_MTE_ASYNC_FAULT 6 /* MTE Asynchronous Tag Check Fault */ #define TIF_NOTIFY_SIGNAL 7 /* signal notifications exist */ -#define TIF_SYSCALL_TRACE 8 /* syscall trace active */ -#define TIF_SYSCALL_AUDIT 9 /* syscall auditing */ -#define TIF_SYSCALL_TRACEPOINT 10 /* syscall tracepoint for ftrace */ -#define TIF_SECCOMP 11 /* syscall secure computing */ -#define TIF_SYSCALL_EMU 12 /* syscall emulation active */ #define TIF_PATCH_PENDING 13 /* pending live patching update */ #define TIF_MEMDIE 18 /* is terminating due to OOM killer */ #define TIF_FREEZE 19 @@ -92,30 +88,14 @@ void arch_setup_new_exec(void); #define _TIF_NEED_RESCHED_LAZY (1 << TIF_NEED_RESCHED_LAZY) #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) #define _TIF_FOREIGN_FPSTATE (1 << TIF_FOREIGN_FPSTATE) -#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) -#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) -#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) -#define _TIF_SECCOMP (1 << TIF_SECCOMP) -#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU) #define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING) #define _TIF_UPROBE (1 << TIF_UPROBE) -#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) #define _TIF_32BIT (1 << TIF_32BIT) #define _TIF_SVE (1 << TIF_SVE) #define _TIF_MTE_ASYNC_FAULT (1 << TIF_MTE_ASYNC_FAULT) #define _TIF_NOTIFY_SIGNAL (1 << TIF_NOTIFY_SIGNAL) #define _TIF_TSC_SIGSEGV (1 << TIF_TSC_SIGSEGV)
-#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY | \ - _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \ - _TIF_UPROBE | _TIF_MTE_ASYNC_FAULT | \ - _TIF_NOTIFY_SIGNAL | _TIF_SIGPENDING | \ - _TIF_PATCH_PENDING) - -#define _TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \ - _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \ - _TIF_SYSCALL_EMU) - #ifdef CONFIG_SHADOW_CALL_STACK #define INIT_SCS \ .scs_base = init_shadow_call_stack, \ diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c index 29307642f4c9..e67643a70405 100644 --- a/arch/arm64/kernel/debug-monitors.c +++ b/arch/arm64/kernel/debug-monitors.c @@ -385,11 +385,18 @@ void user_enable_single_step(struct task_struct *task)
if (!test_and_set_ti_thread_flag(ti, TIF_SINGLESTEP)) set_regs_spsr_ss(task_pt_regs(task)); + + /* + * Ensure that a trap is triggered once stepping out of a system + * call prior to executing any user instruction. + */ + set_task_syscall_work(task, SYSCALL_EXIT_TRAP); } NOKPROBE_SYMBOL(user_enable_single_step);
void user_disable_single_step(struct task_struct *task) { clear_ti_thread_flag(task_thread_info(task), TIF_SINGLESTEP); + clear_task_syscall_work(task, SYSCALL_EXIT_TRAP); } NOKPROBE_SYMBOL(user_disable_single_step); diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 9455ccf1ecc1..9e3b39e207d1 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -42,9 +42,6 @@ #include <asm/traps.h> #include <asm/system_misc.h>
-#define CREATE_TRACE_POINTS -#include <trace/events/syscalls.h> - struct pt_regs_offset { const char *name; int offset; @@ -2312,137 +2309,6 @@ long arch_ptrace(struct task_struct *child, long request, return ptrace_request(child, request, addr, data); }
-enum ptrace_syscall_dir { - PTRACE_SYSCALL_ENTER = 0, - PTRACE_SYSCALL_EXIT, -}; - -static int report_syscall_enter(struct pt_regs *regs) -{ - unsigned long saved_reg; - int regno, ret; - - /* - * We have some ABI weirdness here in the way that we handle syscall - * exit stops because we indicate whether or not the stop has been - * signalled from syscall entry or syscall exit by clobbering a general - * purpose register (ip/r12 for AArch32, x7 for AArch64) in the tracee - * and restoring its old value after the stop. This means that: - * - * - Any writes by the tracer to this register during the stop are - * ignored/discarded. - * - * - The actual value of the register is not available during the stop, - * so the tracer cannot save it and restore it later. - * - * - Syscall stops behave differently to seccomp and pseudo-step traps - * (the latter do not nobble any registers). - */ - regno = (is_compat_task() ? 12 : 7); - saved_reg = regs->regs[regno]; - regs->regs[regno] = PTRACE_SYSCALL_ENTER; - - ret = ptrace_report_syscall_entry(regs); - if (ret) - forget_syscall(regs); - - regs->regs[regno] = saved_reg; - - return ret; -} - -static void report_syscall_exit(struct pt_regs *regs) -{ - int regno; - unsigned long saved_reg; - - /* See comment for report_syscall_enter() above */ - regno = (is_compat_task() ? 12 : 7); - saved_reg = regs->regs[regno]; - regs->regs[regno] = PTRACE_SYSCALL_EXIT; - - if (!test_thread_flag(TIF_SINGLESTEP)) { - ptrace_report_syscall_exit(regs, 0); - regs->regs[regno] = saved_reg; - } else { - regs->regs[regno] = saved_reg; - - /* - * Signal a pseudo-step exception since we are stepping but - * tracer modifications to the registers may have rewound the - * state machine. - */ - ptrace_report_syscall_exit(regs, 1); - } -} - -static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) -{ - if (unlikely(audit_context())) { - unsigned long args[6]; - - syscall_get_arguments(current, regs, args); - audit_syscall_entry(syscall, args[0], args[1], args[2], args[3]); - } - -} - -int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags) -{ - int ret; - - if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { - ret = report_syscall_enter(regs); - if (ret || (flags & _TIF_SYSCALL_EMU)) - return NO_SYSCALL; - } - - /* Do the secure computing after ptrace; failures should be fast. */ - if (flags & _TIF_SECCOMP) { - ret = __secure_computing(NULL); - if (ret == -1L) - return NO_SYSCALL; - } - - /* Either of the above might have changed the syscall number */ - syscall = syscall_get_nr(current, regs); - - if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) { - trace_sys_enter(regs, syscall); - - /* - * Probes or BPF hooks in the tracepoint may have changed the - * system call number as well. - */ - syscall = syscall_get_nr(current, regs); - } - - syscall_enter_audit(regs, syscall); - - return regs->syscallno; -} - -static void syscall_trace_exit(struct pt_regs *regs, unsigned long flags) -{ - audit_syscall_exit(regs); - - if (flags & _TIF_SYSCALL_TRACEPOINT) - trace_sys_exit(regs, syscall_get_return_value(current, regs)); - - if (flags & (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP)) - report_syscall_exit(regs); -} - -void syscall_exit_to_user_mode_prepare(struct pt_regs *regs) -{ - unsigned long flags = read_thread_flags(); - - rseq_syscall(regs); - - if (has_syscall_work(flags) || flags & _TIF_SINGLESTEP) - syscall_trace_exit(regs, flags); -} - /* * SPSR_ELx bits which are always architecturally RES0 per ARM DDI 0487D.a. * We permit userspace to set SSBS (AArch64 bit 12, AArch32 bit 23) which is diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c index 1110eeb21f57..d3ec1892b3c7 100644 --- a/arch/arm64/kernel/signal.c +++ b/arch/arm64/kernel/signal.c @@ -8,8 +8,8 @@
#include <linux/cache.h> #include <linux/compat.h> +#include <linux/entry-common.h> #include <linux/errno.h> -#include <linux/irq-entry-common.h> #include <linux/kernel.h> #include <linux/signal.h> #include <linux/freezer.h> diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index 9713b038d750..61e24eba9a86 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -2,6 +2,7 @@
#include <linux/compiler.h> #include <linux/context_tracking.h> +#include <linux/entry-common.h> #include <linux/errno.h> #include <linux/nospec.h> #include <linux/ptrace.h> @@ -68,6 +69,7 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno, static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, const syscall_fn_t syscall_table[]) { + unsigned long work = READ_ONCE(current_thread_info()->syscall_work); unsigned long flags = read_thread_flags();
regs->orig_x0 = regs->regs[0]; @@ -101,7 +103,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, return; }
- if (has_syscall_work(flags)) { + if (has_syscall_work(work)) { /* * The de-facto standard way to skip a system call using ptrace * is to set the system call to -1 (NO_SYSCALL) and set x0 to a @@ -119,7 +121,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, */ if (scno == NO_SYSCALL) syscall_set_return_value(current, regs, -ENOSYS, 0); - scno = syscall_trace_enter(regs, regs->syscallno, flags); + scno = syscall_trace_enter(regs, regs->syscallno, work); if (scno == NO_SYSCALL) { syscall_exit_to_user_mode_prepare(regs); return;
On 17/11/2025 14:30, Jinjie Ruan wrote:
[...]
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h index 6225981fbbdb..c91938718468 100644 --- a/arch/arm64/include/asm/syscall.h +++ b/arch/arm64/include/asm/syscall.h @@ -9,6 +9,8 @@ #include <linux/compat.h> #include <linux/err.h> +#include <asm/vdso.h>
typedef long (*syscall_fn_t)(const struct pt_regs *regs); extern const syscall_fn_t sys_call_table[]; @@ -114,12 +116,30 @@ static inline int syscall_get_arch(struct task_struct *task) return AUDIT_ARCH_AARCH64; } -static inline bool has_syscall_work(unsigned long flags) +static inline bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs) {
- return unlikely(flags & _TIF_SYSCALL_WORK);
-}
- unsigned long vdso = (unsigned long)current->mm->context.vdso;
- unsigned long vdso_pages, vdso_text_len;
- unsigned long pc = regs->pc - 4;
On AArch32 (i.e. COMPAT), instructions may be 16-bit (in T32/Thumb), so we shouldn't blindly use PC - 4.
-int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags); -void syscall_exit_to_user_mode_prepare(struct pt_regs *regs); +#ifdef CONFIG_COMPAT
- if (is_compat_task()) {
vdso = (unsigned long)current->mm->context.sigpage;if (pc >= vdso && pc < vdso + PAGE_SIZE)
Just return that expression (instead of true/false).
I think the approach is reasonable, as we have 4 possible trampolines in COMPAT and they all live in a dedicated page. I don't think we need to worry about offsetting PC, because even if it points after the last trampoline, it will still fall within the page. IOW, just use the unmodified value of regs->pc.
return true;return false;- }
+#endif
- if (regs->syscallno != __NR_rt_sigreturn)
return false;- vdso_pages = (vdso_end - vdso_start) >> PAGE_SHIFT;
- vdso_text_len = vdso_pages << PAGE_SHIFT;
- if (pc < vdso || pc >= vdso + vdso_text_len)
return false;
Why not use the same approach as x86 and simply check that regs->pc points after the trampoline? We already have a way to get the address of the vDSO's sigreturn trampoline on arm64: VDSO_SYMBOL(current->mm->context.vdso, sigtramp) (see signal.c). The trampoline consists of two instructions that cannot be changed (pretty much part of the ABI), so we could compare regs->pc with sigtramp + 8.
- return true;
+} #endif /* __ASM_SYSCALL_H */ diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h index f241b8601ebd..0c083be23018 100644 --- a/arch/arm64/include/asm/thread_info.h +++ b/arch/arm64/include/asm/thread_info.h @@ -43,6 +43,7 @@ struct thread_info { void *scs_sp; #endif u32 cpu;
- unsigned long syscall_work; /* SYSCALL_WORK_ flags */
}; #define thread_saved_pc(tsk) \ @@ -65,11 +66,6 @@ void arch_setup_new_exec(void); #define TIF_UPROBE 5 /* uprobe breakpoint or singlestep */ #define TIF_MTE_ASYNC_FAULT 6 /* MTE Asynchronous Tag Check Fault */ #define TIF_NOTIFY_SIGNAL 7 /* signal notifications exist */ -#define TIF_SYSCALL_TRACE 8 /* syscall trace active */ -#define TIF_SYSCALL_AUDIT 9 /* syscall auditing */ -#define TIF_SYSCALL_TRACEPOINT 10 /* syscall tracepoint for ftrace */ -#define TIF_SECCOMP 11 /* syscall secure computing */ -#define TIF_SYSCALL_EMU 12 /* syscall emulation active */ #define TIF_PATCH_PENDING 13 /* pending live patching update */ #define TIF_MEMDIE 18 /* is terminating due to OOM killer */ #define TIF_FREEZE 19 @@ -92,30 +88,14 @@ void arch_setup_new_exec(void); #define _TIF_NEED_RESCHED_LAZY (1 << TIF_NEED_RESCHED_LAZY) #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) #define _TIF_FOREIGN_FPSTATE (1 << TIF_FOREIGN_FPSTATE) -#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) -#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) -#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) -#define _TIF_SECCOMP (1 << TIF_SECCOMP) -#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU) #define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING) #define _TIF_UPROBE (1 << TIF_UPROBE) -#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) #define _TIF_32BIT (1 << TIF_32BIT) #define _TIF_SVE (1 << TIF_SVE) #define _TIF_MTE_ASYNC_FAULT (1 << TIF_MTE_ASYNC_FAULT) #define _TIF_NOTIFY_SIGNAL (1 << TIF_NOTIFY_SIGNAL) #define _TIF_TSC_SIGSEGV (1 << TIF_TSC_SIGSEGV) -#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY | \
_TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \_TIF_UPROBE | _TIF_MTE_ASYNC_FAULT | \_TIF_NOTIFY_SIGNAL | _TIF_SIGPENDING | \_TIF_PATCH_PENDING)
AFAICT this was already unused before this series, since commit b3cf07851b6c ("arm64: entry: Switch to generic IRQ entry"). It should be removed in a separate commit.
[...]
-void syscall_exit_to_user_mode_prepare(struct pt_regs *regs) -{
- unsigned long flags = read_thread_flags();
- rseq_syscall(regs);
- if (has_syscall_work(flags) || flags & _TIF_SINGLESTEP)
I believe switching to the generic function introduces a change here: syscall_exit_work() is only called if a flag in SYSCALL_WORK_EXIT is set, and this set does not include SYSCALL_EMU and SECCOMP. Practically this means that audit_syscall_exit() will no longer be called if only SECCOMP and/or SYSCALL_EMU is set.
It doesn't feel like a major behaviour change, but it should be pointed out.
- Kevin
syscall_trace_exit(regs, flags);-}
[...]
From: kemal kmal@cock.li
Support aarch64 to test "Syscall User Dispatch" with sud_test selftest testcase.
Signed-off-by: kemal kmal@cock.li --- tools/testing/selftests/syscall_user_dispatch/sud_test.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/syscall_user_dispatch/sud_test.c b/tools/testing/selftests/syscall_user_dispatch/sud_test.c index 2eb2c06303f2..f53ebc89befc 100644 --- a/tools/testing/selftests/syscall_user_dispatch/sud_test.c +++ b/tools/testing/selftests/syscall_user_dispatch/sud_test.c @@ -192,6 +192,10 @@ static void handle_sigsys(int sig, siginfo_t *info, void *ucontext) ((ucontext_t *)ucontext)->uc_mcontext.__gregs[REG_A0] = ((ucontext_t *)ucontext)->uc_mcontext.__gregs[REG_A7]; #endif +#ifdef __aarch64__ + ((ucontext_t *)ucontext)->uc_mcontext.regs[0] = (unsigned int) + ((ucontext_t *)ucontext)->uc_mcontext.regs[8]; +#endif }
int setup_sigsys_handler(void)
On 21/11/2025 05:15, Jinjie Ruan wrote:
On 2025/11/19 1:12, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
The generic entry handle error of ptrace_report_syscall_entry(), but arm64 not.
This suggests that arm64 ignores the error completely, which isn't the case: no syscall will be performed, but tracing will still occur as normal.
What this patch seems to be doing is to abort the _enter sequence if ptrace_report_syscall_entry() errors out. The commit title and message should be reworded accordingly.
You are right,the description is unclear .
As the comment said, the calling arch code should abort the system
Which comment?
ptrace_report_syscall_entry()
I found that later, this should be clarified in the commit message.
call and must prevent normal entry so no system call is made if ptrace_report_syscall_entry() return nonzero.
This is already the case since we're calling forget_syscall().
Yes. it is similar with the generic entry returns NO_SYSCALL.
My point is that this patch is not changing this - arm64 was already skipping the syscall if ptrace_report_syscall_entry() returns an error.
In preparation for moving arm64 over to the generic entry code, return early if ptrace_report_syscall_entry() encounters an error.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/kernel/ptrace.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 95984bbf53db..707951ad5d24 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2317,10 +2317,10 @@ enum ptrace_syscall_dir { PTRACE_SYSCALL_EXIT, }; -static void report_syscall_enter(struct pt_regs *regs) +static int report_syscall_enter(struct pt_regs *regs) {
- int regno; unsigned long saved_reg;
- int regno, ret;
/* * We have some ABI weirdness here in the way that we handle syscall @@ -2342,9 +2342,13 @@ static void report_syscall_enter(struct pt_regs *regs) saved_reg = regs->regs[regno]; regs->regs[regno] = PTRACE_SYSCALL_ENTER;
- if (ptrace_report_syscall_entry(regs))
- ret = ptrace_report_syscall_entry(regs);
- if (ret) forget_syscall(regs);
The generic syscall_trace_enter() doesn't do this (i.e. setting regs->syscallno to NO_SYSCALL). Is that an oversight or do we just not need it? In principle this does have a visible effect (e.g. via REGSET_SYSTEM_CALL).
We just not need it because the original syscall_trace_enter() need use regs->syscallno as the return value, but now we return early by using NO_SYSCALL.
Calling forget_syscall() means setting regs->syscallno to NO_SYSCALL. It is indeed no longer required for the entry sequence, but it does have visible side effects. For instance, regs->syscallno can be inspected via ptrace(PTRACE_GETREGSET, REGSET_SYSTEM_CALL). So the question is whether we need to deviate from the generic path on arm64 (this is mostly a question for arm64 maintainers).
- Kevin
On 2025/11/19 1:13, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
Differ from generic entry, due to historical reasons, ARM64 need to save/restore during syscall entry/exit because ARM64 use a scratch register (ip(r12) on AArch32, x7 on AArch64) to denote syscall entry/exit.
In preparation for moving arm64 over to the generic entry code, add arch_ptrace_report_syscall_entry/exit() as the default ptrace_report_syscall_entry/exit() implementation. This allows arm64 to implement the architecture specific version.
Suggested-by: Mark Rutland mark.rutland@arm.com Suggested-by: Kevin Brodsky kevin.brodsky@arm.com
I don't think I suggested this patch. I see that I suggested renaming some functions on v3, but I don't think that justifies a Suggested-by tag.
Suggested-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
kernel/entry/syscall-common.c | 43 +++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c index 66e6ba7fa80c..27310e611567 100644 --- a/kernel/entry/syscall-common.c +++ b/kernel/entry/syscall-common.c @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) } } +/**
- arch_ptrace_report_syscall_entry - Architecture specific
ptrace_report_syscall_entry().
- Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
- Defaults to ptrace_report_syscall_entry.
- The main purpose is to support arch-specific ptrace_report_syscall_entry()
- implementation.
- */
+static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
+#ifndef arch_ptrace_report_syscall_entry +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs) +{
- return ptrace_report_syscall_entry(regs);
I saw that Thomas suggested this approach on v4, and it makes sense to me, but I find the naming surprising. If an architecture does need extra handling, then the generic function should never be called from generic code. So it seems to me that the more logical change would be:
- Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
- Introduce ptrace_report_syscall_entry(), defaults to
__ptrace_report_syscall_entry()
If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an architecture also needs to redefine this function, but the architecture's own <asm/entry-common.h> must include <linux/ptrace.h>, the function will end up being defined twice and cause a "duplicate definition" compile error.
All this would be done in <linux/ptrace.h>, where it clearly belongs. The __ prefix makes it clear that the generic function is not the main interface. Even better, no need to change any caller with that approach.
- Kevin
+} +#endif
long syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long work) { @@ -34,7 +53,7 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall, /* Handle ptrace */ if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
ret = ptrace_report_syscall_entry(regs);
if (ret || (work & SYSCALL_WORK_SYSCALL_EMU)) return -1L; }ret = arch_ptrace_report_syscall_entry(regs);@@ -84,6 +103,26 @@ static inline bool report_single_step(unsigned long work) return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP; } +/**
- arch_ptrace_report_syscall_exit - Architecture specific
ptrace_report_syscall_exit.
- Invoked from syscall_exit_work() to wrap ptrace_report_syscall_exit().
- The main purpose is to support arch-specific ptrace_report_syscall_exit
- implementation.
- */
+static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
int step);+#ifndef arch_ptrace_report_syscall_exit +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs *regs,
int step)+{
- ptrace_report_syscall_exit(regs, step);
+} +#endif
void syscall_exit_work(struct pt_regs *regs, unsigned long work) { bool step; @@ -108,5 +147,5 @@ void syscall_exit_work(struct pt_regs *regs, unsigned long work) step = report_single_step(work); if (step || work & SYSCALL_WORK_SYSCALL_TRACE)
ptrace_report_syscall_exit(regs, step);
arch_ptrace_report_syscall_exit(regs, step);}
On 24/11/2025 10:34, Jinjie Ruan wrote:
On 2025/11/19 1:13, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
[...]
diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c index 66e6ba7fa80c..27310e611567 100644 --- a/kernel/entry/syscall-common.c +++ b/kernel/entry/syscall-common.c @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) } } +/**
- arch_ptrace_report_syscall_entry - Architecture specific
ptrace_report_syscall_entry().
- Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
- Defaults to ptrace_report_syscall_entry.
- The main purpose is to support arch-specific ptrace_report_syscall_entry()
- implementation.
- */
+static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
+#ifndef arch_ptrace_report_syscall_entry +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs) +{
- return ptrace_report_syscall_entry(regs);
I saw that Thomas suggested this approach on v4, and it makes sense to me, but I find the naming surprising. If an architecture does need extra handling, then the generic function should never be called from generic code. So it seems to me that the more logical change would be:
- Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
- Introduce ptrace_report_syscall_entry(), defaults to
__ptrace_report_syscall_entry()
If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an architecture also needs to redefine this function, but the architecture's own <asm/entry-common.h> must include <linux/ptrace.h>, the function will end up being defined twice and cause a "duplicate definition" compile error.
There's plenty of arch-defined functions in <linux/ptrace.h> already. __ptrace_report_syscall_entry() should be defined inside an #ifndef and architectures can define their own implementation in <asm/ptrace.h>, like force_successful_syscall_return() for instance.
- Kevin
All this would be done in <linux/ptrace.h>, where it clearly belongs. The __ prefix makes it clear that the generic function is not the main interface. Even better, no need to change any caller with that approach.
- Kevin
[...]
On 2025/11/24 23:23, Kevin Brodsky wrote:
On 24/11/2025 10:34, Jinjie Ruan wrote:
On 2025/11/19 1:13, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
[...]
diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c index 66e6ba7fa80c..27310e611567 100644 --- a/kernel/entry/syscall-common.c +++ b/kernel/entry/syscall-common.c @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) } } +/**
- arch_ptrace_report_syscall_entry - Architecture specific
ptrace_report_syscall_entry().
- Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
- Defaults to ptrace_report_syscall_entry.
- The main purpose is to support arch-specific ptrace_report_syscall_entry()
- implementation.
- */
+static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
+#ifndef arch_ptrace_report_syscall_entry +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs) +{
- return ptrace_report_syscall_entry(regs);
I saw that Thomas suggested this approach on v4, and it makes sense to me, but I find the naming surprising. If an architecture does need extra handling, then the generic function should never be called from generic code. So it seems to me that the more logical change would be:
- Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
- Introduce ptrace_report_syscall_entry(), defaults to
__ptrace_report_syscall_entry()
If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an architecture also needs to redefine this function, but the architecture's own <asm/entry-common.h> must include <linux/ptrace.h>, the function will end up being defined twice and cause a "duplicate definition" compile error.
There's plenty of arch-defined functions in <linux/ptrace.h> already. __ptrace_report_syscall_entry() should be defined inside an #ifndef and architectures can define their own implementation in <asm/ptrace.h>, like force_successful_syscall_return() for instance.
Shared functions like ptrace_report_syscall() are all defined in <linux/ptrace.h>. When we want to override __ptrace_report_syscall_entry() in <asm/ptrace.h> we still have to include <linux/ptrace.h> again,then the redefine problem occurs again.
What we actually need to reuse is ptrace_report_syscall_entry() (or __ptrace_report_syscall_entry()).
The arch version need to reuse and wrap ptrace_report_syscall_entry(), because for instance arm64 needs to perform additional operations before and after this step. Therefore, I believe the current implementation is appropriate.
- Kevin
All this would be done in <linux/ptrace.h>, where it clearly belongs. The __ prefix makes it clear that the generic function is not the main interface. Even better, no need to change any caller with that approach.
- Kevin
[...]
On 2025/11/19 1:13, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
Add has_syscall_work() helper to facilitate reuse of this function in other places.
No functional changes.
Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
include/linux/entry-common.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h index cd6dacb2d8bf..e0f94e937e84 100644 --- a/include/linux/entry-common.h +++ b/include/linux/entry-common.h @@ -44,6 +44,11 @@ SYSCALL_WORK_SYSCALL_EXIT_TRAP | \ ARCH_SYSCALL_WORK_EXIT) +static inline bool has_syscall_work(unsigned long work) +{
- return unlikely(work & SYSCALL_WORK_ENTER);
I'm not sure this is a good idea, since the generic syscall machinery has two separate sets of flags (SYSCALL_WORK_ENTER and SYSCALL_WORK_EXIT). Of course we could reflect that in the helper's name, but since it's only used twice after patch 10, maybe we don't need a helper at all.
Will remove it.
- Kevin
+}
/**
- syscall_enter_from_user_mode_prepare - Establish state and enable interrupts
- @regs: Pointer to currents pt_regs
@@ -91,7 +96,7 @@ static __always_inline long syscall_enter_from_user_mode_work(struct pt_regs *re { unsigned long work = READ_ONCE(current_thread_info()->syscall_work);
- if (work & SYSCALL_WORK_ENTER)
- if (has_syscall_work(work)) syscall = syscall_trace_enter(regs, syscall, work);
return syscall;
On 2025/11/19 1:14, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
[...]
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h index 6225981fbbdb..c91938718468 100644 --- a/arch/arm64/include/asm/syscall.h +++ b/arch/arm64/include/asm/syscall.h @@ -9,6 +9,8 @@ #include <linux/compat.h> #include <linux/err.h> +#include <asm/vdso.h>
typedef long (*syscall_fn_t)(const struct pt_regs *regs); extern const syscall_fn_t sys_call_table[]; @@ -114,12 +116,30 @@ static inline int syscall_get_arch(struct task_struct *task) return AUDIT_ARCH_AARCH64; } -static inline bool has_syscall_work(unsigned long flags) +static inline bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs) {
- return unlikely(flags & _TIF_SYSCALL_WORK);
-}
- unsigned long vdso = (unsigned long)current->mm->context.vdso;
- unsigned long vdso_pages, vdso_text_len;
- unsigned long pc = regs->pc - 4;
On AArch32 (i.e. COMPAT), instructions may be 16-bit (in T32/Thumb), so we shouldn't blindly use PC - 4.
-int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags); -void syscall_exit_to_user_mode_prepare(struct pt_regs *regs); +#ifdef CONFIG_COMPAT
- if (is_compat_task()) {
vdso = (unsigned long)current->mm->context.sigpage;if (pc >= vdso && pc < vdso + PAGE_SIZE)Just return that expression (instead of true/false).
I think the approach is reasonable, as we have 4 possible trampolines in COMPAT and they all live in a dedicated page. I don't think we need to worry about offsetting PC, because even if it points after the last trampoline, it will still fall within the page. IOW, just use the unmodified value of regs->pc.
return true;return false;- }
+#endif
- if (regs->syscallno != __NR_rt_sigreturn)
return false;- vdso_pages = (vdso_end - vdso_start) >> PAGE_SHIFT;
- vdso_text_len = vdso_pages << PAGE_SHIFT;
- if (pc < vdso || pc >= vdso + vdso_text_len)
return false;Why not use the same approach as x86 and simply check that regs->pc points after the trampoline? We already have a way to get the address of the vDSO's sigreturn trampoline on arm64: VDSO_SYMBOL(current->mm->context.vdso, sigtramp) (see signal.c). The trampoline consists of two instructions that cannot be changed (pretty much part of the ABI), so we could compare regs->pc with sigtramp + 8.
Will update it.
- return true;
+} #endif /* __ASM_SYSCALL_H */ diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h index f241b8601ebd..0c083be23018 100644 --- a/arch/arm64/include/asm/thread_info.h +++ b/arch/arm64/include/asm/thread_info.h @@ -43,6 +43,7 @@ struct thread_info { void *scs_sp; #endif u32 cpu;
- unsigned long syscall_work; /* SYSCALL_WORK_ flags */
}; #define thread_saved_pc(tsk) \ @@ -65,11 +66,6 @@ void arch_setup_new_exec(void); #define TIF_UPROBE 5 /* uprobe breakpoint or singlestep */ #define TIF_MTE_ASYNC_FAULT 6 /* MTE Asynchronous Tag Check Fault */ #define TIF_NOTIFY_SIGNAL 7 /* signal notifications exist */ -#define TIF_SYSCALL_TRACE 8 /* syscall trace active */ -#define TIF_SYSCALL_AUDIT 9 /* syscall auditing */ -#define TIF_SYSCALL_TRACEPOINT 10 /* syscall tracepoint for ftrace */ -#define TIF_SECCOMP 11 /* syscall secure computing */ -#define TIF_SYSCALL_EMU 12 /* syscall emulation active */ #define TIF_PATCH_PENDING 13 /* pending live patching update */ #define TIF_MEMDIE 18 /* is terminating due to OOM killer */ #define TIF_FREEZE 19 @@ -92,30 +88,14 @@ void arch_setup_new_exec(void); #define _TIF_NEED_RESCHED_LAZY (1 << TIF_NEED_RESCHED_LAZY) #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) #define _TIF_FOREIGN_FPSTATE (1 << TIF_FOREIGN_FPSTATE) -#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) -#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) -#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) -#define _TIF_SECCOMP (1 << TIF_SECCOMP) -#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU) #define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING) #define _TIF_UPROBE (1 << TIF_UPROBE) -#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) #define _TIF_32BIT (1 << TIF_32BIT) #define _TIF_SVE (1 << TIF_SVE) #define _TIF_MTE_ASYNC_FAULT (1 << TIF_MTE_ASYNC_FAULT) #define _TIF_NOTIFY_SIGNAL (1 << TIF_NOTIFY_SIGNAL) #define _TIF_TSC_SIGSEGV (1 << TIF_TSC_SIGSEGV) -#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY | \
_TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \_TIF_UPROBE | _TIF_MTE_ASYNC_FAULT | \_TIF_NOTIFY_SIGNAL | _TIF_SIGPENDING | \_TIF_PATCH_PENDING)AFAICT this was already unused before this series, since commit b3cf07851b6c ("arm64: entry: Switch to generic IRQ entry"). It should be removed in a separate commit.
Yes, it was already unused before this series
[...]
-void syscall_exit_to_user_mode_prepare(struct pt_regs *regs) -{
- unsigned long flags = read_thread_flags();
- rseq_syscall(regs);
- if (has_syscall_work(flags) || flags & _TIF_SINGLESTEP)
I believe switching to the generic function introduces a change here: syscall_exit_work() is only called if a flag in SYSCALL_WORK_EXIT is set, and this set does not include SYSCALL_EMU and SECCOMP. Practically this means that audit_syscall_exit() will no longer be called if only SECCOMP and/or SYSCALL_EMU is set.
It doesn't feel like a major behaviour change, but it should be pointed out.
We can add "ARCH_SYSCALL_WORK_EXIT" to be defined as "SECCOMP ||SYSCALL_EMU" to keep the behaviour unchanged.
- Kevin
syscall_trace_exit(regs, flags);-}
[...]
On 19/11/2025 10:49, Jinjie Ruan wrote:
On 2025/11/19 1:09, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
The generic syscall entry code has the form:
| syscall_trace_enter() | { | ptrace_report_syscall_entry() | } | | syscall_exit_work() | { | ptrace_report_syscall_exit() | }
In preparation for moving arm64 over to the generic entry code, split report_syscall() to two separate enter and exit functions to align the structure of the arm64 code with syscall_trace_enter() and syscall_exit_work() from the generic entry code.
No functional changes.
Suggested-by: Mark Rutland mark.rutland@arm.com Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com
arch/arm64/kernel/ptrace.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 4b001121c72d..5534c175ceb7 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -2317,7 +2317,7 @@ enum ptrace_syscall_dir { PTRACE_SYSCALL_EXIT, };
This is now unused so it should be removed.
Sure.
In fact it is not unused, not sure how I missed that... r12/x7 is set to one of those values during ptrace_syscall_entry().
It would be a good idea to add a comment saying those values are part of the ABI and must not be changed.
- Kevin
On 25/11/2025 03:43, Jinjie Ruan wrote:
On 2025/11/24 23:23, Kevin Brodsky wrote:
On 24/11/2025 10:34, Jinjie Ruan wrote:
On 2025/11/19 1:13, Kevin Brodsky wrote:
On 17/11/2025 14:30, Jinjie Ruan wrote:
[...]
diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c index 66e6ba7fa80c..27310e611567 100644 --- a/kernel/entry/syscall-common.c +++ b/kernel/entry/syscall-common.c @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) } } +/**
- arch_ptrace_report_syscall_entry - Architecture specific
ptrace_report_syscall_entry().
- Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
- Defaults to ptrace_report_syscall_entry.
- The main purpose is to support arch-specific ptrace_report_syscall_entry()
- implementation.
- */
+static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
+#ifndef arch_ptrace_report_syscall_entry +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs) +{
- return ptrace_report_syscall_entry(regs);
I saw that Thomas suggested this approach on v4, and it makes sense to me, but I find the naming surprising. If an architecture does need extra handling, then the generic function should never be called from generic code. So it seems to me that the more logical change would be:
- Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
- Introduce ptrace_report_syscall_entry(), defaults to
__ptrace_report_syscall_entry()
If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an architecture also needs to redefine this function, but the architecture's own <asm/entry-common.h> must include <linux/ptrace.h>, the function will end up being defined twice and cause a "duplicate definition" compile error.
There's plenty of arch-defined functions in <linux/ptrace.h> already. __ptrace_report_syscall_entry() should be defined inside an #ifndef and architectures can define their own implementation in <asm/ptrace.h>, like force_successful_syscall_return() for instance.
Shared functions like ptrace_report_syscall() are all defined in <linux/ptrace.h>. When we want to override __ptrace_report_syscall_entry() in <asm/ptrace.h> we still have to include <linux/ptrace.h> again,then the redefine problem occurs again.
What we actually need to reuse is ptrace_report_syscall_entry() (or __ptrace_report_syscall_entry()).
You're right, this is yet another of those circular definition problems...
The arch version need to reuse and wrap ptrace_report_syscall_entry(), because for instance arm64 needs to perform additional operations before and after this step. Therefore, I believe the current implementation is appropriate.
I'm still not fond of arch_X() wrapping X() as this is unusual, but I don't have a better idea so let's stick to that. It also makes sense to have this done in syscall-common.c rather than a header considering the risk of circular dependency.
- Kevin
linux-kselftest-mirror@lists.linaro.org