<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[213599] trunk/Source/JavaScriptCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/213599">213599</a></dd>
<dt>Author</dt> <dd>keith_miller@apple.com</dd>
<dt>Date</dt> <dd>2017-03-08 14:50:51 -0800 (Wed, 08 Mar 2017)</dd>
</dl>

<h3>Log Message</h3>
<pre>WebAssembly: Make OOB for fast memory do an extra safety check by ensuring the faulting address is in the range we allocated for fast memory
https://bugs.webkit.org/show_bug.cgi?id=169290

Reviewed by Saam Barati.

This patch adds an extra sanity check by ensuring that the the memory address we faulting trying to load is in range
of some wasm fast memory.

* wasm/WasmFaultSignalHandler.cpp:
(JSC::Wasm::trapHandler):
(JSC::Wasm::enableFastMemory):
* wasm/WasmMemory.cpp:
(JSC::Wasm::activeFastMemories):
(JSC::Wasm::viewActiveFastMemories):
(JSC::Wasm::tryGetFastMemory):
(JSC::Wasm::releaseFastMemory):
* wasm/WasmMemory.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmWasmFaultSignalHandlercpp">trunk/Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmWasmMemorycpp">trunk/Source/JavaScriptCore/wasm/WasmMemory.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmWasmMemoryh">trunk/Source/JavaScriptCore/wasm/WasmMemory.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (213598 => 213599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2017-03-08 22:44:42 UTC (rev 213598)
+++ trunk/Source/JavaScriptCore/ChangeLog        2017-03-08 22:50:51 UTC (rev 213599)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2017-03-08  Keith Miller  &lt;keith_miller@apple.com&gt;
+
+        WebAssembly: Make OOB for fast memory do an extra safety check by ensuring the faulting address is in the range we allocated for fast memory
+        https://bugs.webkit.org/show_bug.cgi?id=169290
+
+        Reviewed by Saam Barati.
+
+        This patch adds an extra sanity check by ensuring that the the memory address we faulting trying to load is in range
+        of some wasm fast memory.
+
+        * wasm/WasmFaultSignalHandler.cpp:
+        (JSC::Wasm::trapHandler):
+        (JSC::Wasm::enableFastMemory):
+        * wasm/WasmMemory.cpp:
+        (JSC::Wasm::activeFastMemories):
+        (JSC::Wasm::viewActiveFastMemories):
+        (JSC::Wasm::tryGetFastMemory):
+        (JSC::Wasm::releaseFastMemory):
+        * wasm/WasmMemory.h:
+
</ins><span class="cx"> 2017-03-07  Dean Jackson  &lt;dino@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Some platforms won't be able to create a GPUDevice
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmWasmFaultSignalHandlercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp (213598 => 213599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp        2017-03-08 22:44:42 UTC (rev 213598)
+++ trunk/Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp        2017-03-08 22:50:51 UTC (rev 213599)
</span><span class="lines">@@ -31,6 +31,7 @@
</span><span class="cx"> #include &quot;ExecutableAllocator.h&quot;
</span><span class="cx"> #include &quot;VM.h&quot;
</span><span class="cx"> #include &quot;WasmExceptionType.h&quot;
</span><ins>+#include &quot;WasmMemory.h&quot;
</ins><span class="cx"> 
</span><span class="cx"> #include &lt;signal.h&gt;
</span><span class="cx"> #include &lt;wtf/Lock.h&gt;
</span><span class="lines">@@ -79,7 +80,7 @@
</span><span class="cx"> 
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><del>-static void trapHandler(int signal, siginfo_t*, void* ucontext)
</del><ins>+static void trapHandler(int signal, siginfo_t* sigInfo, void* ucontext)
</ins><span class="cx"> {
</span><span class="cx">     mcontext_t&amp; context = static_cast&lt;ucontext_t*&gt;(ucontext)-&gt;uc_mcontext;
</span><span class="cx">     void* faultingInstruction = reinterpret_cast&lt;void*&gt;(InstructionPointerGPR);
</span><span class="lines">@@ -91,25 +92,42 @@
</span><span class="cx">     if (reinterpret_cast&lt;void*&gt;(startOfFixedExecutableMemoryPool) &lt;= faultingInstruction
</span><span class="cx">         &amp;&amp; faultingInstruction &lt; reinterpret_cast&lt;void*&gt;(endOfFixedExecutableMemoryPool)) {
</span><span class="cx"> 
</span><del>-        LockHolder locker(codeLocationsLock);
-        for (auto range : codeLocations.get()) {
-            VM* vm;
-            void* start;
-            void* end;
-            std::tie(vm, start, end) = range;
-            dataLogLnIf(verbose, &quot;function start: &quot;, RawPointer(start), &quot; end: &quot;, RawPointer(end));
-            if (start &lt;= faultingInstruction &amp;&amp; faultingInstruction &lt; end) {
-                dataLogLnIf(verbose, &quot;found match&quot;);
-                MacroAssemblerCodeRef exceptionStub = vm-&gt;jitStubs-&gt;existingCTIStub(throwExceptionFromWasmThunkGenerator);
-                // If for whatever reason we don't have a stub then we should just treat this like a regular crash.
-                if (!exceptionStub)
</del><ins>+        bool faultedInActiveFastMemory = false;
+        {
+            void* faultingAddress = sigInfo-&gt;si_addr;
+            dataLogLnIf(verbose, &quot;checking faulting address: &quot;, RawPointer(faultingAddress), &quot; is in an active fast memory&quot;);
+            LockHolder locker(memoryLock);
+            auto&amp; activeFastMemories = viewActiveFastMemories(locker);
+            for (void* activeMemory : activeFastMemories) {
+                dataLogLnIf(verbose, &quot;checking fast memory at: &quot;, RawPointer(activeMemory));
+                if (activeMemory &lt;= faultingAddress &amp;&amp; faultingAddress &lt; static_cast&lt;char*&gt;(activeMemory) + fastMemoryMappedBytes) {
+                    faultedInActiveFastMemory = true;
</ins><span class="cx">                     break;
</span><del>-                dataLogLnIf(verbose, &quot;found stub: &quot;, RawPointer(exceptionStub.code().executableAddress()));
-                FirstArgumentGPR = static_cast&lt;uint64_t&gt;(ExceptionType::OutOfBoundsMemoryAccess);
-                InstructionPointerGPR = reinterpret_cast&lt;uint64_t&gt;(exceptionStub.code().executableAddress());
-                return;
</del><ins>+                }
</ins><span class="cx">             }
</span><span class="cx">         }
</span><ins>+        if (faultedInActiveFastMemory) {
+            dataLogLnIf(verbose, &quot;found active fast memory for faulting address&quot;);
+            LockHolder locker(codeLocationsLock);
+            for (auto range : codeLocations.get()) {
+                VM* vm;
+                void* start;
+                void* end;
+                std::tie(vm, start, end) = range;
+                dataLogLnIf(verbose, &quot;function start: &quot;, RawPointer(start), &quot; end: &quot;, RawPointer(end));
+                if (start &lt;= faultingInstruction &amp;&amp; faultingInstruction &lt; end) {
+                    dataLogLnIf(verbose, &quot;found match&quot;);
+                    MacroAssemblerCodeRef exceptionStub = vm-&gt;jitStubs-&gt;existingCTIStub(throwExceptionFromWasmThunkGenerator);
+                    // If for whatever reason we don't have a stub then we should just treat this like a regular crash.
+                    if (!exceptionStub)
+                        break;
+                    dataLogLnIf(verbose, &quot;found stub: &quot;, RawPointer(exceptionStub.code().executableAddress()));
+                    FirstArgumentGPR = static_cast&lt;uint64_t&gt;(ExceptionType::OutOfBoundsMemoryAccess);
+                    InstructionPointerGPR = reinterpret_cast&lt;uint64_t&gt;(exceptionStub.code().executableAddress());
+                    return;
+                }
+            }
+        }
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     // Since we only use fast memory in processes we control, if we restore we will just fall back to the default handler.
</span><span class="lines">@@ -159,10 +177,10 @@
</span><span class="cx">         // Thus, we must not fail in the following attempts.
</span><span class="cx">         int ret = 0;
</span><span class="cx">         ret = sigaction(SIGBUS, &amp;action, &amp;oldSigBusHandler);
</span><del>-        ASSERT_UNUSED(ret, !ret);
</del><ins>+        RELEASE_ASSERT(!ret);
</ins><span class="cx"> 
</span><span class="cx">         ret = sigaction(SIGSEGV, &amp;action, &amp;oldSigSegvHandler);
</span><del>-        ASSERT_UNUSED(ret, !ret);
</del><ins>+        RELEASE_ASSERT(!ret);
</ins><span class="cx"> 
</span><span class="cx">         codeLocations.construct();
</span><span class="cx">         fastHandlerInstalled = true;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmWasmMemorycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/WasmMemory.cpp (213598 => 213599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/WasmMemory.cpp        2017-03-08 22:44:42 UTC (rev 213598)
+++ trunk/Source/JavaScriptCore/wasm/WasmMemory.cpp        2017-03-08 22:50:51 UTC (rev 213599)
</span><span class="lines">@@ -65,12 +65,9 @@
</span><span class="cx">     return lastAllocatedMemoryMode;
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-static_assert(sizeof(uint64_t) == sizeof(size_t), &quot;We rely on allowing the maximum size of Memory we map to be 2^33 which is larger than fits in a 32-bit integer that we'd pass to mprotect if this didn't hold.&quot;);
-
-static const size_t fastMemoryMappedBytes = (static_cast&lt;size_t&gt;(std::numeric_limits&lt;uint32_t&gt;::max()) + 1) * 2; // pointer max + offset max. This is all we need since a load straddling readable memory will trap.
</del><span class="cx"> static const unsigned maxFastMemories = 4;
</span><span class="cx"> static unsigned allocatedFastMemories { 0 };
</span><del>-static StaticLock memoryLock;
</del><ins>+StaticLock memoryLock;
</ins><span class="cx"> inline Deque&lt;void*, maxFastMemories&gt;&amp; availableFastMemories(const LockHolder&amp;)
</span><span class="cx"> {
</span><span class="cx">     static NeverDestroyed&lt;Deque&lt;void*, maxFastMemories&gt;&gt; availableFastMemories;
</span><span class="lines">@@ -77,6 +74,17 @@
</span><span class="cx">     return availableFastMemories;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+inline HashSet&lt;void*&gt;&amp; activeFastMemories(const LockHolder&amp;)
+{
+    static NeverDestroyed&lt;HashSet&lt;void*&gt;&gt; activeFastMemories;
+    return activeFastMemories;
+}
+
+const HashSet&lt;void*&gt;&amp; viewActiveFastMemories(const LockHolder&amp; locker)
+{
+    return activeFastMemories(locker);
+}
+
</ins><span class="cx"> inline bool tryGetFastMemory(VM&amp; vm, void*&amp; memory, size_t&amp; mappedCapacity, Memory::Mode&amp; mode)
</span><span class="cx"> {
</span><span class="cx">     // We might GC here so we should be holding the API lock.
</span><span class="lines">@@ -95,6 +103,8 @@
</span><span class="cx">         LockHolder locker(memoryLock);
</span><span class="cx">         if (!availableFastMemories(locker).isEmpty()) {
</span><span class="cx">             memory = availableFastMemories(locker).takeFirst();
</span><ins>+            auto result = activeFastMemories(locker).add(memory);
+            ASSERT_UNUSED(result, result.isNewEntry);
</ins><span class="cx">             mappedCapacity = fastMemoryMappedBytes;
</span><span class="cx">             mode = Memory::Signaling;
</span><span class="cx">             return true;
</span><span class="lines">@@ -117,6 +127,9 @@
</span><span class="cx">         mappedCapacity = fastMemoryMappedBytes;
</span><span class="cx">         mode = Memory::Signaling;
</span><span class="cx">         allocatedFastMemories++;
</span><ins>+        LockHolder locker(memoryLock);
+        auto result = activeFastMemories(locker).add(memory);
+        ASSERT_UNUSED(result, result.isNewEntry);
</ins><span class="cx">     }
</span><span class="cx">     return memory;
</span><span class="cx"> }
</span><span class="lines">@@ -134,6 +147,8 @@
</span><span class="cx">         CRASH();
</span><span class="cx"> 
</span><span class="cx">     LockHolder locker(memoryLock);
</span><ins>+    bool result = activeFastMemories(locker).remove(memory);
+    ASSERT_UNUSED(result, result);
</ins><span class="cx">     ASSERT(availableFastMemories(locker).size() &lt; allocatedFastMemories);
</span><span class="cx">     availableFastMemories(locker).append(memory);
</span><span class="cx">     memory = nullptr;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmWasmMemoryh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/WasmMemory.h (213598 => 213599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/WasmMemory.h        2017-03-08 22:44:42 UTC (rev 213598)
+++ trunk/Source/JavaScriptCore/wasm/WasmMemory.h        2017-03-08 22:50:51 UTC (rev 213599)
</span><span class="lines">@@ -29,6 +29,7 @@
</span><span class="cx"> 
</span><span class="cx"> #include &quot;WasmPageCount.h&quot;
</span><span class="cx"> 
</span><ins>+#include &lt;wtf/HashSet.h&gt;
</ins><span class="cx"> #include &lt;wtf/Optional.h&gt;
</span><span class="cx"> #include &lt;wtf/RefCounted.h&gt;
</span><span class="cx"> #include &lt;wtf/RefPtr.h&gt;
</span><span class="lines">@@ -93,6 +94,12 @@
</span><span class="cx">     Mode m_mode { Mode::BoundsChecking };
</span><span class="cx"> };
</span><span class="cx"> 
</span><ins>+static_assert(sizeof(uint64_t) == sizeof(size_t), &quot;We rely on allowing the maximum size of Memory we map to be 2^33 which is larger than fits in a 32-bit integer that we'd pass to mprotect if this didn't hold.&quot;);
+
+const size_t fastMemoryMappedBytes = (static_cast&lt;size_t&gt;(std::numeric_limits&lt;uint32_t&gt;::max()) + 1) * 2; // pointer max + offset max. This is all we need since a load straddling readable memory will trap.
+extern StaticLock memoryLock;
+const HashSet&lt;void*&gt;&amp; viewActiveFastMemories(const LockHolder&amp;);
+
</ins><span class="cx"> } } // namespace JSC::Wasm
</span><span class="cx"> 
</span><span class="cx"> #endif // ENABLE(WEBASSEMLY)
</span></span></pre>
</div>
</div>

</body>
</html>