[PATCH v7 03/11] arm64/ptrace: Refator el0_svc_common()

Jinjie Ruan posted 11 patches 2 weeks ago
There is a newer version of this series
[PATCH v7 03/11] arm64/ptrace: Refator el0_svc_common()
Posted by Jinjie Ruan 2 weeks ago
Compared to the generic entry code, arm64 terminate the process
after report_syscall_exit() if the syscall is issued within
a restartable sequence.

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@huawei.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);
 }
 
 /*
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;
+		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)
-- 
2.34.1
Re: [PATCH v7 03/11] arm64/ptrace: Refator el0_svc_common()
Posted by Kevin Brodsky 1 week, 6 days ago
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)

> 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.

> 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@huawei.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.

>  }
>  
>  /*
> 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.

- 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)
Re: [PATCH v7 03/11] arm64/ptrace: Refator el0_svc_common()
Posted by Jinjie Ruan 1 week, 4 days ago

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@huawei.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)
>