teavm icon indicating copy to clipboard operation
teavm copied to clipboard

Out-of-bounds memory access in GC after heap shrink with WebAssembly

Open dicej opened this issue 2 years ago • 0 comments

In GC.resizeHeapConsistent, the code for the "shrink" case (i.e. when the heap is made smaller) either shrinks the last chunk or removes it entirely, depending on how much the heap was shrunk by. However, WasmHeap.resizeHeap ignores shrinks entirely, so the next time GC asks what the heap size is, it thinks it needs to shrink it again. This can cause GC.freeChunks and GC.totalChunks to be decremented repeatedly, which eventually leads to heap corruption and out-of-bounds memory access.

The following patch to GC.resizeHeapConsistent fixes the problem, although there may be a better way to address it:

-            if (newSize == minimumSize) {
-                freeChunks--;
-                totalChunks--;
-            } else {
-                lastChunk.size -= (int) (oldSize - newSize);
-            }
             resizeHeap(newSize);
+            if (availableBytes() != oldSize) {
+                if (newSize == minimumSize) {
+                    freeChunks--;
+                    totalChunks--;
+                } else {
+                    lastChunk.size -= (int) (oldSize - newSize);
+                }
+            }

This patch effectively disables heap shrinking for WebAssembly (i.e. makes it consistent with WasmHeap.resizeHeap, so the heap tends to grow quickly over time. The rate of growth can optionally be reduced using e.g.:

-    private static int youngGCCount;
 
     static native Address gcStorageAddress();
 
@@ -182,14 +181,9 @@ public final class GC {
             minRequestedSize = computeMinRequestedSize(size);
         }
 
-        if (!isFullGC) {
-            if (++youngGCCount >= 8 && isAboutToExpand(minRequestedSize)) {
-                triggerFullGC();
-                doCollectGarbage();
-                youngGCCount = 0;
-            }
-        } else {
-            youngGCCount = 0;
+        if (!isFullGC && isAboutToExpand(minRequestedSize)) {
+            triggerFullGC();
+            doCollectGarbage();
         }

dicej avatar Oct 31 '22 22:10 dicej