Re: enable -fstack-clash-protection in llvm ports
Le Thu, Sep 14, 2023 at 05:53:53PM +0200, Mark Kettenis a écrit : > > From: Theo de Raadt > > Date: Thu, 14 Sep 2023 01:02:14 -0600 (MDT) > > > > I do not think this should be enabled. > > Our stacks work differently. > > We don't put shit near the bottom of the main stack, because we > > reserve the space. > > For pthread stacks, we allocate them randomly also so you cannot > > determistically trash a specific object. > > This change also make very small stacks (meaning pthreads) potentially > > reach beyond the bottom in a weird new way. > > We simply don't need to cpu and pte's for this. > > But if the stack space is used we'll have to spend the CPU time to > fault the pages in and allocate the PTEs anyway. Only stupid code > allocates large amounts of stack space and doesn't use it! > > Now with -fstack-clash-protection, there will be a few additional > loads and the access pattern will be slightly different and there will > be slightly different. But if firefox enables this the performance > impact must be really, really small. > > As far as I understand, the proposed change would only make the option > available, but it would still be turned off by default. So only ports > that explicitly turn this option on would be affected. i have no desire to die on this hill, so i've had upstream mozilla drop the -fstack-clash-protection option from CFLAGS when on OpenBSD. Landry
Re: enable -fstack-clash-protection in llvm ports
Mark Kettenis wrote: > > From: Theo de Raadt > > Date: Thu, 14 Sep 2023 01:02:14 -0600 (MDT) > > > > I do not think this should be enabled. > > Our stacks work differently. > > We don't put shit near the bottom of the main stack, because we > > reserve the space. > > For pthread stacks, we allocate them randomly also so you cannot > > determistically trash a specific object. > > This change also make very small stacks (meaning pthreads) potentially > > reach beyond the bottom in a weird new way. > > We simply don't need to cpu and pte's for this. > > But if the stack space is used we'll have to spend the CPU time to > fault the pages in and allocate the PTEs anyway. Only stupid code > allocates large amounts of stack space and doesn't use it! That is not correct. Sometimes it prefetches a page which you will use. Sometimes it prefetches a page you won't use. Quite often you don't use it. Then the page has to eventually expire. A PTE is also loaded. It's a waste of time there are no pages to clash against. We do not have known objects to collide against. We have address space randomization. We don't have some "limited" form of it which puts other objects in those locations.
Re: enable -fstack-clash-protection in llvm ports
> From: Theo de Raadt > Date: Thu, 14 Sep 2023 01:02:14 -0600 (MDT) > > I do not think this should be enabled. > Our stacks work differently. > We don't put shit near the bottom of the main stack, because we > reserve the space. > For pthread stacks, we allocate them randomly also so you cannot > determistically trash a specific object. > This change also make very small stacks (meaning pthreads) potentially > reach beyond the bottom in a weird new way. > We simply don't need to cpu and pte's for this. But if the stack space is used we'll have to spend the CPU time to fault the pages in and allocate the PTEs anyway. Only stupid code allocates large amounts of stack space and doesn't use it! Now with -fstack-clash-protection, there will be a few additional loads and the access pattern will be slightly different and there will be slightly different. But if firefox enables this the performance impact must be really, really small. As far as I understand, the proposed change would only make the option available, but it would still be turned off by default. So only ports that explicitly turn this option on would be affected.
Re: enable -fstack-clash-protection in llvm ports
I do not think this should be enabled. Our stacks work differently. We don't put shit near the bottom of the main stack, because we reserve the space. For pthread stacks, we allocate them randomly also so you cannot determistically trash a specific object. This change also make very small stacks (meaning pthreads) potentially reach beyond the bottom in a weird new way. We simply don't need to cpu and pte's for this.
enable -fstack-clash-protection in llvm ports
hi,
currently building mozillas, i get tons of this on stderr:
warning: clang-13: warning: argument unused during compilation:
'-fstack-clash-protection' [-Wunused-command-line-argument]
filed an upstream issue about it
(https://bugzilla.mozilla.org/show_bug.cgi?id=1852202) to realize its an
llvm feature that was originally added in llvm 11
(https://reviews.llvm.org/D68720), but only enabled on
linux, and later on (14 or 15) in FreeBSD
(https://github.com/llvm/llvm-project/blob/b0ea2790c41db65b3c283f78a5f534bc26fc6f8f/clang/lib/Driver/ToolChains/Clang.cpp#L3476)
- according to
https://reviews.llvm.org/D68720#2415629 it should have been enabled
everywhere. In mozilla-land of course the flag is added if clang > 11 is
found on non-osx/non-windows, which includes OpenBSD, which triggers the
-Wunused-command-line-argument warning.
it's to protect against
https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt
the attached patch enables it in both ports llvm flavors. Been able to
build firefox with way less noise on stderr with that.
ofc, this can/should also be enabled in base llvm.
Landry
Index: 13/Makefile
===
RCS file: /cvs/ports/devel/llvm/13/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- 13/Makefile 8 Sep 2023 14:11:33 - 1.6
+++ 13/Makefile 11 Sep 2023 06:44:34 -
@@ -2,7 +2,7 @@ LLVM_MAJOR =13
LLVM_VERSION = ${LLVM_MAJOR}.0.0
LLVM_PKGSPEC = >=13,<14
-REVISION-main =11
+REVISION-main =12
REVISION-lldb =4
REVISION-python = 3
Index: 13/patches/patch-clang_lib_Driver_ToolChains_Clang_cpp
===
RCS file:
/cvs/ports/devel/llvm/13/patches/patch-clang_lib_Driver_ToolChains_Clang_cpp,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 patch-clang_lib_Driver_ToolChains_Clang_cpp
--- 13/patches/patch-clang_lib_Driver_ToolChains_Clang_cpp 3 Sep 2023
15:59:27 - 1.1.1.1
+++ 13/patches/patch-clang_lib_Driver_ToolChains_Clang_cpp 11 Sep 2023
06:44:34 -
@@ -39,6 +39,7 @@
- Turn on pointer-authentication on arm64 as well by default. This means
we effectively enable -mbranch-protection=standard on arm64 now.
- Make sure -msign-return-address doesn't disable BTI support.
+- Enable -fstack-clash-protection on OpenBSD
Index: clang/lib/Driver/ToolChains/Clang.cpp
--- clang/lib/Driver/ToolChains/Clang.cpp.orig
@@ -80,6 +81,15 @@ Index: clang/lib/Driver/ToolChains/Clang
MipsTargetFeature = llvm::StringSwitch(Value)
.Case("-mips1", "+mips1")
+@@ -3180,7 +3194,7 @@ static void RenderSCPOptions(const ToolChain &TC, cons
+ ArgStringList &CmdArgs) {
+ const llvm::Triple &EffectiveTriple = TC.getEffectiveTriple();
+
+- if (!EffectiveTriple.isOSLinux())
++ if (!EffectiveTriple.isOSOpenBSD() && !EffectiveTriple.isOSLinux())
+ return;
+
+ if (!EffectiveTriple.isX86() && !EffectiveTriple.isSystemZ() &&
@@ -4943,9 +4957,12 @@ void Clang::ConstructJob(Compilation &C, const JobActi
OFastEnabled ? options::OPT_Ofast : options::OPT_fstrict_aliasing;
// We turn strict aliasing off by default if we're in CL mode, since MSVC
Index: 16/Makefile
===
RCS file: /cvs/ports/devel/llvm/16/Makefile,v
retrieving revision 1.7
diff -u -p -r1.7 Makefile
--- 16/Makefile 8 Sep 2023 14:11:33 - 1.7
+++ 16/Makefile 11 Sep 2023 06:44:34 -
@@ -2,7 +2,7 @@ LLVM_MAJOR =16
LLVM_VERSION = ${LLVM_MAJOR}.0.6
LLVM_PKGSPEC = >=16,<17
-REVISION-main =5
+REVISION-main =6
REVISION-lldb =1
REVISION-python = 0
Index: 16/patches/patch-clang_lib_Driver_ToolChains_Clang_cpp
===
RCS file:
/cvs/ports/devel/llvm/16/patches/patch-clang_lib_Driver_ToolChains_Clang_cpp,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 patch-clang_lib_Driver_ToolChains_Clang_cpp
--- 16/patches/patch-clang_lib_Driver_ToolChains_Clang_cpp 3 Sep 2023
16:00:03 - 1.1.1.1
+++ 16/patches/patch-clang_lib_Driver_ToolChains_Clang_cpp 11 Sep 2023
06:44:34 -
@@ -27,7 +27,17 @@ Index: clang/lib/Driver/ToolChains/Clang
MipsTargetFeature = llvm::StringSwitch(Value)
.Case("-mips1", "+mips1")
-@@ -5289,9 +5301,12 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+@@ -3363,7 +3375,8 @@ static void RenderSCPOptions(const ToolChain &TC, cons
+ ArgStringList &CmdArgs) {
+ const llvm::Triple &EffectiveTriple = TC.getEffectiveTriple();
+
+- if (!EffectiveTriple.isOSFreeBSD() && !EffectiveTriple.isOSLinux())
++ if (!EffectiveTriple.isOSFreeBSD() && !EffectiveTriple.isOSOpenBSD() &&
++ !EffectiveTriple.isOSLinux())
+ return;
+
+ if (
