Re: [clang] stack protector and f1f029c7bf

[Sedat Dilek]

On Thu, May 24, 2018 at 10:26 PM, Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
[...]
>> Issue 2: ... The other option is to turn stack canary explicitly off for
> all such functions.
>
> We're looking to add the compiler attribute no_stack_protector.  It's added
> in mainline clang, the llvm bug cited earlier is about getting it
> backported into clang-6.0.1 release.  Sedat has tested/verified a set of
> patches to the kernel that use this new feature in:
> https://marc.info/?l=linux-kernel&m=152697630812366&w=2
>
Hi Nick,

sorry, if I was not clear/precise on this.
You referenced the patches from my 1st tryouts which were wrong in the
sense of "did-not-compile".
I have attached the correct two patches.
The commit-bodies needs some more "massage" to quote Thomas Gleixner,
useful web-links and credits should be added also.
I appreciate your help here.

>From my understanding...
The more correct approach in fixing the issue is to add Clang's
"no_stack_protector" function attribute support and mark
native_save_fl() accordingly.
The 2nd solution partly revert "x86: allow "=rm" in native_save_fl()"
is not favoured as the impact on other parts of the Linux-kernel are
not clear.
The 1st solution requires a Clang >= 7-svn331925.
Is that correct?

What does that mean in fixing the issue?
[ Linux-kernel side ]
I guess GCC and marking native_save_fl() should be OK?
After some rework, do you plan to push patches from the 1st solution
to Linus uptream?
[ LLVM/Clang side ]
What about backporting "no_stack_protector" to LLVM/Clang v6.0.1?

Thanks to all involved people.

Sunshiny greetings from North-West Germany,
- Sedat -
From c68cef9048e96af1211a57bd7a5f6ca6efdfc7b2 Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@xxxxxxxxxxx>
Date: Fri, 25 May 2018 09:40:07 +0200
Subject: [PATCH 1/2] compiler-clang.h: Add no_stack_protector function
 attribute support

From [1]:

"Clang supports the __attribute__((no_stack_protector)) attribute which
disables the stack protector on the specified function.
This attribute is useful for selectively disabling the stack protector
on some functions when building with -fstack-protector compiler option."

This is needed to mark native_save_fl() with __nostackprotector attribute to
fix a bug in the x86/paravirt/stackprotector area with Clang (see [2] and [3]).

NOTE: Clang-7 (>= svn331925) supports no_stack_protector function attribute.

[1] https://clang.llvm.org/docs/AttributeReference.html#no-stack-protector-clang-no-stack-protector-clang-no-stack-protector
[2] https://bugs.llvm.org/show_bug.cgi?id=37512
[3] https://github.com/ClangBuiltLinux/linux/issues/16
---
 include/linux/compiler-clang.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 070f85d92c15..5f49ff0bf7ee 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -21,3 +21,9 @@
 #ifdef __noretpoline
 #undef __noretpoline
 #endif
+
+/* Clang-7 (>= svn331925) supports no_stack_protector function attribute. */
+#ifdef __nostackprotector
+#undef __nostackprotector
+#define __nostackprotector __attribute__((no_stack_protector))
+#endif
-- 
2.17.0

From 2e1dc1599af6e599afd9eda70e9d2d819cc7df0c Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@xxxxxxxxxxx>
Date: Tue, 22 May 2018 12:07:24 +0200
Subject: [PATCH 2/2] x86/paravirt: Mark native_save_fl() with
 __nostackprotector attribute

For details please see [1] and [2].

This requires Clang-7 (>= svn331925) which supports __nostackprotector attribute.

[1] https://bugs.llvm.org/show_bug.cgi?id=37512
[2] https://github.com/ClangBuiltLinux/linux/issues/16
---
 arch/x86/include/asm/irqflags.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index 89f08955fff7..7e6765097adc 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -13,7 +13,7 @@
  * Interrupt control:
  */
 
-static inline unsigned long native_save_fl(void)
+static inline __nostackprotector unsigned long native_save_fl(void)
 {
 	unsigned long flags;
 
-- 
2.17.0