[libFuzzer] Fix stack overflow detection

[sebastianpoeplau]

Summary

Address sanitizer can detect stack exhaustion via its SEGV handler, which is
executed on a separate stack using the sigaltstack mechanism. When libFuzzer is
used with address sanitizer, it installs its own signal handlers which defer to
those put in place by the sanitizer before performing additional actions. In the
particular case of a stack overflow, the current setup fails because libFuzzer
doesn't preserve the flag for executing the signal handler on a separate stack:
when we run out of stack space, the operating system can't run the SEGV handler,
so address sanitizer never reports the issue. See the included test for an
example.

This commit fixes the issue by making libFuzzer preserve the SA_ONSTACK flag
when installing its signal handlers; the dedicated signal-handler stack set up
by the sanitizer runtime appears to be large enough to support the additional
frames from the fuzzer.

[morehouse]

Thanks for the fix. Could you add a test that fails before this fix and passes after it?

compiler-rt/lib/fuzzer/FuzzerUtilPosix.cpp

86

Nit: since we're touching this line, could you change 0 to nullptr?

[sebastianpoeplau]

In D101824#2737042, @morehouse wrote:

Thanks for the fix. Could you add a test that fails before this fix and passes after it?

Sure. Is there a better place for tests that combine ASan and libFuzzer than compiler-rt/test/asan?

[morehouse]

compiler-rt/test/fuzzer is better. Most tests there already build with ASan + libFuzzer.

[sebastianpoeplau]

Added a test and rephrased the commit message accordingly.

[sebastianpoeplau]

In D101824#2739110, @morehouse wrote:

compiler-rt/test/fuzzer is better. Most tests there already build with ASan + libFuzzer.

Done. The test fails as expected if you comment out the new SA_ONSTACK logic.

[morehouse]

LGTM

[sebastianpoeplau]

Looks like I made a mistake in the test that causes the Windows build to fail (just received an email notification): infinite_recursion should be declared with return type void. Should I submit a new differential, or is there some way to fix this one?

[morehouse]

Thanks for notifying. I've submitted https://reviews.llvm.org/rGf09414499c47 to address the Windows bot.

compiler-rt/lib/fuzzer/FuzzerUtilPosix.cpp

if (sigact.sa_sigaction) {
       upstream_segv_handler = sigact.sa_sigaction;
     }
   } else {
     if (sigact.sa_handler != SIG_DFL && sigact.sa_handler != SIG_IGN &&
         sigact.sa_handler != SIG_ERR)
       return;
   }
 
-  sigact = {};
-  sigact.sa_flags = SA_SIGINFO;
-  sigact.sa_sigaction = callback;
-  if (sigaction(signum, &sigact, 0)) {
+  struct sigaction new_sigact = {};
+  // Address sanitizer needs SA_ONSTACK (causing the signal handler to run on a
+  // dedicated stack) in order to be able to detect stack overflows; keep the
+  // flag if it's set.
+  new_sigact.sa_flags = SA_SIGINFO | (sigact.sa_flags & SA_ONSTACK);
+  new_sigact.sa_sigaction = callback;
+  if (sigaction(signum, &new_sigact, nullptr)) {
     Printf("libFuzzer: sigaction failed with %d\n", errno);
     exit(1);
   }
 }
 
 // Return true on success, false otherwise.
 bool ExecuteCommand(const Command &Cmd, std::string *CmdOutput) {
   FILE *Pipe = popen(Cmd.toString().c_str(), "r");

compiler-rt/test/fuzzer/StackOverflowTest.cpp

+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+// Stack overflow test for a fuzzer. The fuzzer must find the string "Hi" and
+// cause a stack overflow.
+#include <cstddef>
+#include <cstdint>
+
+volatile int x;
+volatile int y = 1;
+
+int infinite_recursion(char *p) {
+  char *buf = nullptr;
+
+  if (y)
+    infinite_recursion(buf);
+
+  x = 1;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
+  if (Size >= 2 && Data[0] == 'H' && Data[1] == 'i')
+    infinite_recursion(nullptr);
+  return 0;
+}

compiler-rt/test/fuzzer/stack-overflow-with-asan.test

+CHECK: SUMMARY: AddressSanitizer: stack-overflow
+RUN: %cpp_compiler %S/StackOverflowTest.cpp -o %t-StackOverflowTest
+RUN: not %run %t-StackOverflowTest 2>&1 | FileCheck %s