Messages in this thread |  | | Date | Tue, 25 Nov 2025 10:43:16 +0800 | | Subject | Re: [PATCH v7 08/11] entry: Add arch_ptrace_report_syscall_entry/exit() | | From | Jinjie Ruan <> |
| |
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 >>> >>>> [...] >
|  |