diff options
author | Nick Desaulniers <ndesaulniers@google.com> | 2023-04-13 09:30:19 -0700 |
---|---|---|
committer | Tobias Hieta <tobias@hieta.se> | 2023-04-17 16:26:07 +0200 |
commit | bf80902fdd43eed5b7a4c401bccc0a06ceac582c (patch) | |
tree | 62caebca610db76dfc71f8959cb756394ca0a78a | |
parent | 82432ac5cc7d824328a9b59d73cdbd3ca1e9a07d (diff) | |
download | llvm-bf80902fdd43eed5b7a4c401bccc0a06ceac582c.tar.gz |
[StackProtector] don't check stack protector before calling nounwind functions
https://reviews.llvm.org/rGd656ae28095726830f9beb8dbd4d69f5144ef821
introduced a additional checks before calling noreturn functions in
response to this security paper related to Catch Handler Oriented
Programming (CHOP):
https://download.vusec.net/papers/chop_ndss23.pdf
See also:
https://bugs.chromium.org/p/llvm/issues/detail?id=30
This causes stack canaries to be inserted in C code which was
unexpected; we noticed certain Linux kernel trees stopped booting after
this (in functions trying to initialize the stack canary itself).
https://github.com/ClangBuiltLinux/linux/issues/1815
There is no point checking the stack canary like this when exceptions
are disabled (-fno-exceptions or function is marked noexcept) or for C
code. The GCC patch for this issue does something similar:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=a25982ada523689c8745d7fb4b1b93c8f5dab2e7
Android measured a 2% regression in RSS as a result of d656ae280957 and
undid it globally:
https://android-review.googlesource.com/c/platform/build/soong/+/2524336
Reviewed By: xiangzhangllvm
Differential Revision: https://reviews.llvm.org/D147975
(cherry picked from commit fc4494dffa5422b2be5442c235554e76bed79c8a)
-rw-r--r-- | llvm/lib/CodeGen/StackProtector.cpp | 21 | ||||
-rw-r--r-- | llvm/test/CodeGen/X86/stack-protector-2.ll | 12 | ||||
-rw-r--r-- | llvm/test/CodeGen/X86/stack-protector-recursively.ll | 7 |
3 files changed, 23 insertions, 17 deletions
diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp index 46685f7b8208..c5cf6ae6578b 100644 --- a/llvm/lib/CodeGen/StackProtector.cpp +++ b/llvm/lib/CodeGen/StackProtector.cpp @@ -455,18 +455,15 @@ bool StackProtector::InsertStackProtectors() { if (&BB == FailBB) continue; Instruction *CheckLoc = dyn_cast<ReturnInst>(BB.getTerminator()); - if (!CheckLoc && !DisableCheckNoReturn) { - for (auto &Inst : BB) { - auto *CB = dyn_cast<CallBase>(&Inst); - if (!CB) - continue; - if (!CB->doesNotReturn()) - continue; - // Do stack check before non-return calls (e.g: __cxa_throw) - CheckLoc = CB; - break; - } - } + if (!CheckLoc && !DisableCheckNoReturn) + for (auto &Inst : BB) + if (auto *CB = dyn_cast<CallBase>(&Inst)) + // Do stack check before noreturn calls that aren't nounwind (e.g: + // __cxa_throw). + if (CB->doesNotReturn() && !CB->doesNotThrow()) { + CheckLoc = CB; + break; + } if (!CheckLoc) continue; diff --git a/llvm/test/CodeGen/X86/stack-protector-2.ll b/llvm/test/CodeGen/X86/stack-protector-2.ll index 14909520f914..bd6998171475 100644 --- a/llvm/test/CodeGen/X86/stack-protector-2.ll +++ b/llvm/test/CodeGen/X86/stack-protector-2.ll @@ -1,4 +1,5 @@ -; RUN: llc -mtriple=x86_64-pc-linux-gnu -start-before=stack-protector -stop-after=stack-protector -o - < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-pc-linux-gnu -start-before=stack-protector \ +; RUN: -stop-after=stack-protector -o - < %s | FileCheck %s ; Bugs 42238/43308: Test some additional situations not caught previously. define void @store_captures() #0 { @@ -219,5 +220,14 @@ return: ; preds = %if.end, %if.then ret i32 0 } +declare void @callee() noreturn nounwind +define void @caller() sspstrong { +; Test that a stack protector is NOT inserted when we call nounwind functions. +; CHECK-LABEL: @caller +; CHECK-NEXT: call void @callee + call void @callee() noreturn nounwind + ret void +} + attributes #0 = { sspstrong } attributes #1 = { noreturn sspreq} diff --git a/llvm/test/CodeGen/X86/stack-protector-recursively.ll b/llvm/test/CodeGen/X86/stack-protector-recursively.ll index 383af168de77..ad7af3f302a6 100644 --- a/llvm/test/CodeGen/X86/stack-protector-recursively.ll +++ b/llvm/test/CodeGen/X86/stack-protector-recursively.ll @@ -12,15 +12,14 @@ define dso_local void @__stack_chk_fail() local_unnamed_addr #0 { ; CHECK-NEXT: cmpq (%rsp), %rax ; CHECK-NEXT: jne .LBB0_2 ; CHECK-NEXT: # %bb.1: # %SP_return -; CHECK-NEXT: ud2 +; CHECK-NEXT: callq foo@PLT ; CHECK-NEXT: .LBB0_2: # %CallStackCheckFailBlk ; CHECK-NEXT: callq __stack_chk_fail entry: - tail call void @llvm.trap() + tail call void @foo() noreturn unreachable } -declare void @llvm.trap() #1 +declare void @foo() noreturn attributes #0 = { noreturn nounwind sspreq } -attributes #1 = { noreturn nounwind } |