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 <[email protected]>
>> ---
>> 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;
>> }
>>
>
