Skip to content

Commit 987be10

Browse files
sosukesuzukisosukesuzuki
committed
[Win] Wasm memory and resizable ArrayBuffer crash when the commit limit is reached
https://bugs.webkit.org/show_bug.cgi?id=325406 Reviewed by Ian Grunert. Since 321850@main, Windows only reserves the address range of a Wasm memory or a growable buffer, and commits the bytes in use when the memory is created or grown. Windows has no overcommit, so this commit fails once the commit limit is reached, and OSAllocator::commit() and OSAllocator::protect() crash when it does. Creating or growing a Wasm memory, a resizable ArrayBuffer or a growable SharedArrayBuffer crashed instead of throwing a RangeError or returning -1 from memory.grow(). BufferMemoryManager allows up to three times the RAM size, which can be above the commit limit. This patch makes these paths commit through OSAllocator::tryProtect() on Windows and report out of memory when it fails. tryProtect() now retries a commit that hit the commit limit in the same way as libpas, which these allocations went through before 321850@main. Other platforms keep calling OSAllocator::protect(). * Source/JavaScriptCore/runtime/ArrayBuffer.cpp: (JSC::tryAllocateResizableMemory): (JSC::ArrayBuffer::resize): (JSC::SharedArrayBufferContents::tryGrow): * Source/JavaScriptCore/runtime/BufferMemoryHandle.cpp: (JSC::BufferMemoryManager::tryMakeReadableAndWritable): * Source/JavaScriptCore/runtime/BufferMemoryHandle.h: * Source/JavaScriptCore/wasm/WasmMemory.cpp: (JSC::Wasm::Memory::tryCreate): (JSC::Wasm::Memory::grow): * Source/WTF/wtf/win/OSAllocatorWin.cpp: (WTF::tryCommitWithRetry): (WTF::OSAllocator::tryProtect): Canonical link: https://commits.webkit.org/322017@main
1 parent d249df4 commit 987be10

5 files changed

Lines changed: 61 additions & 16 deletions

File tree

‎Source/JavaScriptCore/runtime/ArrayBuffer.cpp‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,11 @@ static RefPtr<BufferMemoryHandle> tryAllocateResizableMemory(VM* vm, size_t size
140140
}
141141

142142
#if OS(WINDOWS)
143-
if (initialBytes)
144-
OSAllocator::commit(slowMemory, initialBytes, /* writable */ true, /* executable */ false);
143+
if (!BufferMemoryManager::tryMakeReadableAndWritable(slowMemory, initialBytes)) {
144+
BufferMemoryManager::singleton().freeGrowableBoundsCheckingMemory(slowMemory, maximumBytes);
145+
BufferMemoryManager::singleton().freePhysicalBytes(initialBytes);
146+
return nullptr;
147+
}
145148
#endif
146149
constexpr bool readable = false;
147150
constexpr bool writable = false;
@@ -603,9 +606,10 @@ std::expected<int64_t, GrowFailReason> ArrayBuffer::resize(VM& vm, size_t newByt
603606
uint8_t* startAddress = static_cast<uint8_t*>(memory) + memoryHandle->size();
604607

605608
dataLogLnIf(ArrayBufferInternal::verbose, "Marking memory's ", RawPointer(memory), " as read+write in range [", RawPointer(startAddress), ", ", RawPointer(startAddress + bytesToAdd), ")");
606-
constexpr bool readable = true;
607-
constexpr bool writable = true;
608-
OSAllocator::protect(startAddress, bytesToAdd, readable, writable);
609+
if (!BufferMemoryManager::tryMakeReadableAndWritable(startAddress, bytesToAdd)) {
610+
BufferMemoryManager::singleton().freePhysicalBytes(bytesToAdd);
611+
return makeUnexpected(GrowFailReason::OutOfMemory);
612+
}
609613
} else {
610614
size_t bytesToSubtract = memoryHandle->size() - desiredSize;
611615
ASSERT(bytesToSubtract);
@@ -736,9 +740,10 @@ std::expected<int64_t, GrowFailReason> SharedArrayBufferContents::tryGrow(const
736740
uint8_t* startAddress = static_cast<uint8_t*>(memory) + memoryHandle->size();
737741

738742
dataLogLnIf(ArrayBufferInternal::verbose, "Marking memory's ", RawPointer(memory), " as read+write in range [", RawPointer(startAddress), ", ", RawPointer(startAddress + extraBytes), ")");
739-
constexpr bool readable = true;
740-
constexpr bool writable = true;
741-
OSAllocator::protect(startAddress, extraBytes, readable, writable);
743+
if (!BufferMemoryManager::tryMakeReadableAndWritable(startAddress, extraBytes)) {
744+
BufferMemoryManager::singleton().freePhysicalBytes(extraBytes);
745+
return makeUnexpected(GrowFailReason::OutOfMemory);
746+
}
742747
memoryHandle->updateSize(desiredSize);
743748
}
744749

‎Source/JavaScriptCore/runtime/BufferMemoryHandle.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,18 @@ void BufferMemoryManager::freeGrowableBoundsCheckingMemory(void* basePtr, size_t
159159
dataLogLnIf(Options::logWasmMemory(), "Freed virtual; state: ", *this);
160160
}
161161

162+
bool BufferMemoryManager::tryMakeReadableAndWritable(void* address, size_t bytes)
163+
{
164+
constexpr bool readable = true;
165+
constexpr bool writable = true;
166+
#if OS(WINDOWS)
167+
return OSAllocator::tryProtect(address, bytes, readable, writable);
168+
#else
169+
OSAllocator::protect(address, bytes, readable, writable);
170+
return true;
171+
#endif
172+
}
173+
162174
bool BufferMemoryManager::isInGrowableOrFastMemory(void* address)
163175
{
164176
// NOTE: This can be called from a signal handler, but only after we proved that we're in JIT code or IPInt code.

‎Source/JavaScriptCore/runtime/BufferMemoryHandle.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ class BufferMemoryManager {
102102

103103
void freeGrowableBoundsCheckingMemory(void* basePtr, size_t mappedCapacity);
104104

105+
// Windows commits the range at this point, so this fails there when the commit limit is reached.
106+
static bool tryMakeReadableAndWritable(void* address, size_t bytes);
107+
105108
bool isInGrowableOrFastMemory(void* address);
106109

107110
// We allow people to "commit" more wasm memory than there is on the system since most of the time

‎Source/JavaScriptCore/wasm/WasmMemory.cpp‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,11 @@ RefPtr<Memory> Memory::tryCreate(VM& vm, PageCount initial, PageCount maximum, M
200200
if (fastMemory) {
201201
#if OS(WINDOWS)
202202
// The region is only reserved; commit the in-use bytes before the no-access guard below.
203-
if (initialBytes)
204-
OSAllocator::commit(fastMemory, initialBytes, /* writable */ true, /* executable */ false);
203+
if (!BufferMemoryManager::tryMakeReadableAndWritable(fastMemory, initialBytes)) {
204+
BufferMemoryManager::singleton().freeFastMemory(fastMemory);
205+
BufferMemoryManager::singleton().freePhysicalBytes(initialBytes);
206+
return nullptr;
207+
}
205208
#endif
206209
constexpr bool readable = false;
207210
constexpr bool writable = false;
@@ -255,8 +258,11 @@ RefPtr<Memory> Memory::tryCreate(VM& vm, PageCount initial, PageCount maximum, M
255258
}
256259

257260
#if OS(WINDOWS)
258-
if (initialBytes)
259-
OSAllocator::commit(slowMemory, initialBytes, /* writable */ true, /* executable */ false);
261+
if (!BufferMemoryManager::tryMakeReadableAndWritable(slowMemory, initialBytes)) {
262+
BufferMemoryManager::singleton().freeGrowableBoundsCheckingMemory(slowMemory, reservedMaximumBytes);
263+
BufferMemoryManager::singleton().freePhysicalBytes(initialBytes);
264+
return nullptr;
265+
}
260266
#endif
261267
constexpr bool readable = false;
262268
constexpr bool writable = false;
@@ -421,9 +427,10 @@ std::expected<PageCount, GrowFailReason> Memory::grow(VM& vm, PageCount delta)
421427
uint8_t* startAddress = static_cast<uint8_t*>(memory) + size();
422428

423429
dataLogLnIf(verbose, "Marking WebAssembly memory's ", RawPointer(memory), " as read+write in range [", RawPointer(startAddress), ", ", RawPointer(startAddress + extraBytes), ")");
424-
constexpr bool readable = true;
425-
constexpr bool writable = true;
426-
OSAllocator::protect(startAddress, extraBytes, readable, writable);
430+
if (!BufferMemoryManager::tryMakeReadableAndWritable(startAddress, extraBytes)) {
431+
BufferMemoryManager::singleton().freePhysicalBytes(extraBytes);
432+
return makeUnexpected(GrowFailReason::OutOfMemory);
433+
}
427434
m_handle->updateSize(desiredSize);
428435
return success();
429436
}

‎Source/WTF/wtf/win/OSAllocatorWin.cpp‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,24 @@ void OSAllocator::hintMemoryNotNeededSoon(void*, size_t)
131131
{
132132
}
133133

134+
static bool tryCommitWithRetry(void* address, size_t bytes, DWORD protection)
135+
{
136+
// A commit that hits the commit limit is retried the way libpas does it, with the same numbers
137+
// (virtual_alloc_with_retry in pas_page_malloc.c).
138+
constexpr unsigned maxRetries = 10;
139+
constexpr DWORD retryDelayMS = 50;
140+
for (unsigned retry = 0; ; ++retry) {
141+
if (VirtualAlloc(address, bytes, MEM_COMMIT, protection))
142+
return true;
143+
DWORD error = GetLastError();
144+
if (error != ERROR_COMMITMENT_LIMIT && error != ERROR_NOT_ENOUGH_MEMORY)
145+
return false;
146+
if (retry == maxRetries)
147+
return false;
148+
Sleep(retryDelayMS);
149+
}
150+
}
151+
134152
bool OSAllocator::tryProtect(void* address, size_t bytes, bool readable, bool writable)
135153
{
136154
if (!bytes)
@@ -180,7 +198,7 @@ bool OSAllocator::tryProtect(void* address, size_t bytes, bool readable, bool wr
180198
ASSERT(memInfo.RegionSize > 0);
181199
ASSERT(static_cast<char*>(memInfo.BaseAddress) == currentPtr);
182200
size_t chunkSize = std::min(static_cast<size_t>(memInfo.RegionSize), bytes - totalSeen);
183-
if (!VirtualAlloc(currentPtr, chunkSize, MEM_COMMIT, protection))
201+
if (!tryCommitWithRetry(currentPtr, chunkSize, protection))
184202
return false;
185203
currentPtr += chunkSize;
186204
totalSeen += chunkSize;

0 commit comments

Comments
 (0)