lkml.org 
[lkml]   [2018]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [clang] stack protector and f1f029c7bf
From
On May 24, 2018 12:49:56 PM PDT, Tom Stellard <tstellar@redhat.com> wrote:
>On 05/24/2018 11:19 AM, hpa@zytor.com wrote:
>> On May 23, 2018 3:08:19 PM PDT, Nick Desaulniers
><ndesaulniers@google.com> wrote:
>>> 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
>>
>> A stack canary on an *inlined* function? That's bound to break things
>elsewhere too sooner or later.
>>
>> It feels like *once again* clang is asking for the Linux kernel to
>change to paper over technical or compatibility problems in clang/LLVM.
>This is not exactly helping the feeling that we should just rip out any
>and all clang hacks and call it a loss.
>>
>
>In this case this fix is working-around a bug in the kernel. The
>problem
>here is that the caller of native_save_fl() assumes that it won't
>clobber any of the general purpose register even though the function
>is defined as a normal c function which tells the compiler that it's
>OK to clobber the registers.
>
>This is something that really needs to be fixed in the kernel. Relying
>on heuristics of internal compiler algorithms in order for code to work
>correctly is always going to lead to problems like this.
>
>-Tom

Ok, yet another problem (this time with paravirtualization, no surprise there.). Good, let's find the actual problems and fix them where they should be fixed.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

\
 
 \ /
  Last update: 2018-05-24 23:34    [W:0.125 / U:1.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site