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 <[email protected]>
>> Signed-off-by: Jinjie Ruan <[email protected]>
>> ---
>> 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);
>> + 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);
>> }
>
