linux-kernel - [clang] stack protector and f1f029c7bf

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKwvOd=jmi_7LvZbS0PA5-zt62m5ZFPUNG7JtMQVmerCR63ebQ@mail.gmail.com>
Date:   Wed, 23 May 2018 15:08:19 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     hpa@...or.com
Cc:     Alistair Strachan <astrachan@...gle.com>,
        Manoj Gupta <manojgupta@...gle.com>,
        Matthias Kaehlcke <mka@...gle.com>,
        Greg Hackmann <ghackmann@...gle.com>, sedat.dilek@...il.com,
        tstellar@...hat.com, LKML <linux-kernel@...r.kernel.org>
Subject: [clang] stack protector and f1f029c7bf

H. Peter,

It was reported [0] that compiling the Linux kernel with Clang +
CC_STACKPROTECTOR_STRONG was causing a crash in native_save_fl(), due to
how GCC does not emit a stack guard for static inline functions (see
Alistair's excellent report in [1]) but Clang does.

When working with the LLVM release maintainers, Tom had suggested [2]
changing the inline assembly constraint in native_save_fl() from '=rm' to
'=r', and Alistair had verified the disassembly:

(good) code generated w/o -fstack-protector-strong:

native_save_fl:
          pushfq
          popq    -8(%rsp)
          movq    -8(%rsp), %rax
          retq

(good) code generated w/ =r input constraint:

native_save_fl:
          pushfq
          popq    %rax
          retq

(bad) code generated with -fstack-protector-strong:

native_save_fl:
          subq    $24, %rsp
          movq    %fs:40, %rax
          movq    %rax, 16(%rsp)
          pushfq
          popq    8(%rsp)
          movq    8(%rsp), %rax
          movq    %fs:40, %rcx
          cmpq    16(%rsp), %rcx
          jne     .LBB0_2
          addq    $24, %rsp
          retq
.LBB0_2:
          callq   __stack_chk_fail

It looks like the sugguestion is actually a revert of your commit:
ab94fcf528d127fcb490175512a8910f37e5b346:
x86: allow "=rm" in native_save_fl()

It seemed like there was a question internally about why worry about pop
adjusting the stack if the stack could be avoided altogether.

I think Sedat can retest this, but I was curious as well about the commit
message in ab94fcf528d: "[ Impact: performance ]", but Alistair's analysis
of the disassembly seems to indicate there is no performance impact (in
fact, looks better as there's one less mov).

Is there a reason we should not revert ab94fcf528d12, or maybe a better
approach?

[0] https://lkml.org/lkml/2018/5/7/534
[1] https://bugs.llvm.org/show_bug.cgi?id=37512#c15
[2] https://bugs.llvm.org/show_bug.cgi?id=37512#c22
-- 
Thanks,
~Nick Desaulniers