Skip to content

Commit d4b0fe7

Browse files
author
Lars T Hansen
committed
Bug 1352681 - Overflow checking on SAB reference count. r=sfink
--HG-- extra : rebase_source : 86549600fd1fc755c490546052e1bbd40326331d
1 parent 997e7ce commit d4b0fe7

File tree

5 files changed

+57
-27
lines changed

5 files changed

+57
-27
lines changed

js/src/js.msg

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,7 @@ MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE, 0, JSEXN_TYPEERR, "unsupported type for s
434434
MSG_DEF(JSMSG_SC_NOT_CLONABLE, 1, JSEXN_TYPEERR, "{0} cannot be cloned in this context")
435435
MSG_DEF(JSMSG_SC_SAB_TRANSFERABLE, 0, JSEXN_TYPEERR, "SharedArrayBuffer must not be in the transfer list")
436436
MSG_DEF(JSMSG_SC_SAB_DISABLED, 0, JSEXN_TYPEERR, "SharedArrayBuffer not cloned - shared memory disabled in receiver")
437+
MSG_DEF(JSMSG_SC_SAB_REFCNT_OFLO, 0, JSEXN_TYPEERR, "SharedArrayBuffer has too many references")
437438

438439
// Debugger
439440
MSG_DEF(JSMSG_ASSIGN_FUNCTION_OR_NULL, 1, JSEXN_TYPEERR, "value assigned to {0} must be a function or null")

js/src/shell/js.cpp

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5515,25 +5515,31 @@ GetSharedArrayBuffer(JSContext* cx, unsigned argc, Value* vp)
55155515
{
55165516
CallArgs args = CallArgsFromVp(argc, vp);
55175517
JSObject* newObj = nullptr;
5518-
bool rval = true;
5519-
5520-
sharedArrayBufferMailboxLock->lock();
5521-
SharedArrayRawBuffer* buf = sharedArrayBufferMailbox;
5522-
if (buf) {
5523-
buf->addReference();
5524-
// Shared memory is enabled globally in the shell: there can't be a worker
5525-
// that does not enable it if the main thread has it.
5526-
MOZ_ASSERT(cx->compartment()->creationOptions().getSharedMemoryAndAtomicsEnabled());
5527-
newObj = SharedArrayBufferObject::New(cx, buf);
5528-
if (!newObj) {
5529-
buf->dropReference();
5530-
rval = false;
5518+
5519+
{
5520+
sharedArrayBufferMailboxLock->lock();
5521+
auto unlockMailbox = MakeScopeExit([]() { sharedArrayBufferMailboxLock->unlock(); });
5522+
5523+
SharedArrayRawBuffer* buf = sharedArrayBufferMailbox;
5524+
if (buf) {
5525+
if (!buf->addReference()) {
5526+
JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_SC_SAB_REFCNT_OFLO);
5527+
return false;
5528+
}
5529+
5530+
// Shared memory is enabled globally in the shell: there can't be a worker
5531+
// that does not enable it if the main thread has it.
5532+
MOZ_ASSERT(cx->compartment()->creationOptions().getSharedMemoryAndAtomicsEnabled());
5533+
newObj = SharedArrayBufferObject::New(cx, buf);
5534+
if (!newObj) {
5535+
buf->dropReference();
5536+
return false;
5537+
}
55315538
}
55325539
}
5533-
sharedArrayBufferMailboxLock->unlock();
55345540

55355541
args.rval().setObjectOrNull(newObj);
5536-
return rval;
5542+
return true;
55375543
}
55385544

55395545
static bool
@@ -5547,18 +5553,24 @@ SetSharedArrayBuffer(JSContext* cx, unsigned argc, Value* vp)
55475553
}
55485554
else if (args.get(0).isObject() && args[0].toObject().is<SharedArrayBufferObject>()) {
55495555
newBuffer = args[0].toObject().as<SharedArrayBufferObject>().rawBufferObject();
5550-
newBuffer->addReference();
5556+
if (!newBuffer->addReference()) {
5557+
JS_ReportErrorASCII(cx, "Reference count overflow on SharedArrayBuffer");
5558+
return false;
5559+
}
55515560
} else {
55525561
JS_ReportErrorASCII(cx, "Only a SharedArrayBuffer can be installed in the global mailbox");
55535562
return false;
55545563
}
55555564

5556-
sharedArrayBufferMailboxLock->lock();
5557-
SharedArrayRawBuffer* oldBuffer = sharedArrayBufferMailbox;
5558-
if (oldBuffer)
5559-
oldBuffer->dropReference();
5560-
sharedArrayBufferMailbox = newBuffer;
5561-
sharedArrayBufferMailboxLock->unlock();
5565+
{
5566+
sharedArrayBufferMailboxLock->lock();
5567+
auto unlockMailbox = MakeScopeExit([]() { sharedArrayBufferMailboxLock->unlock(); });
5568+
5569+
SharedArrayRawBuffer* oldBuffer = sharedArrayBufferMailbox;
5570+
if (oldBuffer)
5571+
oldBuffer->dropReference();
5572+
sharedArrayBufferMailbox = newBuffer;
5573+
}
55625574

55635575
args.rval().setUndefined();
55645576
return true;

js/src/vm/SharedArrayObject.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,16 +165,30 @@ SharedArrayRawBuffer::New(JSContext* cx, uint32_t length)
165165
return rawbuf;
166166
}
167167

168-
void
168+
bool
169169
SharedArrayRawBuffer::addReference()
170170
{
171-
MOZ_ASSERT(this->refcount_ > 0);
172-
++this->refcount_; // Atomic.
171+
MOZ_RELEASE_ASSERT(this->refcount_ > 0);
172+
173+
// Be careful never to overflow the refcount field.
174+
for (;;) {
175+
uint32_t old_refcount = this->refcount_;
176+
uint32_t new_refcount = old_refcount+1;
177+
if (new_refcount == 0)
178+
return false;
179+
if (this->refcount_.compareExchange(old_refcount, new_refcount))
180+
return true;
181+
}
173182
}
174183

175184
void
176185
SharedArrayRawBuffer::dropReference()
177186
{
187+
// Normally if the refcount is zero then the memory will have been unmapped
188+
// and this test may just crash, but if the memory has been retained for any
189+
// reason we will catch the underflow here.
190+
MOZ_RELEASE_ASSERT(this->refcount_ > 0);
191+
178192
// Drop the reference to the buffer.
179193
uint32_t refcount = --this->refcount_; // Atomic.
180194
if (refcount)

js/src/vm/SharedArrayObject.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ class SharedArrayRawBuffer
9292

9393
uint32_t refcount() const { return refcount_; }
9494

95-
void addReference();
95+
MOZ_MUST_USE bool addReference();
9696
void dropReference();
9797
};
9898

js/src/vm/StructuredClone.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1163,7 +1163,10 @@ JSStructuredCloneWriter::writeSharedArrayBuffer(HandleObject obj)
11631163

11641164
// Avoids a race condition where the parent thread frees the buffer
11651165
// before the child has accepted the transferable.
1166-
rawbuf->addReference();
1166+
if (!rawbuf->addReference()) {
1167+
JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_SAB_REFCNT_OFLO);
1168+
return false;
1169+
}
11671170

11681171
intptr_t p = reinterpret_cast<intptr_t>(rawbuf);
11691172
return out.writePair(SCTAG_SHARED_ARRAY_BUFFER_OBJECT, static_cast<uint32_t>(sizeof(p))) &&

0 commit comments

Comments
 (0)