|
| 1 | +From 9d77c4191030576fd502faa04148b52fa6dbcb43 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Yaksh Bariya <yakshbari4@gmail.com> |
| 3 | +Date: Mon, 25 Aug 2025 14:19:59 +0530 |
| 4 | +Subject: [PATCH] src: correctly report memory changes to V8 |
| 5 | + |
| 6 | +Call `V8::ExternalMemoryAccounter::Update` instead of |
| 7 | +`V8::ExternalMemoryAccounter::Increase` to report memory difference to |
| 8 | +V8 |
| 9 | + |
| 10 | +Calling `V8::ExternalMemoryAccounter::Increase` with a signed integer on |
| 11 | +32-bit platforms causes instances where GC inside GC takes place leading |
| 12 | +to a crash in certain cases. |
| 13 | + |
| 14 | +During GC, native objects are destructed. In destructor for |
| 15 | +`CompressionStream` class used by zlib, memory release information is |
| 16 | +passed onto `V8::ExternalMemoryAccounter::Increase()` instead of |
| 17 | +`V8::ExternalMemoryAccounter::Decrease()` which triggers V8's memory |
| 18 | +limits, thus triggering GC inside GC which leads to crash. |
| 19 | + |
| 20 | +Bug initially introduced in commit |
| 21 | +1d5d7b6eedb2274c9ad48b5f378598a10479e4a7 |
| 22 | + |
| 23 | +For full report see https://hackerone.com/reports/3302484 |
| 24 | +--- |
| 25 | + src/node_mem-inl.h | 2 +- |
| 26 | + src/node_zlib.cc | 2 +- |
| 27 | + 2 files changed, 2 insertions(+), 2 deletions(-) |
| 28 | + |
| 29 | +diff --git a/src/node_mem-inl.h b/src/node_mem-inl.h |
| 30 | +index 06871d031d3..70d28dd524b 100644 |
| 31 | +--- a/src/node_mem-inl.h |
| 32 | ++++ b/src/node_mem-inl.h |
| 33 | +@@ -59,7 +59,7 @@ void* NgLibMemoryManager<Class, T>::ReallocImpl(void* ptr, |
| 34 | + // Environment*/Isolate* parameter and call the V8 method transparently. |
| 35 | + const int64_t new_size = size - previous_size; |
| 36 | + manager->IncreaseAllocatedSize(new_size); |
| 37 | +- manager->env()->external_memory_accounter()->Increase( |
| 38 | ++ manager->env()->external_memory_accounter()->Update( |
| 39 | + manager->env()->isolate(), new_size); |
| 40 | + *reinterpret_cast<size_t*>(mem) = size; |
| 41 | + mem += sizeof(size_t); |
| 42 | +diff --git a/src/node_zlib.cc b/src/node_zlib.cc |
| 43 | +index c088c547539..b8617093bdf 100644 |
| 44 | +--- a/src/node_zlib.cc |
| 45 | ++++ b/src/node_zlib.cc |
| 46 | +@@ -644,7 +644,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { |
| 47 | + if (report == 0) return; |
| 48 | + CHECK_IMPLIES(report < 0, zlib_memory_ >= static_cast<size_t>(-report)); |
| 49 | + zlib_memory_ += report; |
| 50 | +- AsyncWrap::env()->external_memory_accounter()->Increase( |
| 51 | ++ AsyncWrap::env()->external_memory_accounter()->Update( |
| 52 | + AsyncWrap::env()->isolate(), report); |
| 53 | + } |
| 54 | + |
| 55 | +-- |
| 56 | +2.51.0 |
| 57 | + |
0 commit comments