From c4d903ddb009aa6472530699e154d85a24eac51d Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Fri, 30 Oct 2020 20:55:53 +0100
Subject: [PATCH] Fix memory corruption caused by GC-invisible coroutine stacks

Crucially this introduces BoehmGCStackAllocator, but it also
adds a bunch of wiring to avoid making libutil depend on bdw-gc.

Part of the solutions for #4178, #4200
---
 src/libexpr/eval.cc      | 26 ++++++++++++++++++++++++++
 src/libexpr/local.mk     |  2 +-
 src/libutil/serialise.cc | 35 ++++++++++++++++++++++++++++++++++-
 src/libutil/serialise.hh | 14 ++++++++++++++
 4 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 4de87d647..ea7ba0a6a 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -27,6 +27,10 @@
 #include <gc/gc.h>
 #include <gc/gc_cpp.h>
 
+#include <boost/coroutine2/coroutine.hpp>
+#include <boost/coroutine2/protected_fixedsize_stack.hpp>
+#include <boost/context/stack_context.hpp>
+
 #endif
 
 namespace nix {
@@ -220,6 +224,26 @@ static void * oomHandler(size_t requested)
     /* Convert this to a proper C++ exception. */
     throw std::bad_alloc();
 }
+
+class BoehmGCStackAllocator : public StackAllocator {
+  boost::coroutines2::protected_fixedsize_stack stack;
+
+  public:
+    boost::context::stack_context allocate() override {
+        auto sctx = stack.allocate();
+        GC_add_roots(static_cast<char *>(sctx.sp) - sctx.size, sctx.sp);
+        return sctx;
+    }
+
+    void deallocate(boost::context::stack_context sctx) override {
+        GC_remove_roots(static_cast<char *>(sctx.sp) - sctx.size, sctx.sp);
+        stack.deallocate(sctx);
+    }
+
+};
+
+static BoehmGCStackAllocator boehmGCStackAllocator;
+
 #endif
 
 
@@ -257,6 +281,8 @@ void initGC()
 
     GC_set_oom_fn(oomHandler);
 
+    StackAllocator::defaultAllocator = &boehmGCStackAllocator;
+
     /* Set the initial heap size to something fairly big (25% of
        physical RAM, up to a maximum of 384 MiB) so that in most cases
        we don't need to garbage collect at all.  (Collection has a
diff --git a/src/libexpr/local.mk b/src/libexpr/local.mk
index 687a8ccda..a5422169d 100644
--- a/src/libexpr/local.mk
+++ b/src/libexpr/local.mk
@@ -15,7 +15,7 @@ libexpr_CXXFLAGS += -I src/libutil -I src/libstore -I src/libfetchers -I src/lib
 
 libexpr_LIBS = libutil libstore libfetchers
 
-libexpr_LDFLAGS =
+libexpr_LDFLAGS = -lboost_context
 ifneq ($(OS), FreeBSD)
  libexpr_LDFLAGS += -ldl
 endif
diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc
index 5c9f6f901..28f6968d0 100644
--- a/src/libutil/serialise.cc
+++ b/src/libutil/serialise.cc
@@ -171,6 +171,39 @@ size_t StringSource::read(unsigned char * data, size_t len)
 #error Coroutines are broken in this version of Boost!
 #endif
 
+/* A concrete datatype allow virtual dispatch of stack allocation methods. */
+struct VirtualStackAllocator {
+    StackAllocator *allocator = StackAllocator::defaultAllocator;
+
+    boost::context::stack_context allocate() {
+        return allocator->allocate();
+    }
+
+    void deallocate(boost::context::stack_context sctx) {
+        allocator->deallocate(sctx);
+    }
+};
+
+
+/* This class reifies the default boost coroutine stack allocation strategy with
+   a virtual interface. */
+class DefaultStackAllocator : public StackAllocator {
+    boost::coroutines2::default_stack stack;
+
+    boost::context::stack_context allocate() {
+        return stack.allocate();
+    }
+
+    void deallocate(boost::context::stack_context sctx) {
+        deallocate(sctx);
+    }
+};
+
+static DefaultStackAllocator defaultAllocatorSingleton;
+
+StackAllocator *StackAllocator::defaultAllocator = &defaultAllocatorSingleton;
+
+
 std::unique_ptr<Source> sinkToSource(
     std::function<void(Sink &)> fun,
     std::function<void()> eof)
@@ -195,7 +228,7 @@ std::unique_ptr<Source> sinkToSource(
         size_t read(unsigned char * data, size_t len) override
         {
             if (!coro)
-                coro = coro_t::pull_type([&](coro_t::push_type & yield) {
+                coro = coro_t::pull_type(VirtualStackAllocator{}, [&](coro_t::push_type & yield) {
                     LambdaSink sink([&](const unsigned char * data, size_t len) {
                             if (len) yield(std::string((const char *) data, len));
                         });
diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh
index d7fe0b81e..5c7d3ce76 100644
--- a/src/libutil/serialise.hh
+++ b/src/libutil/serialise.hh
@@ -5,6 +5,7 @@
 #include "types.hh"
 #include "util.hh"
 
+namespace boost::context { struct stack_context; }
 
 namespace nix {
 
@@ -497,5 +498,18 @@ struct FramedSink : nix::BufferedSink
     };
 };
 
+/* Stack allocation strategy for sinkToSource.
+   Mutable to avoid a boehm gc dependency in libutil.
+
+   boost::context doesn't provide a virtual class, so we define our own.
+ */
+struct StackAllocator {
+    virtual boost::context::stack_context allocate() = 0;
+    virtual void deallocate(boost::context::stack_context sctx) = 0;
+
+    /* The stack allocator to use in sinkToSource and potentially elsewhere.
+       It is reassigned by the initGC() method in libexpr. */
+    static StackAllocator *defaultAllocator;
+};
 
 }