linux-kernel - Re: [PATCH 04/12] tools/nolibc: use attribute((naked)) if available

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <713abd5e-1f72-4cf8-9857-c6795b4b3187@t-8ch.de>
Date: Sat, 3 Aug 2024 20:28:08 +0200 (GMT+02:00)
From: Thomas Weißschuh  <thomas@...ch.de>
To: Willy Tarreau <w@....eu>
Cc: Thomas Weißschuh <linux@...ssschuh.net>,
	Shuah Khan <shuah@...nel.org>, linux-kernel@...r.kernel.org,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 04/12] tools/nolibc: use attribute((naked)) if available

Aug 3, 2024 11:26:07 Willy Tarreau <w@....eu>:

> On Sun, Jul 28, 2024 at 12:09:58PM +0200, Thomas Weißschuh wrote:
>> The current entrypoint attributes optimize("Os", "omit-frame-pointer")
>> are intended to avoid all compiler generated code, like function
>> porologue and epilogue.
>> This is the exact usecase implemented by the attribute "naked".
>>
>> Unfortunately this is not implemented by GCC for all targets,
>> so only use it where available.
>> This also provides compatibility with clang, which recognizes the
>> "naked" attribute but not the previously used attribute "optimized".
>>
>> Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
>> ---
>> tools/include/nolibc/compiler.h | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h
>> index fe3701863634..f77bb7d3e1a8 100644
>> --- a/tools/include/nolibc/compiler.h
>> +++ b/tools/include/nolibc/compiler.h
>> @@ -9,6 +9,15 @@
>> #define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
>> #define __entrypoint_epilogue() __builtin_unreachable()
>>
>> +#if defined(__has_attribute)
>> +#  if __has_attribute(naked)
>> +#    undef  __entrypoint
>> +#    define __entrypoint __attribute__((naked))
>> +#    undef __entrypoint_epilogue
>> +#    define __entrypoint_epilogue()
>> +#  endif
>> +#endif /* defined(__has_attribute) */
>
> I would find it cleaner to enclose the previous declaration with the #if
> rather than #undef everything just after it has been defined. Also it's
> not very common to undo declarations just after they've been done, and
> it makes quick code analysis harder.
>
> I think that it can resolve to roughly this:
>
> #if defined(__has_attribute) && __has_attribute(naked)
> #  define __entrypoint __attribute__((naked))
> #  define __entrypoint_epilogue()
> #else
> #  define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
> #  define __entrypoint_epilogue() __builtin_unreachable()
> #endif

We would need to duplicate the define for the
!defined(__has_attribute) case.
I wanted to avoid that duplication.

> What do you think ?

With the reasoning above I'll let you choose.