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

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

Jinjie Ruan posted 11 patches 2 weeks ago
There is a newer version of this series
[PATCH v7 05/11] arm64/ptrace: Handle ptrace_report_syscall_entry() error
Posted by Jinjie Ruan 2 weeks ago 
The generic entry handle error of ptrace_report_syscall_entry(), but
arm64 not.

As the comment said, the calling arch code should abort the system
call and must prevent normal entry so no system call is
made if ptrace_report_syscall_entry() return nonzero.

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);
+
 	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;
 	}
 
-- 
2.34.1
Re: [PATCH v7 05/11] arm64/ptrace: Handle ptrace_report_syscall_entry() error
Posted by Kevin Brodsky 1 week, 6 days ago 
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.

> As the comment said, the calling arch code should abort the system

Which comment?

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

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

- 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;
>  	}
>
Re: [PATCH v7 05/11] arm64/ptrace: Handle ptrace_report_syscall_entry() error
Posted by Jinjie Ruan 1 week, 3 days ago 

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;
>>  	}
>>  
>
Re: [PATCH v7 05/11] arm64/ptrace: Handle ptrace_report_syscall_entry() error
Posted by Kevin Brodsky 1 week ago 
On 21/11/2025 05:15, Jinjie Ruan wrote:
>
> 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()

I found that later, this should be clarified in the commit message.

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

My point is that this patch is not changing this - arm64 was already
skipping the syscall if ptrace_report_syscall_entry() returns an error.

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

Calling forget_syscall() means setting regs->syscallno to NO_SYSCALL. It
is indeed no longer required for the entry sequence, but it does have
visible side effects. For instance, regs->syscallno can be inspected via
ptrace(PTRACE_GETREGSET, REGSET_SYSTEM_CALL). So the question is whether
we need to deviate from the generic path on arm64 (this is mostly a
question for arm64 maintainers).

- Kevin

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.