Changeset 224122 in webkit

Timestamp:

Oct 27, 2017 11:42:27 AM (6 years ago)

Author:

jfbastien@apple.com

Message:

WebAssembly: update arbitrary limits to what browsers use
​https://bugs.webkit.org/show_bug.cgi?id=178946
<rdar://problem/34257412>
<rdar://problem/34501154>

Reviewed by Saam Barati.

​https://github.com/WebAssembly/design/issues/1138 discusses the
arbitrary function size limit, which it turns out Chrome and
Firefox didn't enforce. We didn't use it because it was
ridiculously low and actual programs ran into that limit (bummer
for Edge which just shipped it...). Now that we agree on a high
arbitrary program limit, let's update it! While I'm doing this
there are a few other spots that I polished to use Checked or
better check limits overall.

• wasm/WasmB3IRGenerator.cpp:

(JSC::Wasm::B3IRGenerator::addLocal):

• wasm/WasmFormat.cpp:

(JSC::Wasm::Segment::create):

• wasm/WasmFunctionParser.h:

(JSC::Wasm::FunctionParser<Context>::parse):

• wasm/WasmInstance.cpp:
• wasm/WasmLimits.h:
• wasm/WasmModuleParser.cpp:

(JSC::Wasm::ModuleParser::parseGlobal):
(JSC::Wasm::ModuleParser::parseCode):
(JSC::Wasm::ModuleParser::parseData):

• wasm/WasmSignature.h:

(JSC::Wasm::Signature::allocatedSize):

• wasm/WasmTable.cpp:

(JSC::Wasm::Table::Table):

• wasm/js/JSWebAssemblyTable.cpp:

(JSC::JSWebAssemblyTable::JSWebAssemblyTable):
(JSC::JSWebAssemblyTable::grow):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• wasm/WasmB3IRGenerator.cpp (modified) (1 diff)
• wasm/WasmFormat.cpp (modified) (2 diffs)
• wasm/WasmFunctionParser.h (modified) (2 diffs)
• wasm/WasmInstance.cpp (modified) (2 diffs)
• wasm/WasmLimits.h (modified) (1 diff)
• wasm/WasmModuleParser.cpp (modified) (3 diffs)
• wasm/WasmSignature.h (modified) (2 diffs)
• wasm/WasmTable.cpp (modified) (1 diff)
• wasm/js/JSWebAssemblyTable.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-10-27  JF Bastien  <jfbastien@apple.com>
+
+        WebAssembly: update arbitrary limits to what browsers use
+        https://bugs.webkit.org/show_bug.cgi?id=178946
+        <rdar://problem/34257412>
+        <rdar://problem/34501154>
+
+        Reviewed by Saam Barati.
+
+        https://github.com/WebAssembly/design/issues/1138 discusses the
+        arbitrary function size limit, which it turns out Chrome and
+        Firefox didn't enforce. We didn't use it because it was
+        ridiculously low and actual programs ran into that limit (bummer
+        for Edge which just shipped it...). Now that we agree on a high
+        arbitrary program limit, let's update it! While I'm doing this
+        there are a few other spots that I polished to use Checked or
+        better check limits overall.
+
+        * wasm/WasmB3IRGenerator.cpp:
+        (JSC::Wasm::B3IRGenerator::addLocal):
+        * wasm/WasmFormat.cpp:
+        (JSC::Wasm::Segment::create):
+        * wasm/WasmFunctionParser.h:
+        (JSC::Wasm::FunctionParser<Context>::parse):
+        * wasm/WasmInstance.cpp:
+        * wasm/WasmLimits.h:
+        * wasm/WasmModuleParser.cpp:
+        (JSC::Wasm::ModuleParser::parseGlobal):
+        (JSC::Wasm::ModuleParser::parseCode):
+        (JSC::Wasm::ModuleParser::parseData):
+        * wasm/WasmSignature.h:
+        (JSC::Wasm::Signature::allocatedSize):
+        * wasm/WasmTable.cpp:
+        (JSC::Wasm::Table::Table):
+        * wasm/js/JSWebAssemblyTable.cpp:
+        (JSC::JSWebAssemblyTable::JSWebAssemblyTable):
+        (JSC::JSWebAssemblyTable::grow):
+
 2017-10-26  Michael Saboff  <msaboff@apple.com>
 

trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp

@@ -512 +512 @@
 auto B3IRGenerator::addLocal(Type type, uint32_t count) -> PartialResult
 {
-    WASM_COMPILE_FAIL_IF(!m_locals.tryReserveCapacity(m_locals.size() + count), "can't allocate memory for ", m_locals.size() + count, " locals");
+    Checked<uint32_t, RecordOverflow> totalBytesChecked = count;
+    totalBytesChecked += m_locals.size();
+    uint32_t totalBytes;
+    WASM_COMPILE_FAIL_IF((totalBytesChecked.safeGet(totalBytes) == CheckedState::DidOverflow) || !m_locals.tryReserveCapacity(totalBytes), "can't allocate memory for ", totalBytes, " locals");
 
     for (uint32_t i = 0; i < count; ++i) {

trunk/Source/JavaScriptCore/wasm/WasmFormat.cpp

@@ -31 +31 @@
 
 #include "WasmMemory.h"
+#include <wtf/CheckedArithmetic.h>
 #include <wtf/FastMalloc.h>
 
@@ -37 +38 @@
 Segment* Segment::create(I32InitExpr offset, uint32_t sizeInBytes)
 {
-    auto allocated = tryFastCalloc(sizeof(Segment) + sizeInBytes, 1);
+    Checked<uint32_t, RecordOverflow> totalBytesChecked = sizeInBytes;
+    totalBytesChecked += sizeof(Segment);
+    uint32_t totalBytes;
+    if (totalBytesChecked.safeGet(totalBytes) == CheckedState::DidOverflow)
+        return nullptr;
+    auto allocated = tryFastCalloc(totalBytes, 1);
     Segment* segment;
     if (!allocated.getValue(segment))

trunk/Source/JavaScriptCore/wasm/WasmFunctionParser.h

@@ -113 +113 @@
     WASM_PARSER_FAIL_IF(!m_context.addArguments(m_signature), "can't add ", m_signature.argumentCount(), " arguments to Function");
     WASM_PARSER_FAIL_IF(!parseVarUInt32(localCount), "can't get local count");
-    WASM_PARSER_FAIL_IF(localCount == std::numeric_limits<uint32_t>::max(), "Function section's local count is too big ", localCount);
+    WASM_PARSER_FAIL_IF(localCount > maxFunctionLocals, "Function section's local count is too big ", localCount, " maximum ", maxFunctionLocals);
 
     for (uint32_t i = 0; i < localCount; ++i) {
@@ -120 +120 @@
 
         WASM_PARSER_FAIL_IF(!parseVarUInt32(numberOfLocals), "can't get Function's number of locals in group ", i);
-        WASM_PARSER_FAIL_IF(numberOfLocals == std::numeric_limits<uint32_t>::max(), "Function section's ", i, "th local group count is too big ", numberOfLocals);
+        WASM_PARSER_FAIL_IF(numberOfLocals > maxFunctionLocals, "Function section's ", i, "th local group count is too big ", numberOfLocals, " maximum ", maxFunctionLocals);
         WASM_PARSER_FAIL_IF(!parseValueType(typeOfLocal), "can't get Function local's type in group ", i);
         WASM_TRY_ADD_TO_CONTEXT(addLocal(typeOfLocal, numberOfLocals));

trunk/Source/JavaScriptCore/wasm/WasmInstance.cpp

@@ -27 +27 @@
 #include "WasmInstance.h"
 
+#if ENABLE(WEBASSEMBLY)
+
 #include "Register.h"
 #include "WasmModuleInformation.h"
-
-#if ENABLE(WEBASSEMBLY)
+#include <wtf/CheckedArithmetic.h>
 
 namespace JSC { namespace Wasm {
@@ -37 +38 @@
 size_t globalMemoryByteSize(Module& module)
 {
-    return module.moduleInformation().globals.size() * sizeof(Register);
+    return (Checked<size_t>(module.moduleInformation().globals.size()) * sizeof(Register)).unsafeGet();
 }
 }

trunk/Source/JavaScriptCore/wasm/WasmLimits.h

@@ -47 +47 @@
 constexpr size_t maxStringSize = 100000;
 constexpr size_t maxModuleSize = 1024 * 1024 * 1024;
+constexpr size_t maxFunctionSize = 7654321;
+constexpr size_t maxFunctionLocals = 50000;
 constexpr size_t maxFunctionParams = 1000;
 

trunk/Source/JavaScriptCore/wasm/WasmModuleParser.cpp

@@ -341 +341 @@
     WASM_PARSER_FAIL_IF(!parseVarUInt32(globalCount), "can't get Global section's count");
     WASM_PARSER_FAIL_IF(globalCount > maxGlobals, "Global section's count is too big ", globalCount, " maximum ", maxGlobals);
-    WASM_PARSER_FAIL_IF(!m_info->globals.tryReserveCapacity(globalCount + m_info->firstInternalGlobal), "can't allocate memory for ", globalCount + m_info->firstInternalGlobal, " globals");
+    size_t totalBytes = globalCount + m_info->firstInternalGlobal;
+    WASM_PARSER_FAIL_IF((static_cast<uint32_t>(totalBytes) < globalCount) || !m_info->globals.tryReserveCapacity(totalBytes), "can't allocate memory for ", totalBytes, " globals");
 
     for (uint32_t globalIndex = 0; globalIndex < globalCount; ++globalIndex) {
@@ -475 +476 @@
         WASM_PARSER_FAIL_IF(functionSize > length(), "Code function's size ", functionSize, " exceeds the module's size ", length());
         WASM_PARSER_FAIL_IF(functionSize > length() - m_offset, "Code function's size ", functionSize, " exceeds the module's remaining size", length() - m_offset);
-        WASM_PARSER_FAIL_IF(functionSize > std::numeric_limits<uint32_t>::max(), "Code function's size ", functionSize, " is too big");
+        WASM_PARSER_FAIL_IF(functionSize > maxFunctionSize, "Code function's size ", functionSize, " is too big");
 
         m_info->functionLocationInBinary[i].start = m_offset;
@@ -574 +575 @@
         WASM_PARSER_FAIL_IF(initExprType != I32, segmentNumber, "th Data segment's init_expr must produce an i32");
         WASM_PARSER_FAIL_IF(!parseVarUInt32(dataByteLength), "can't get ", segmentNumber, "th Data segment's data byte length");
-        WASM_PARSER_FAIL_IF(dataByteLength == std::numeric_limits<uint32_t>::max(), segmentNumber, "th Data segment's data byte length is too big ", dataByteLength);
+        WASM_PARSER_FAIL_IF(dataByteLength > maxModuleSize, segmentNumber, "th Data segment's data byte length is too big ", dataByteLength, " maximum ", maxModuleSize);
 
         Segment* segment = Segment::create(makeI32InitExpr(initOpcode, initExprBits), dataByteLength);

trunk/Source/JavaScriptCore/wasm/WasmSignature.h

@@ -32 +32 @@
 #include <cstdint>
 #include <cstring>
+#include <wtf/CheckedArithmetic.h>
 #include <wtf/HashMap.h>
 #include <wtf/HashTraits.h>
@@ -65 +66 @@
     }
     Type* storage(SignatureArgCount i) const { return const_cast<Signature*>(this)->storage(i); }
-    static size_t allocatedSize(SignatureArgCount argCount)
+    static size_t allocatedSize(Checked<SignatureArgCount> argCount)
     {
-        return sizeof(Signature) + (s_retCount + argCount) * sizeof(Type);
+        return (sizeof(Signature) + (s_retCount + argCount) * sizeof(Type)).unsafeGet();
     }
 

trunk/Source/JavaScriptCore/wasm/WasmTable.cpp

@@ -55 +55 @@
     // FIXME: It might be worth trying to pre-allocate maximum here. The spec recommends doing so.
     // But for now, we're not doing that.
-    m_functions = MallocPtr<Wasm::CallableFunction>::malloc(sizeof(Wasm::CallableFunction) * static_cast<size_t>(size()));
-    m_instances = MallocPtr<Instance*>::malloc(sizeof(Instance*) * static_cast<size_t>(size()));
+    m_functions = MallocPtr<Wasm::CallableFunction>::malloc((sizeof(Wasm::CallableFunction) * Checked<size_t>(size())).unsafeGet());
+    m_instances = MallocPtr<Instance*>::malloc((sizeof(Instance*) * Checked<size_t>(size())).unsafeGet());
     for (uint32_t i = 0; i < size(); ++i) {
         new (&m_functions.get()[i]) CallableFunction();

trunk/Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp

@@ -31 +31 @@
 #include "JSCInlines.h"
 #include "JSWebAssemblyInstance.h"
+#include <wtf/CheckedArithmetic.h>
 
 namespace JSC {
@@ -62 +63 @@
     // FIXME: It might be worth trying to pre-allocate maximum here. The spec recommends doing so.
     // But for now, we're not doing that.
-    m_jsFunctions = MallocPtr<WriteBarrier<JSObject>>::malloc(sizeof(WriteBarrier<JSObject>) * static_cast<size_t>(size()));
+    m_jsFunctions = MallocPtr<WriteBarrier<JSObject>>::malloc((sizeof(WriteBarrier<JSObject>) * Checked<size_t>(size())).unsafeGet());
     for (uint32_t i = 0; i < size(); ++i)
         new(&m_jsFunctions.get()[i]) WriteBarrier<JSObject>();
@@ -101 +102 @@
 
     size_t newSize = grew.value();
-    m_jsFunctions.realloc(sizeof(WriteBarrier<JSObject>) * newSize);
+    m_jsFunctions.realloc((sizeof(WriteBarrier<JSObject>) * Checked<size_t>(newSize)).unsafeGet());
 
     for (size_t i = oldSize; i < newSize; ++i)