1. Patchew
  2. linux
  3. [PATCH v7 00/11] arm64: entry: Convert to Generic Entry
  4. View patch

[PATCH v7 01/11] arm64/ptrace: Split report_syscall()

Jinjie Ruan posted 11 patches 2 weeks ago
There is a newer version of this series
[PATCH v7 01/11] arm64/ptrace: Split report_syscall()
Posted by Jinjie Ruan 2 weeks ago 
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@arm.com>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.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,
 };
 
-static void report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir)
+static void report_syscall_enter(struct pt_regs *regs)
 {
 	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);
+	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);
 }
-- 
2.34.1
Re: [PATCH v7 01/11] arm64/ptrace: Split report_syscall()
Posted by Kevin Brodsky 1 week, 6 days ago 
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@arm.com>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.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.

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

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

- 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);
>  }
Re: [PATCH v7 01/11] arm64/ptrace: Split report_syscall()
Posted by Jinjie Ruan 1 week, 5 days ago 

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@arm.com>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.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);
>>  }
>
Re: [PATCH v7 01/11] arm64/ptrace: Split report_syscall()
Posted by Kevin Brodsky 6 days, 13 hours ago 
On 19/11/2025 10:49, Jinjie Ruan wrote:
>
> 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@arm.com>
>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.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.

In fact it is not unused, not sure how I missed that... r12/x7 is set to
one of those values during ptrace_syscall_entry().

It would be a good idea to add a comment saying those values are part of
the ABI and must not be changed.

- Kevin

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.