linux-kernel - Re: [PATCH v7 01/11] arm64/ptrace: Split report

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9adaa51a-09d1-8450-afb0-838ace0f28e9@huawei.com>
Date: Wed, 19 Nov 2025 17:49:48 +0800
From: Jinjie Ruan <ruanjinjie@...wei.com>
To: Kevin Brodsky <kevin.brodsky@....com>, <catalin.marinas@....com>,
	<will@...nel.org>, <oleg@...hat.com>, <tglx@...utronix.de>,
	<peterz@...radead.org>, <luto@...nel.org>, <shuah@...nel.org>,
	<kees@...nel.org>, <wad@...omium.org>, <akpm@...ux-foundation.org>,
	<ldv@...ace.io>, <macro@...am.me.uk>, <deller@....de>,
	<mark.rutland@....com>, <song@...nel.org>, <mbenes@...e.cz>,
	<ryan.roberts@....com>, <ada.coupriediaz@....com>,
	<anshuman.khandual@....com>, <broonie@...nel.org>, <pengcan@...inos.cn>,
	<dvyukov@...gle.com>, <kmal@...k.li>, <lihongbo22@...wei.com>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v7 01/11] arm64/ptrace: Split report_syscall()



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@....com>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@...wei.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);
>> +		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);
>>  }
>