<!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>[160529] trunk/Source/WebKit2</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/160529">160529</a></dd>
<dt>Author</dt> <dd>benjamin@webkit.org</dd>
<dt>Date</dt> <dd>2013-12-12 23:04:42 -0800 (Thu, 12 Dec 2013)</dd>
</dl>
<h3>Log Message</h3>
<pre>Fix unsafe memory load/store from the argument encoder/decoder affecting ARM
https://bugs.webkit.org/show_bug.cgi?id=125674
Patch by Benjamin Poulain <bpoulain@apple.com> on 2013-12-12
Reviewed by Darin Adler.
Depending on the CPU and CPU config, load and store may have to be aligned.
The argument buffer has no particular alignment which can cause problems.
In this case, on ARMv7, strd and ldrd can have alignment restriction on 16 bytes.
The code encoding double and 64 bits integers was causing bugs.
To avoid problems, the encoders/decoders are modified to just use memcpy. The compiler optimizes
it away for the right instructions (clang uses two ldr/str in the case of 64bits values on ARMv7).
* Platform/CoreIPC/ArgumentDecoder.cpp:
(CoreIPC::decodeValueFromBuffer):
(CoreIPC::ArgumentDecoder::decode):
* Platform/CoreIPC/ArgumentEncoder.cpp:
(CoreIPC::copyValueToBuffer):
(CoreIPC::ArgumentEncoder::encode):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2PlatformCoreIPCArgumentDecodercpp">trunk/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp</a></li>
<li><a href="#trunkSourceWebKit2PlatformCoreIPCArgumentEncodercpp">trunk/Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (160528 => 160529)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2013-12-13 06:52:57 UTC (rev 160528)
+++ trunk/Source/WebKit2/ChangeLog 2013-12-13 07:04:42 UTC (rev 160529)
</span><span class="lines">@@ -1,3 +1,26 @@
</span><ins>+2013-12-12 Benjamin Poulain <bpoulain@apple.com>
+
+ Fix unsafe memory load/store from the argument encoder/decoder affecting ARM
+ https://bugs.webkit.org/show_bug.cgi?id=125674
+
+ Reviewed by Darin Adler.
+
+ Depending on the CPU and CPU config, load and store may have to be aligned.
+ The argument buffer has no particular alignment which can cause problems.
+
+ In this case, on ARMv7, strd and ldrd can have alignment restriction on 16 bytes.
+ The code encoding double and 64 bits integers was causing bugs.
+
+ To avoid problems, the encoders/decoders are modified to just use memcpy. The compiler optimizes
+ it away for the right instructions (clang uses two ldr/str in the case of 64bits values on ARMv7).
+
+ * Platform/CoreIPC/ArgumentDecoder.cpp:
+ (CoreIPC::decodeValueFromBuffer):
+ (CoreIPC::ArgumentDecoder::decode):
+ * Platform/CoreIPC/ArgumentEncoder.cpp:
+ (CoreIPC::copyValueToBuffer):
+ (CoreIPC::ArgumentEncoder::encode):
+
</ins><span class="cx"> 2013-12-12 Tim Horton <timothy_horton@apple.com>
</span><span class="cx">
</span><span class="cx"> Build fix for 32-bit.
</span></span></pre></div>
<a id="trunkSourceWebKit2PlatformCoreIPCArgumentDecodercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp (160528 => 160529)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp 2013-12-13 06:52:57 UTC (rev 160528)
+++ trunk/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp 2013-12-13 07:04:42 UTC (rev 160529)
</span><span class="lines">@@ -126,13 +126,19 @@
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+template<typename Type>
+static void decodeValueFromBuffer(Type& value, uint8_t*& bufferPosition)
+{
+ memcpy(&value, bufferPosition, sizeof(value));
+ bufferPosition += sizeof(Type);
+}
+
</ins><span class="cx"> bool ArgumentDecoder::decode(bool& result)
</span><span class="cx"> {
</span><span class="cx"> if (!alignBufferPosition(sizeof(result), sizeof(result)))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- result = *reinterpret_cast<bool*>(m_bufferPos);
- m_bufferPos += sizeof(result);
</del><ins>+ decodeValueFromBuffer(result, m_bufferPos);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -141,8 +147,7 @@
</span><span class="cx"> if (!alignBufferPosition(sizeof(result), sizeof(result)))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- result = *reinterpret_cast<uint8_t*>(m_bufferPos);
- m_bufferPos += sizeof(result);
</del><ins>+ decodeValueFromBuffer(result, m_bufferPos);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -151,8 +156,7 @@
</span><span class="cx"> if (!alignBufferPosition(sizeof(result), sizeof(result)))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- result = *reinterpret_cast_ptr<uint16_t*>(m_bufferPos);
- m_bufferPos += sizeof(result);
</del><ins>+ decodeValueFromBuffer(result, m_bufferPos);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -160,9 +164,8 @@
</span><span class="cx"> {
</span><span class="cx"> if (!alignBufferPosition(sizeof(result), sizeof(result)))
</span><span class="cx"> return false;
</span><del>-
- result = *reinterpret_cast_ptr<uint32_t*>(m_bufferPos);
- m_bufferPos += sizeof(result);
</del><ins>+
+ decodeValueFromBuffer(result, m_bufferPos);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -171,8 +174,7 @@
</span><span class="cx"> if (!alignBufferPosition(sizeof(result), sizeof(result)))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- result = *reinterpret_cast_ptr<uint64_t*>(m_bufferPos);
- m_bufferPos += sizeof(result);
</del><ins>+ decodeValueFromBuffer(result, m_bufferPos);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -181,8 +183,7 @@
</span><span class="cx"> if (!alignBufferPosition(sizeof(result), sizeof(result)))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- result = *reinterpret_cast_ptr<uint32_t*>(m_bufferPos);
- m_bufferPos += sizeof(result);
</del><ins>+ decodeValueFromBuffer(result, m_bufferPos);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -190,9 +191,8 @@
</span><span class="cx"> {
</span><span class="cx"> if (!alignBufferPosition(sizeof(result), sizeof(result)))
</span><span class="cx"> return false;
</span><del>-
- result = *reinterpret_cast_ptr<uint64_t*>(m_bufferPos);
- m_bufferPos += sizeof(result);
</del><ins>+
+ decodeValueFromBuffer(result, m_bufferPos);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -200,9 +200,8 @@
</span><span class="cx"> {
</span><span class="cx"> if (!alignBufferPosition(sizeof(result), sizeof(result)))
</span><span class="cx"> return false;
</span><del>-
- result = *reinterpret_cast_ptr<float*>(m_bufferPos);
- m_bufferPos += sizeof(result);
</del><ins>+
+ decodeValueFromBuffer(result, m_bufferPos);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -211,8 +210,7 @@
</span><span class="cx"> if (!alignBufferPosition(sizeof(result), sizeof(result)))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- result = *reinterpret_cast_ptr<double*>(m_bufferPos);
- m_bufferPos += sizeof(result);
</del><ins>+ decodeValueFromBuffer(result, m_bufferPos);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebKit2PlatformCoreIPCArgumentEncodercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.cpp (160528 => 160529)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.cpp 2013-12-13 06:52:57 UTC (rev 160528)
+++ trunk/Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.cpp 2013-12-13 07:04:42 UTC (rev 160529)
</span><span class="lines">@@ -123,67 +123,64 @@
</span><span class="cx"> encodeFixedLengthData(dataReference.data(), dataReference.size(), 1);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+template<typename Type>
+static void copyValueToBuffer(Type value, uint8_t* bufferPosition)
+{
+ memcpy(bufferPosition, &value, sizeof(Type));
+}
+
</ins><span class="cx"> void ArgumentEncoder::encode(bool n)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* buffer = grow(sizeof(n), sizeof(n));
</span><del>-
- *reinterpret_cast<bool*>(buffer) = n;
</del><ins>+ copyValueToBuffer(n, buffer);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ArgumentEncoder::encode(uint8_t n)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* buffer = grow(sizeof(n), sizeof(n));
</span><del>-
- *reinterpret_cast<uint8_t*>(buffer) = n;
</del><ins>+ copyValueToBuffer(n, buffer);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ArgumentEncoder::encode(uint16_t n)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* buffer = grow(sizeof(n), sizeof(n));
</span><del>-
- *reinterpret_cast_ptr<uint16_t*>(buffer) = n;
</del><ins>+ copyValueToBuffer(n, buffer);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ArgumentEncoder::encode(uint32_t n)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* buffer = grow(sizeof(n), sizeof(n));
</span><del>-
- *reinterpret_cast_ptr<uint32_t*>(buffer) = n;
</del><ins>+ copyValueToBuffer(n, buffer);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ArgumentEncoder::encode(uint64_t n)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* buffer = grow(sizeof(n), sizeof(n));
</span><del>-
- *reinterpret_cast_ptr<uint64_t*>(buffer) = n;
</del><ins>+ copyValueToBuffer(n, buffer);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ArgumentEncoder::encode(int32_t n)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* buffer = grow(sizeof(n), sizeof(n));
</span><del>-
- *reinterpret_cast_ptr<int32_t*>(buffer) = n;
</del><ins>+ copyValueToBuffer(n, buffer);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ArgumentEncoder::encode(int64_t n)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* buffer = grow(sizeof(n), sizeof(n));
</span><del>-
- *reinterpret_cast_ptr<int64_t*>(buffer) = n;
</del><ins>+ copyValueToBuffer(n, buffer);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ArgumentEncoder::encode(float n)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* buffer = grow(sizeof(n), sizeof(n));
</span><del>-
- *reinterpret_cast_ptr<float*>(buffer) = n;
</del><ins>+ copyValueToBuffer(n, buffer);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ArgumentEncoder::encode(double n)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* buffer = grow(sizeof(n), sizeof(n));
</span><del>-
- *reinterpret_cast_ptr<double*>(buffer) = n;
</del><ins>+ copyValueToBuffer(n, buffer);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ArgumentEncoder::addAttachment(const Attachment& attachment)
</span></span></pre>
</div>
</div>
</body>
</html>