Fix ASAN integration with detect_stack_use_after_return turned on

Summary: detect_stack_use_after_return enables fake stack logic. Previous implementation was relying on fiber releasing its fake stack before switch, which is not safe, because that fake stack may actually be in use. This diff implements a hacky workaround by building a freeFakeStack function, which can release fake stack outside of the fiber. Ideally this function should be provided by ASAN itself.

Reviewed By: yfeldblum

Differential Revision: D4631962

fbshipit-source-id: 6213e2a6b3cbc35fae3793fcf86b0614dd8ff825

diff --git a/folly/fibers/Fiber.cpp b/folly/fibers/Fiber.cpp
index 79866600e2b7575a866a007b211fa5f058ccb74d..39dcc3332983d1bde8e0867d05d53d2d1599c8f9 100644 (file)
--- a/folly/fibers/Fiber.cpp
+++ b/folly/fibers/Fiber.cpp
@@ -103,6 +103,9 @@ void Fiber::init(bool recordStackUsed) {
 
 Fiber::~Fiber() {
 #ifdef FOLLY_SANITIZE_ADDRESS
+  if (asanFakeStack_ != nullptr) {
+    fiberManager_.freeFakeStack(asanFakeStack_);
+  }
   fiberManager_.unpoisonFiberStack(this);
 #endif
   fiberManager_.stackAllocator_.deallocate(fiberStackLimit_, fiberStackSize_);

diff --git a/folly/fibers/FiberManager.cpp b/folly/fibers/FiberManager.cpp
index 2d17a4a34dff4f79ec8bf070c7ff16229d87c909..8a04bde1132245840bfb95f27d0e2974b538aff3 100644 (file)
--- a/folly/fibers/FiberManager.cpp
+++ b/folly/fibers/FiberManager.cpp
@@ -217,6 +217,22 @@ void FiberManager::registerFinishSwitchStackWithAsan(
   }
 }
 
+void FiberManager::freeFakeStack(void* fakeStack) {
+  static AsanStartSwitchStackFuncPtr fnStart = getStartSwitchStackFunc();
+  static AsanFinishSwitchStackFuncPtr fnFinish = getFinishSwitchStackFunc();
+  if (fnStart == nullptr || fnFinish == nullptr) {
+    LOG(FATAL) << "The version of ASAN in use doesn't support fibers";
+  }
+
+  void* saveFakeStack;
+  const void* stackBottom;
+  size_t stackSize;
+  fnStart(&saveFakeStack, nullptr, 0);
+  fnFinish(fakeStack, &stackBottom, &stackSize);
+  fnStart(nullptr, stackBottom, stackSize);
+  fnFinish(saveFakeStack, nullptr, nullptr);
+}
+
 void FiberManager::unpoisonFiberStack(const Fiber* fiber) {
   auto stack = fiber->getStack();
 

diff --git a/folly/fibers/FiberManagerInternal-inl.h b/folly/fibers/FiberManagerInternal-inl.h
index 89b87f08332f187650bcc9e34b0929b720c4e91f..df354aaa4070a9b7933758ea31f83695f08fd639 100644 (file)
--- a/folly/fibers/FiberManagerInternal-inl.h
+++ b/folly/fibers/FiberManagerInternal-inl.h
@@ -85,11 +85,10 @@ inline void FiberManager::deactivateFiber(Fiber* fiber) {
   DCHECK(fiber->asanMainStackBase_);
   DCHECK(fiber->asanMainStackSize_);
 
-  // Release fake stack if fiber is completed
-  auto saveFakeStackPtr =
-      fiber->state_ == Fiber::INVALID ? nullptr : &fiber->asanFakeStack_;
   registerStartSwitchStackWithAsan(
-      saveFakeStackPtr, fiber->asanMainStackBase_, fiber->asanMainStackSize_);
+      &fiber->asanFakeStack_,
+      fiber->asanMainStackBase_,
+      fiber->asanMainStackSize_);
   SCOPE_EXIT {
     registerFinishSwitchStackWithAsan(
         fiber->asanFakeStack_,

diff --git a/folly/fibers/FiberManagerInternal.h b/folly/fibers/FiberManagerInternal.h
index b58ab0ed873e7096ffa933b81eb8a58a91c51efd..18ec76287b017ff44eaa6d0deb77e56442db3e04 100644 (file)
--- a/folly/fibers/FiberManagerInternal.h
+++ b/folly/fibers/FiberManagerInternal.h
@@ -478,6 +478,7 @@ class FiberManager : public ::folly::Executor {
       void* fakeStack,
       const void** saveStackBase,
       size_t* saveStackSize);
+  void freeFakeStack(void* fakeStack);
   void unpoisonFiberStack(const Fiber* fiber);
 
 #endif // FOLLY_SANITIZE_ADDRESS