linux-kernel - Re: [PATCH v7 08/11] entry: Add arch_ptrace_report_syscall

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: <1a8034b2-bb8f-4e73-b205-11453df6d02b@arm.com>
Date: Tue, 25 Nov 2025 14:10:49 +0100
From: Kevin Brodsky <kevin.brodsky@....com>
To: Jinjie Ruan <ruanjinjie@...wei.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 08/11] entry: Add
 arch_ptrace_report_syscall_entry/exit()

On 25/11/2025 03:43, Jinjie Ruan wrote:
>
> On 2025/11/24 23:23, Kevin Brodsky wrote:
>> On 24/11/2025 10:34, Jinjie Ruan wrote:
>>> On 2025/11/19 1:13, Kevin Brodsky wrote:
>>>> On 17/11/2025 14:30, Jinjie Ruan wrote:
>>>>> [...]
>>>>>
>>>>> diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c
>>>>> index 66e6ba7fa80c..27310e611567 100644
>>>>> --- a/kernel/entry/syscall-common.c
>>>>> +++ b/kernel/entry/syscall-common.c
>>>>> @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
>>>>>  	}
>>>>>  }
>>>>>  
>>>>> +/**
>>>>> + * arch_ptrace_report_syscall_entry - Architecture specific
>>>>> + *				      ptrace_report_syscall_entry().
>>>>> + *
>>>>> + * Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
>>>>> + * Defaults to ptrace_report_syscall_entry.
>>>>> + *
>>>>> + * The main purpose is to support arch-specific ptrace_report_syscall_entry()
>>>>> + * implementation.
>>>>> + */
>>>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
>>>>> +
>>>>> +#ifndef arch_ptrace_report_syscall_entry
>>>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs)
>>>>> +{
>>>>> +	return ptrace_report_syscall_entry(regs);
>>>> I saw that Thomas suggested this approach on v4, and it makes sense to
>>>> me, but I find the naming surprising. If an architecture does need extra
>>>> handling, then the generic function should never be called from generic
>>>> code. So it seems to me that the more logical change would be:
>>>>
>>>> * Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
>>>> * Introduce ptrace_report_syscall_entry(), defaults to
>>>> __ptrace_report_syscall_entry()
>>> If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an
>>> architecture also needs to redefine this function, but the
>>> architecture's own <asm/entry-common.h> must include <linux/ptrace.h>,
>>> the function will end up being defined twice and cause a "duplicate
>>> definition" compile error.
>> There's plenty of arch-defined functions in <linux/ptrace.h> already.
>> __ptrace_report_syscall_entry() should be defined inside an #ifndef and
>> architectures can define their own implementation in <asm/ptrace.h>,
>> like force_successful_syscall_return() for instance.
> Shared functions like ptrace_report_syscall() are all defined in
> <linux/ptrace.h>.
> When we want to override __ptrace_report_syscall_entry() in
> <asm/ptrace.h> we still have to include <linux/ptrace.h> again,then the
> redefine problem occurs again.
>
> What we actually need to reuse is ptrace_report_syscall_entry() (or
> __ptrace_report_syscall_entry()).

You're right, this is yet another of those circular definition problems...

> The arch version need to reuse and wrap ptrace_report_syscall_entry(),
> because for instance arm64 needs to perform additional operations before
> and after this step. Therefore, I believe the current implementation is
> appropriate.

I'm still not fond of arch_X() wrapping X() as this is unusual, but I
don't have a better idea so let's stick to that. It also makes sense to
have this done in syscall-common.c rather than a header considering the
risk of circular dependency.

- Kevin