Re: [PATCH v7 05/11] arm64/ptrace: Handle ptrace_report_syscall_entry() error

[Jinjie Ruan <ruanjinjie@huawei.com>]

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;
>>  	}
>>  
>