diff --git a/js/src/js.msg b/js/src/js.msg index f21a371eff..5125cb12b8 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -445,6 +445,7 @@ MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE, 0, JSEXN_TYPEERR, "unsupported type for s MSG_DEF(JSMSG_SC_NOT_CLONABLE, 1, JSEXN_TYPEERR, "{0} cannot be cloned in this context") MSG_DEF(JSMSG_SC_SAB_TRANSFER, 0, JSEXN_WARN, "SharedArrayBuffer must not be in the transfer list") MSG_DEF(JSMSG_SC_SAB_DISABLED, 0, JSEXN_TYPEERR, "SharedArrayBuffer not cloned - shared memory disabled in receiver") +MSG_DEF(JSMSG_SC_SAB_TOO_MANY_REFS, 0, JSEXN_TYPEERR, "SharedArrayBuffer has too many references") // Debugger MSG_DEF(JSMSG_ASSIGN_FUNCTION_OR_NULL, 1, JSEXN_TYPEERR, "value assigned to {0} must be a function or null") diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 387c78849f..6dd903e20b 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -5347,25 +5347,31 @@ GetSharedArrayBuffer(JSContext* cx, unsigned argc, Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); JSObject* newObj = nullptr; - bool rval = true; - sharedArrayBufferMailboxLock->lock(); - SharedArrayRawBuffer* buf = sharedArrayBufferMailbox; - if (buf) { - buf->addReference(); - // Shared memory is enabled globally in the shell: there can't be a worker - // that does not enable it if the main thread has it. - MOZ_ASSERT(cx->compartment()->creationOptions().getSharedMemoryAndAtomicsEnabled()); - newObj = SharedArrayBufferObject::New(cx, buf); - if (!newObj) { - buf->dropReference(); - rval = false; + { + sharedArrayBufferMailboxLock->lock(); + auto unlockMailbox = MakeScopeExit([]() { sharedArrayBufferMailboxLock->unlock(); }); + + SharedArrayRawBuffer* buf = sharedArrayBufferMailbox; + if (buf) { + if (!buf->addReference()) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_SC_SAB_REFCNT_OFLO); + return false; + } + + // Shared memory is enabled globally in the shell: there can't be a worker + // that does not enable it if the main thread has it. + MOZ_ASSERT(cx->compartment()->creationOptions().getSharedMemoryAndAtomicsEnabled()); + newObj = SharedArrayBufferObject::New(cx, buf); + if (!newObj) { + buf->dropReference(); + return false; + } } } - sharedArrayBufferMailboxLock->unlock(); args.rval().setObjectOrNull(newObj); - return rval; + return true; } static bool @@ -5379,18 +5385,24 @@ SetSharedArrayBuffer(JSContext* cx, unsigned argc, Value* vp) } else if (args.get(0).isObject() && args[0].toObject().is()) { newBuffer = args[0].toObject().as().rawBufferObject(); - newBuffer->addReference(); + if (!newBuffer->addReference()) { + JS_ReportErrorASCII(cx, "Reference count overflow on SharedArrayBuffer"); + return false; + } } else { JS_ReportErrorASCII(cx, "Only a SharedArrayBuffer can be installed in the global mailbox"); return false; } - sharedArrayBufferMailboxLock->lock(); - SharedArrayRawBuffer* oldBuffer = sharedArrayBufferMailbox; - if (oldBuffer) - oldBuffer->dropReference(); - sharedArrayBufferMailbox = newBuffer; - sharedArrayBufferMailboxLock->unlock(); + { + sharedArrayBufferMailboxLock->lock(); + auto unlockMailbox = MakeScopeExit([]() { sharedArrayBufferMailboxLock->unlock(); }); + + SharedArrayRawBuffer* oldBuffer = sharedArrayBufferMailbox; + if (oldBuffer) + oldBuffer->dropReference(); + sharedArrayBufferMailbox = newBuffer; + } args.rval().setUndefined(); return true; diff --git a/js/src/vm/SharedArrayObject.cpp b/js/src/vm/SharedArrayObject.cpp index 44fe3b790d..6a3c6a91c3 100644 --- a/js/src/vm/SharedArrayObject.cpp +++ b/js/src/vm/SharedArrayObject.cpp @@ -166,16 +166,30 @@ SharedArrayRawBuffer::New(JSContext* cx, uint32_t length) return rawbuf; } -void +bool SharedArrayRawBuffer::addReference() { MOZ_ASSERT(this->refcount_ > 0); - ++this->refcount_; // Atomic. + + // Be careful never to overflow the refcount field. + for (;;) { + uint32_t old_refcount = this->refcount_; + uint32_t new_refcount = old_refcount+1; + if (new_refcount == 0) + return false; + if (this->refcount_.compareExchange(old_refcount, new_refcount)) + return true; + } } void SharedArrayRawBuffer::dropReference() { + // Normally if the refcount is zero then the memory will have been unmapped + // and this test may just crash, but if the memory has been retained for any + // reason we will catch the underflow here. + MOZ_RELEASE_ASSERT(this->refcount_ > 0); + // Drop the reference to the buffer. uint32_t refcount = --this->refcount_; // Atomic. if (refcount) diff --git a/js/src/vm/SharedArrayObject.h b/js/src/vm/SharedArrayObject.h index 19048336cb..d6b18b61a3 100644 --- a/js/src/vm/SharedArrayObject.h +++ b/js/src/vm/SharedArrayObject.h @@ -91,7 +91,7 @@ class SharedArrayRawBuffer uint32_t refcount() const { return refcount_; } - void addReference(); + [[nodiscard]] bool addReference(); void dropReference(); }; diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp index 79f2d23d96..e1f1b2777e 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/js/src/vm/StructuredClone.cpp @@ -1201,9 +1201,12 @@ JSStructuredCloneWriter::writeSharedArrayBuffer(HandleObject obj) Rooted sharedArrayBuffer(context(), &CheckedUnwrap(obj)->as()); SharedArrayRawBuffer* rawbuf = sharedArrayBuffer->rawBufferObject(); - // Avoids a race condition where the parent thread frees the buffer + // Adding the reference here avoids a race condition where the parent thread frees the buffer // before the child has accepted the transferable. - rawbuf->addReference(); + if (!rawbuf->addReference()) { + JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_SAB_TOO_MANY_REFS); + return false; + } intptr_t p = reinterpret_cast(rawbuf); return out.writePair(SCTAG_SHARED_ARRAY_BUFFER_OBJECT, static_cast(sizeof(p))) &&