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: <7b317981-659c-fa02-d569-f832bfc09c9c@huawei.com>
Date: Thu, 20 Nov 2025 20:03: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 03/11] arm64/ptrace: Refator el0_svc_common()



On 2025/11/19 1:10, Kevin Brodsky wrote:
> In subject: s/refator/refactor/
> 
> Also nit: this is less about ptrace than about syscall entry, so maybe
> "arm64/syscall:"?
> 
> On 17/11/2025 14:30, Jinjie Ruan wrote:
>> Compared to the generic entry code, arm64 terminate the process
> 
> s/terminate/terminates/ (similar observation for other commit messages)

Will update it.

> 
>> after report_syscall_exit() if the syscall is issued within
>> a restartable sequence.
> 
> Presumably this implies that forcing SIGSEGV before or after calling the
> tracing handlers makes no difference, if so it should be clarified.

It seems that it need a separate patch to clarify.

> 
>> In preparation for moving arm64 over to the generic entry code,
>> refactor el0_svc_common() as below:
>>
>> - Extract syscall_exit_to_user_mode_prepare() to replace the
>>   the combination of read_thread_flags() and syscall_trace_exit(),
>>   also move the syscall exit check logic into it.
>>
>> - Move rseq_syscall() ahead, so the CONFIG_DEBUG_RSEQ check is
>>   not needed.
>>
>> - Move has_syscall_work() helper into asm/syscall.h, which will be
>>   reused by ptrace.c.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@...wei.com>
>> ---
>>  arch/arm64/include/asm/syscall.h |  7 ++++++-
>>  arch/arm64/kernel/ptrace.c       | 10 +++++++++-
>>  arch/arm64/kernel/syscall.c      | 26 +++++---------------------
>>  3 files changed, 20 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
>> index d69f590a989b..6225981fbbdb 100644
>> --- a/arch/arm64/include/asm/syscall.h
>> +++ b/arch/arm64/include/asm/syscall.h
>> @@ -114,7 +114,12 @@ static inline int syscall_get_arch(struct task_struct *task)
>>  	return AUDIT_ARCH_AARCH64;
>>  }
>>  
>> +static inline bool has_syscall_work(unsigned long flags)
>> +{
>> +	return unlikely(flags & _TIF_SYSCALL_WORK);
>> +}
>> +
>>  int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags);
>> -void syscall_trace_exit(struct pt_regs *regs, unsigned long flags);
>> +void syscall_exit_to_user_mode_prepare(struct pt_regs *regs);
>>  
>>  #endif	/* __ASM_SYSCALL_H */
>> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
>> index bbb868f6b292..95984bbf53db 100644
>> --- a/arch/arm64/kernel/ptrace.c
>> +++ b/arch/arm64/kernel/ptrace.c
>> @@ -2403,7 +2403,7 @@ int syscall_trace_enter(struct pt_regs *regs, long syscall, unsigned long flags)
>>  	return regs->syscallno;
>>  }
>>  
>> -void syscall_trace_exit(struct pt_regs *regs, unsigned long flags)
>> +static void syscall_trace_exit(struct pt_regs *regs, unsigned long flags)
>>  {
>>  	audit_syscall_exit(regs);
>>  
>> @@ -2412,8 +2412,16 @@ void syscall_trace_exit(struct pt_regs *regs, unsigned long flags)
>>  
>>  	if (flags & (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP))
>>  		report_syscall_exit(regs);
>> +}
>> +
>> +void syscall_exit_to_user_mode_prepare(struct pt_regs *regs)
>> +{
>> +	unsigned long flags = read_thread_flags();
>>  
>>  	rseq_syscall(regs);
>> +
>> +	if (has_syscall_work(flags) || flags & _TIF_SINGLESTEP)
>> +		syscall_trace_exit(regs, flags);
> 
> Since we're trying to align as much with the generic code as possible,
> we might as well rename this function to syscall_exit_work() as well.

Yes, syscall_trace_exit() is very similar to syscall_exit_work().

> 
>>  }
>>  
>>  /*
>> diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
>> index 6e3fe760e0bb..9713b038d750 100644
>> --- a/arch/arm64/kernel/syscall.c
>> +++ b/arch/arm64/kernel/syscall.c
>> @@ -65,11 +65,6 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno,
>>  	choose_random_kstack_offset(get_random_u16());
>>  }
>>  
>> -static inline bool has_syscall_work(unsigned long flags)
>> -{
>> -	return unlikely(flags & _TIF_SYSCALL_WORK);
>> -}
>> -
>>  static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
>>  			   const syscall_fn_t syscall_table[])
>>  {
>> @@ -125,26 +120,15 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
>>  		if (scno == NO_SYSCALL)
>>  			syscall_set_return_value(current, regs, -ENOSYS, 0);
>>  		scno = syscall_trace_enter(regs, regs->syscallno, flags);
>> -		if (scno == NO_SYSCALL)
>> -			goto trace_exit;
> 
> Why not keep the goto instead of duplicating the call? It could be
> renamed if necessary.

That is good.

> 
> - Kevin
> 
>> +		if (scno == NO_SYSCALL) {
>> +			syscall_exit_to_user_mode_prepare(regs);
>> +			return;
>> +		}
>>  	}
>>  
>>  	invoke_syscall(regs, scno, sc_nr, syscall_table);
>>  
>> -	/*
>> -	 * The tracing status may have changed under our feet, so we have to
>> -	 * check again. However, if we were tracing entry, then we always trace
>> -	 * exit regardless, as the old entry assembly did.
>> -	 */
>> -	if (!has_syscall_work(flags) && !IS_ENABLED(CONFIG_DEBUG_RSEQ)) {
>> -		flags = read_thread_flags();
>> -		if (!has_syscall_work(flags) && !(flags & _TIF_SINGLESTEP))
>> -			return;
>> -	}
>> -
>> -trace_exit:
>> -	flags = read_thread_flags();
>> -	syscall_trace_exit(regs, flags);
>> +	syscall_exit_to_user_mode_prepare(regs);
>>  }
>>  
>>  void do_el0_svc(struct pt_regs *regs)
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ