Bug 1401939 - Align XDR content to avoid undefined behaviours. r=kmag,tcampbell
☠☠ backed out by cd0be57361f3 ☠ ☠
authorNicolas B. Pierron <nicolas.b.pierron@gmail.com>
Mon, 26 Feb 2018 16:49:23 +0000
changeset 407642 294a422d49b01b0b79992b527bf96b416789b9d6
parent 407641 da7cea91d417b215f9bc3eb8f9c4dc593ac44777
child 407643 bd71b6ae8e6c51bd3675adba932db5830c707069
push id100740
push usernpierron@mozilla.com
push dateMon, 12 Mar 2018 18:23:33 +0000
treeherdermozilla-inbound@294a422d49b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag, tcampbell
bugs1401939
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1401939 - Align XDR content to avoid undefined behaviours. r=kmag,tcampbell
js/src/vm/JSAtom.cpp
js/src/vm/JSFunction.cpp
js/src/vm/Xdr.cpp
js/src/vm/Xdr.h
js/xpconnect/loader/ScriptPreloader.cpp
--- a/js/src/vm/JSAtom.cpp
+++ b/js/src/vm/JSAtom.cpp
@@ -707,37 +707,48 @@ js::ToAtom<CanGC>(JSContext* cx, HandleV
 
 template JSAtom*
 js::ToAtom<NoGC>(JSContext* cx, const Value& v);
 
 template<XDRMode mode>
 bool
 js::XDRAtom(XDRState<mode>* xdr, MutableHandleAtom atomp)
 {
+    bool latin1 = false;
+    uint32_t length = 0;
+    uint32_t lengthAndEncoding = 0;
     if (mode == XDR_ENCODE) {
         static_assert(JSString::MAX_LENGTH <= INT32_MAX, "String length must fit in 31 bits");
-        uint32_t length = atomp->length();
-        uint32_t lengthAndEncoding = (length << 1) | uint32_t(atomp->hasLatin1Chars());
-        if (!xdr->codeUint32(&lengthAndEncoding))
-            return false;
-
-        JS::AutoCheckCannotGC nogc;
-        return atomp->hasLatin1Chars()
-               ? xdr->codeChars(atomp->latin1Chars(nogc), length)
-               : xdr->codeChars(const_cast<char16_t*>(atomp->twoByteChars(nogc)), length);
+        latin1 = atomp->hasLatin1Chars();
+        length = atomp->length();
+        lengthAndEncoding = (length << 1) | uint32_t(latin1);
     }
 
-    /* Avoid JSString allocation for already existing atoms. See bug 321985. */
-    uint32_t lengthAndEncoding;
     if (!xdr->codeUint32(&lengthAndEncoding))
         return false;
 
-    uint32_t length = lengthAndEncoding >> 1;
-    bool latin1 = lengthAndEncoding & 0x1;
+    if (mode == XDR_DECODE) {
+        length = lengthAndEncoding >> 1;
+        latin1 = lengthAndEncoding & 0x1;
+    }
+
+    // We need to align the string in the XDR buffer such that we can avoid
+    // non-align loads of 16bits characters.
+    if (!latin1 && !xdr->codeAlign(sizeof(char16_t)))
+        return false;
 
+    if (mode == XDR_ENCODE) {
+        JS::AutoCheckCannotGC nogc;
+        if (latin1)
+            return xdr->codeChars(atomp->latin1Chars(nogc), length);
+        return xdr->codeChars(const_cast<char16_t*>(atomp->twoByteChars(nogc)), length);
+    }
+
+    MOZ_ASSERT(mode == XDR_DECODE);
+    /* Avoid JSString allocation for already existing atoms. See bug 321985. */
     JSContext* cx = xdr->cx();
     JSAtom* atom;
     if (latin1) {
         const Latin1Char* chars = nullptr;
         if (length) {
             const uint8_t *ptr;
             size_t nbyte = length * sizeof(Latin1Char);
             if (!xdr->peekData(&ptr, nbyte))
@@ -749,16 +760,18 @@ js::XDRAtom(XDRState<mode>* xdr, Mutable
 #if MOZ_LITTLE_ENDIAN
         /* Directly access the little endian chars in the XDR buffer. */
         const char16_t* chars = nullptr;
         if (length) {
             const uint8_t *ptr;
             size_t nbyte = length * sizeof(char16_t);
             if (!xdr->peekData(&ptr, nbyte))
                 return false;
+            MOZ_ASSERT(reinterpret_cast<uintptr_t>(ptr) % sizeof(char16_t) == 0,
+                       "non-aligned buffer during JSAtom decoding");
             chars = reinterpret_cast<const char16_t*>(ptr);
         }
         atom = AtomizeChars(cx, chars, length);
 #else
         /*
          * We must copy chars to a temporary buffer to convert between little and
          * big endian data.
          */
--- a/js/src/vm/JSFunction.cpp
+++ b/js/src/vm/JSFunction.cpp
@@ -617,16 +617,18 @@ js::XDRInterpretedFunction(XDRState<mode
         // mirror the scope chain.
         MOZ_ASSERT_IF(fun->isSingleton() &&
                       !((lazy && lazy->hasBeenCloned()) || (script && script->hasBeenCloned())),
                       fun->environment() == nullptr);
     }
 
     // Everything added below can substituted by the non-lazy-script version of
     // this function later.
+    if (!xdr->codeAlign(sizeof(js::XDRAlignment)))
+        return false;
     js::AutoXDRTree funTree(xdr, xdr->getTreeKey(fun));
 
     if (!xdr->codeUint32(&firstword))
         return false;
 
     if ((firstword & HasAtom) && !XDRAtom(xdr, &atom))
         return false;
     if (!xdr->codeUint32(&flagsword))
@@ -675,16 +677,21 @@ js::XDRInterpretedFunction(XDRState<mode
             return false;
         objp.set(fun);
     }
 
     // Verify marker at end of function to detect buffer trunction.
     if (!xdr->codeMarker(0x9E35CA1F))
         return false;
 
+    // Required by AutoXDRTree to copy & paste snipet of sub-trees while keeping
+    // the alignment.
+    if (!xdr->codeAlign(sizeof(js::XDRAlignment)))
+        return false;
+
     return true;
 }
 
 template bool
 js::XDRInterpretedFunction(XDRState<XDR_ENCODE>*, HandleScope, HandleScriptSource,
                            MutableHandleFunction);
 
 template bool
--- a/js/src/vm/Xdr.cpp
+++ b/js/src/vm/Xdr.cpp
@@ -6,16 +6,17 @@
 
 #include "vm/Xdr.h"
 
 #include "mozilla/PodOperations.h"
 
 #include <string.h>
 
 #include "jsapi.h"
+#include "jsutil.h"
 
 #include "vm/Debugger.h"
 #include "vm/EnvironmentObject.h"
 #include "vm/JSContext.h"
 #include "vm/JSScript.h"
 #include "vm/TraceLogging.h"
 
 using namespace js;
@@ -165,16 +166,22 @@ template<XDRMode mode>
 bool
 XDRState<mode>::codeScript(MutableHandleScript scriptp)
 {
     TraceLoggerThread* logger = TraceLoggerForCurrentThread(cx());
     TraceLoggerTextId event =
         mode == XDR_DECODE ? TraceLogger_DecodeScript : TraceLogger_EncodeScript;
     AutoTraceLog tl(logger, event);
 
+    // This should be a no-op when encoding, but when decoding it would eat any
+    // miss-aligned bytes if when encoding the XDR buffer is appended at the end
+    // of an existing buffer, such as in XDRIncrementalEncoder::linearize
+    // function.
+    if (!codeAlign(sizeof(js::XDRAlignment)))
+        return false;
     AutoXDRTree scriptTree(this, getTopLevelTreeKey());
 
     if (mode == XDR_DECODE)
         scriptp.set(nullptr);
     else
         MOZ_ASSERT(!scriptp->enclosingScope());
 
     if (!VersionCheck(this)) {
@@ -183,16 +190,19 @@ XDRState<mode>::codeScript(MutableHandle
     }
 
     if (!XDRScript(this, nullptr, nullptr, nullptr, scriptp)) {
         postProcessContextErrors(cx());
         scriptp.set(nullptr);
         return false;
     }
 
+    if (!codeAlign(sizeof(js::XDRAlignment)))
+        return false;
+
     return true;
 }
 
 template<XDRMode mode>
 bool
 XDRState<mode>::codeConstValue(MutableHandleValue vp)
 {
     return XDRScriptConst(this, vp);
@@ -201,22 +211,26 @@ XDRState<mode>::codeConstValue(MutableHa
 template class js::XDRState<XDR_ENCODE>;
 template class js::XDRState<XDR_DECODE>;
 
 AutoXDRTree::AutoXDRTree(XDRCoderBase* xdr, AutoXDRTree::Key key)
   : key_(key),
     parent_(this),
     xdr_(xdr)
 {
+    // Expect sub-tree to start with the maximum alignment required.
+    MOZ_ASSERT(xdr->isAligned(sizeof(js::XDRAlignment)));
     if (key_ != AutoXDRTree::noKey)
         xdr->createOrReplaceSubTree(this);
 }
 
 AutoXDRTree::~AutoXDRTree()
 {
+    // Expect sub-tree to end with the maximum alignment required.
+    MOZ_ASSERT(xdr_->isAligned(sizeof(js::XDRAlignment)));
     if (key_ != AutoXDRTree::noKey)
         xdr_->endSubTree();
 }
 
 constexpr AutoXDRTree::Key AutoXDRTree::noKey;
 constexpr AutoXDRTree::Key AutoXDRTree::noSubTree;
 constexpr AutoXDRTree::Key AutoXDRTree::topLevel;
 
@@ -335,16 +349,28 @@ XDRIncrementalEncoder::linearize(JS::Tra
     if (oom_) {
         ReportOutOfMemory(cx());
         return fail(JS::TranscodeResult_Throw);
     }
 
     // Do not linearize while we are currently adding bytes.
     MOZ_ASSERT(scope_ == nullptr);
 
+    // Ensure the content of the buffer is properly aligned within the buffer.
+    // This alignment difference should be consumed by the codeAlign function
+    // call of codeScript when decoded.
+    size_t alignLen = sizeof(js::XDRAlignment);
+    if (buffer.length() % alignLen) {
+        alignLen = ComputeByteAlignment(buffer.length(), alignLen);
+        if (!buffer.appendN(0, alignLen)) {
+            ReportOutOfMemory(cx());
+            return fail(JS::TranscodeResult_Throw);
+        }
+    }
+
     // Visit the tree parts in a depth first order, to linearize the bits.
     Vector<SlicesNode::ConstRange> depthFirst(cx());
 
     SlicesTree::Ptr p = tree_.lookup(AutoXDRTree::topLevel);
     MOZ_ASSERT(p);
 
     if (!depthFirst.append(((const SlicesNode&) p->value()).all())) {
         ReportOutOfMemory(cx());
@@ -359,16 +385,18 @@ XDRIncrementalEncoder::linearize(JS::Tra
         MOZ_ASSERT_IF(slice.child == AutoXDRTree::noSubTree, iter.empty());
         if (iter.empty())
             depthFirst.popBack();
 
         // Copy the bytes associated with the current slice to the transcode
         // buffer which would be serialized.
         MOZ_ASSERT(slice.sliceBegin <= slices_.length());
         MOZ_ASSERT(slice.sliceBegin + slice.sliceLength <= slices_.length());
+        MOZ_ASSERT(buffer.length() % sizeof(XDRAlignment) == 0);
+        MOZ_ASSERT(slice.sliceLength % sizeof(XDRAlignment) == 0);
         if (!buffer.append(slices_.begin() + slice.sliceBegin, slice.sliceLength)) {
             ReportOutOfMemory(cx());
             return fail(JS::TranscodeResult_Throw);
         }
 
         // If we are at the end, go to back to the parent script.
         if (slice.child == AutoXDRTree::noSubTree)
             continue;
--- a/js/src/vm/Xdr.h
+++ b/js/src/vm/Xdr.h
@@ -19,58 +19,85 @@
 namespace js {
 
 class LifoAlloc;
 
 class XDRBufferBase
 {
   public:
     explicit XDRBufferBase(JSContext* cx, size_t cursor = 0)
-      : context_(cx), cursor_(cursor) { }
+      : context_(cx), cursor_(cursor)
+#ifdef DEBUG
+        // Note, when decoding the buffer can be set to a range, which does not
+        // have any alignment requirement as opposed to allocations.
+      , aligned_(false)
+#endif
+    { }
 
     JSContext* cx() const {
         return context_;
     }
 
     size_t cursor() const {
         return cursor_;
     }
 
+#ifdef DEBUG
+    // This function records if the cursor got changed by codeAlign or by any
+    // other read/write of data. This is used for AutoXDRTree assertions, as a
+    // way to ensure that the last thing done is properly setting the alignment
+    // with codeAlign function.
+    void setAligned(bool aligned) { aligned_ = aligned; }
+    bool isAligned() const { return aligned_; }
+#else
+    void setAligned(bool) const {}
+    bool isAligned() const { return true; }
+#endif
+
   protected:
     JSContext* const context_;
     size_t cursor_;
+#ifdef DEBUG
+    bool aligned_;
+#endif
 };
 
 template <XDRMode mode>
 class XDRBuffer;
 
 template <>
 class XDRBuffer<XDR_ENCODE> : public XDRBufferBase
 {
   public:
     XDRBuffer(JSContext* cx, JS::TranscodeBuffer& buffer, size_t cursor = 0)
       : XDRBufferBase(cx, cursor),
         buffer_(buffer) { }
 
     uint8_t* write(size_t n) {
         MOZ_ASSERT(n != 0);
+        setAligned(false);
         if (!buffer_.growByUninitialized(n)) {
             ReportOutOfMemory(cx());
             return nullptr;
         }
         uint8_t* ptr = &buffer_[cursor_];
         cursor_ += n;
         return ptr;
     }
 
     const uint8_t* read(size_t n) {
         MOZ_CRASH("Should never read in encode mode");
         return nullptr;
     }
 
+    uintptr_t uptr() const {
+        // Note: Avoid bounds check assertion if the buffer is not yet allocated.
+        return reinterpret_cast<uintptr_t>(buffer_.begin() + cursor_);
+    }
+
   private:
     JS::TranscodeBuffer& buffer_;
 };
 
 template <>
 class XDRBuffer<XDR_DECODE> : public XDRBufferBase
 {
   public:
@@ -79,37 +106,45 @@ class XDRBuffer<XDR_DECODE> : public XDR
         buffer_(range) { }
 
     XDRBuffer(JSContext* cx, JS::TranscodeBuffer& buffer, size_t cursor = 0)
       : XDRBufferBase(cx, cursor),
         buffer_(buffer.begin(), buffer.length()) { }
 
     const uint8_t* read(size_t n) {
         MOZ_ASSERT(cursor_ < buffer_.length());
+        setAligned(false);
         uint8_t* ptr = &buffer_[cursor_];
         cursor_ += n;
 
         // Don't let buggy code read past our buffer
         if (cursor_ > buffer_.length())
             return nullptr;
 
         return ptr;
     }
 
     uint8_t* write(size_t n) {
         MOZ_CRASH("Should never write in decode mode");
         return nullptr;
     }
 
+    uintptr_t uptr() const {
+        // Note: Avoid bounds check assertion at the end of the buffer.
+        return reinterpret_cast<uintptr_t>(buffer_.begin().get() + cursor_);
+    }
+
   private:
     const JS::TranscodeRange buffer_;
 };
 
 class XDRCoderBase;
 class XDRIncrementalEncoder;
+using XDRAlignment = char16_t;
+static const uint8_t AlignPadding[sizeof(XDRAlignment)] = { 0, 0 };
 
 // An AutoXDRTree is used to identify section encoded by an XDRIncrementalEncoder.
 //
 // Its primary goal is to identify functions, such that we can first encode them
 // as LazyScript, and later replaced by them by their corresponding bytecode
 // once delazified.
 //
 // As a convenience, this is also used to identify the top-level of the content
@@ -152,16 +187,17 @@ class XDRCoderBase
   protected:
     XDRCoderBase() {}
 
   public:
     virtual AutoXDRTree::Key getTopLevelTreeKey() const { return AutoXDRTree::noKey; }
     virtual AutoXDRTree::Key getTreeKey(JSFunction* fun) const { return AutoXDRTree::noKey; }
     virtual void createOrReplaceSubTree(AutoXDRTree* child) {};
     virtual void endSubTree() {};
+    virtual bool isAligned(size_t n) = 0;
 };
 
 /*
  * XDR serialization state.  All data is encoded in little endian.
  */
 template <XDRMode mode>
 class XDRState : public XDRCoderBase
 {
@@ -214,16 +250,55 @@ class XDRState : public XDRCoderBase
     bool peekData(const uint8_t** pptr, size_t length) {
         const uint8_t* ptr = buf.read(length);
         if (!ptr)
             return fail(JS::TranscodeResult_Failure_BadDecode);
         *pptr = ptr;
         return true;
     }
 
+    // Alignment is required when doing memcpy of data which contains element
+    // largers than 1 byte.
+    bool isAligned(size_t n) override {
+        MOZ_ASSERT(mozilla::IsPowerOfTwo(n));
+        // If there is a failure, always assume that we are aligned.
+        if (resultCode() != JS::TranscodeResult_Ok)
+            return true;
+        size_t mask = n - 1;
+        size_t offset = buf.uptr() & mask;
+        // In debug build, we not only check if the cursor is aligned, but also
+        // if the last cursor manipulation was made by the codeAlign function.
+        return offset == 0 && buf.isAligned();
+    }
+    bool codeAlign(size_t n) {
+        MOZ_ASSERT(mozilla::IsPowerOfTwo(n));
+        size_t mask = n - 1;
+        MOZ_ASSERT_IF(mode == XDR_ENCODE, (buf.uptr() & mask) == (buf.cursor() & mask));
+        size_t offset = buf.uptr() & mask;
+        if (offset) {
+            size_t padding = n - offset;
+            MOZ_ASSERT(padding < sizeof(AlignPadding));
+            if (mode == XDR_ENCODE) {
+                uint8_t* ptr = buf.write(padding);
+                if (!ptr)
+                    return fail(JS::TranscodeResult_Throw);
+                memcpy(ptr, AlignPadding, padding);
+            } else {
+                const uint8_t* ptr = buf.read(padding);
+                if (!ptr)
+                    return fail(JS::TranscodeResult_Failure_BadDecode);
+                if (memcmp(ptr, AlignPadding, padding) != 0)
+                    return fail(JS::TranscodeResult_Failure_BadDecode);
+            }
+        }
+        buf.setAligned(true);
+        MOZ_ASSERT(isAligned(n));
+        return true;
+    }
+
     bool codeUint8(uint8_t* n) {
         if (mode == XDR_ENCODE) {
             uint8_t* ptr = buf.write(sizeof(*n));
             if (!ptr)
                 return fail(JS::TranscodeResult_Throw);
             *ptr = *n;
         } else {
             const uint8_t* ptr = buf.read(sizeof(*n));
--- a/js/xpconnect/loader/ScriptPreloader.cpp
+++ b/js/xpconnect/loader/ScriptPreloader.cpp
@@ -47,16 +47,26 @@ static LazyLogModule gLog("ScriptPreload
 
 using mozilla::dom::AutoJSAPI;
 using mozilla::dom::ContentChild;
 using mozilla::dom::ContentParent;
 using namespace mozilla::loader;
 
 ProcessType ScriptPreloader::sProcessType;
 
+// This type correspond to js::vm::XDRAlignment type, which is used as a size
+// reference for alignment of XDR buffers.
+using XDRAlign = uint16_t;
+static const uint8_t sAlignPadding[sizeof(XDRAlign)] = { 0, 0 };
+
+static inline size_t
+ComputeByteAlignment(size_t bytes, size_t align)
+{
+    return (align - (bytes % align)) % align;
+}
 
 nsresult
 ScriptPreloader::CollectReports(nsIHandleReportCallback* aHandleReport,
                                 nsISupports* aData, bool aAnonymize)
 {
     MOZ_COLLECT_REPORT(
         "explicit/script-preloader/heap/saved-scripts", KIND_HEAP, UNITS_BYTES,
         SizeOfHashEntries<ScriptStatus::Saved>(mScripts, MallocSizeOf),
@@ -455,16 +465,18 @@ ScriptPreloader::InitCacheInternal(JS::H
     auto size = mCacheData.size();
 
     uint32_t headerSize;
     if (size < sizeof(MAGIC) + sizeof(headerSize)) {
         return Err(NS_ERROR_UNEXPECTED);
     }
 
     auto data = mCacheData.get<uint8_t>();
+    uint8_t* start = data.get();
+    MOZ_ASSERT(reinterpret_cast<uintptr_t>(start) % sizeof(XDRAlign) == 0);
     auto end = data + size;
 
     if (memcmp(MAGIC, data.get(), sizeof(MAGIC))) {
         return Err(NS_ERROR_UNEXPECTED);
     }
     data += sizeof(MAGIC);
 
     headerSize = LittleEndian::readUint32(data.get());
@@ -481,16 +493,20 @@ ScriptPreloader::InitCacheInternal(JS::H
 
         LinkedList<CachedScript> scripts;
 
         Range<uint8_t> header(data, data + headerSize);
         data += headerSize;
 
         InputBuffer buf(header);
 
+        size_t len = data.get() - start;
+        size_t alignLen = ComputeByteAlignment(len, sizeof(XDRAlign));
+        data += alignLen;
+
         size_t offset = 0;
         while (!buf.finished()) {
             auto script = MakeUnique<CachedScript>(*this, buf);
             MOZ_RELEASE_ASSERT(script);
 
             auto scriptData = data + script->mOffset;
             if (scriptData + script->mSize > end) {
                 return Err(NS_ERROR_UNEXPECTED);
@@ -498,16 +514,17 @@ ScriptPreloader::InitCacheInternal(JS::H
 
             // Make sure offsets match what we'd expect based on script ordering and
             // size, as a basic sanity check.
             if (script->mOffset != offset) {
                 return Err(NS_ERROR_UNEXPECTED);
             }
             offset += script->mSize;
 
+            MOZ_ASSERT(reinterpret_cast<uintptr_t>(scriptData.get()) % sizeof(XDRAlign) == 0);
             script->mXDRRange.emplace(scriptData, scriptData + script->mSize);
 
             // Don't pre-decode the script unless it was used in this process type during the
             // previous session.
             if (script->mOriginalProcessTypes.contains(CurrentProcessType())) {
                 scripts.insertBack(script.get());
             } else {
                 script->mReadyToExecute = true;
@@ -652,30 +669,42 @@ ScriptPreloader::WriteCache()
         // Sort scripts by load time, with async loaded scripts before sync scripts.
         // Since async scripts are always loaded immediately at startup, it helps to
         // have them stored contiguously.
         scripts.Sort(CachedScript::Comparator());
 
         OutputBuffer buf;
         size_t offset = 0;
         for (auto script : scripts) {
+            MOZ_ASSERT(offset % sizeof(XDRAlign) == 0);
             script->mOffset = offset;
             script->Code(buf);
 
             offset += script->mSize;
         }
 
         uint8_t headerSize[4];
         LittleEndian::writeUint32(headerSize, buf.cursor());
 
+        size_t len = 0;
         MOZ_TRY(Write(fd, MAGIC, sizeof(MAGIC)));
+        len += sizeof(MAGIC);
         MOZ_TRY(Write(fd, headerSize, sizeof(headerSize)));
+        len += sizeof(headerSize);
         MOZ_TRY(Write(fd, buf.Get(), buf.cursor()));
+        len += buf.cursor();
+        size_t alignLen = ComputeByteAlignment(len, sizeof(XDRAlign));
+        if (alignLen) {
+            MOZ_TRY(Write(fd, sAlignPadding, alignLen));
+            len += alignLen;
+        }
         for (auto script : scripts) {
+            MOZ_ASSERT(script->mSize % sizeof(XDRAlign) == 0);
             MOZ_TRY(Write(fd, script->Range().begin().get(), script->mSize));
+            len += script->mSize;
 
             if (script->mScript) {
                 script->FreeData();
             }
         }
     }
 
     MOZ_TRY(cacheFile->MoveTo(nullptr, mBaseName + NS_LITERAL_STRING(".bin")));