From fd5aa6ee3e33a508f3a8ca35e42d36a2437ebacc Mon Sep 17 00:00:00 2001
From: pennae <github@quasiparticle.net>
Date: Sat, 22 Jan 2022 21:17:35 +0100
Subject: [PATCH] allocate a GC root value for the Value cache pointer

keeping it as a simple data member means it won't be scanned by the GC, so
eventually the GC will collect a cache that is still referenced (resulting in
use-after-free of cache elements).

fixes #5962
---
 src/libexpr/eval.cc | 15 ++++++++++-----
 src/libexpr/eval.hh |  2 +-
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 94ffab175..b884b4001 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -435,6 +435,11 @@ EvalState::EvalState(
     , store(store)
     , buildStore(buildStore ? buildStore : store)
     , regexCache(makeRegexCache())
+#if HAVE_BOEHMGC
+    , valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
+#else
+    , valueAllocCache(std::make_shared<void *>(nullptr))
+#endif
     , baseEnv(allocEnv(128))
     , staticBaseEnv(false, 0)
 {
@@ -852,15 +857,15 @@ Value * EvalState::allocValue()
        GC_malloc_many returns a linked list of objects of the given size, where the first word
        of each object is also the pointer to the next object in the list. This also means that we
        have to explicitly clear the first word of every object we take. */
-    if (!valueAllocCache) {
-        valueAllocCache = GC_malloc_many(sizeof(Value));
-        if (!valueAllocCache) throw std::bad_alloc();
+    if (!*valueAllocCache) {
+        *valueAllocCache = GC_malloc_many(sizeof(Value));
+        if (!*valueAllocCache) throw std::bad_alloc();
     }
 
     /* GC_NEXT is a convenience macro for accessing the first word of an object.
        Take the first list item, advance the list to the next item, and clear the next pointer. */
-    void * p = valueAllocCache;
-    GC_PTR_STORE_AND_DIRTY(&valueAllocCache, GC_NEXT(p));
+    void * p = *valueAllocCache;
+    GC_PTR_STORE_AND_DIRTY(&*valueAllocCache, GC_NEXT(p));
     GC_NEXT(p) = nullptr;
 
     nrValues++;
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 82ce9d1b3..c59203aa5 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -133,7 +133,7 @@ private:
     std::shared_ptr<RegexCache> regexCache;
 
     /* Allocation cache for GC'd Value objects. */
-    void * valueAllocCache = nullptr;
+    std::shared_ptr<void *> valueAllocCache;
 
 public: