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: <20240804034745.GA30236@1wt.eu>
Date: Sun, 4 Aug 2024 05:47:45 +0200
From: Willy Tarreau <w@....eu>
To: Thomas Weißschuh <thomas@...ch.de>
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

On Sat, Aug 03, 2024 at 10:55:07PM +0200, Thomas Weißschuh  wrote:
> Aug 3, 2024 20:33:11 Willy Tarreau <w@....eu>:
> 
> > On Sat, Aug 03, 2024 at 08:28:08PM +0200, Thomas Weißschuh  wrote:
> >>> 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 don't understand why. Above both are tested on the first line.
> > Am I missing something ?
> 
> This specifically does not work [0]:
> 
> a result, combining the two tests into a single expression as shown
> below would only be valid with a compiler that supports the operator but not
> with others that don't.

Ah I didn't remember about that one, thanks for the reference. Indeed
it's annoying then.

We have a similar construct in compiler.h:

  #if defined(__has_attribute)
  #  if __has_attribute(no_stack_protector)
  #    define __no_stack_protector __attribute__((no_stack_protector))
  #  else
  #    define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector")))
  #  endif
  #else
  #  define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector")))
  #endif /* defined(__has_attribute) */

Maybe it would be a good opportunity to have our own macro so as to
simplify such tests:

  #if defined(__has_attribute)
  #  define nolibc_has_attribute(x) __has_attribute(x)
  #else
  #  define nolibc_has_attribute(x) 0
  #endif

  #if nolibc_has_attribute(no_stack_protector)
  #  define __no_stack_protector __attribute__((no_stack_protector))
  #else
  #  define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector"
  #endif

Then:

  #if nolibc_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

It's as you want. Either we take your #undef-based solution or we take
this opportunity to clean up as above. I'm fine with both.

Thanks!
Willy