From df28a738443da3ebde03e24e4c1af68cdf9b21c3 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 29 Apr 2015 20:23:48 +0200 Subject: [PATCH] tls: ensure no synchronous callbacks Make sure that no WriteItem's callback will be invoked synchronously. Doing so may lead to the use of uninitialized `req` object, or even worse use-after-free in the caller code. Fix: https://github.com/iojs/io.js/issues/1512 --- src/tls_wrap.cc | 30 ++++++++++++++++++++++++++++-- src/tls_wrap.h | 32 +++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 79bd1fb0d847bf..8c57b0a46538ac 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -109,7 +109,22 @@ bool TLSWrap::InvokeQueued(int status) { WriteItemList queue; pending_write_items_.MoveBack(&queue); while (WriteItem* wi = queue.PopFront()) { - wi->w_->Done(status); + if (wi->async_) { + wi->w_->Done(status); + } else { + CheckWriteItem* check = new CheckWriteItem(wi->w_, status); + int err = uv_check_init(env()->event_loop(), &check->check_); + check->check_.data = check; + if (err == 0) + err = uv_check_start(&check->check_, CheckWriteItem::CheckCb); + + // No luck today, do it on next InvokeQueued + if (err) { + delete check; + pending_write_items_.PushBack(wi); + continue; + } + } delete wi; } @@ -117,6 +132,15 @@ bool TLSWrap::InvokeQueued(int status) { } +void TLSWrap::CheckWriteItem::CheckCb(uv_check_t* check) { + CheckWriteItem* c = reinterpret_cast(check->data); + + c->w_->Done(c->status_); + uv_close(reinterpret_cast(check), nullptr); + delete c; +} + + void TLSWrap::NewSessionDoneCb() { Cycle(); } @@ -556,7 +580,9 @@ int TLSWrap::DoWrite(WriteWrap* w, } // Queue callback to execute it on next tick - write_item_queue_.PushBack(new WriteItem(w)); + WriteItem* item = new WriteItem(w); + WriteItem::SyncScope item_async(item); + write_item_queue_.PushBack(item); w->Dispatched(); // Write queued data diff --git a/src/tls_wrap.h b/src/tls_wrap.h index 9f095355bb58bd..65e270c6f15f89 100644 --- a/src/tls_wrap.h +++ b/src/tls_wrap.h @@ -65,16 +65,46 @@ class TLSWrap : public crypto::SSLWrap, // Write callback queue's item class WriteItem { public: - explicit WriteItem(WriteWrap* w) : w_(w) { + class SyncScope { + public: + explicit SyncScope(WriteItem* item) : item_(item) { + item_->async_ = false; + } + ~SyncScope() { + item_->async_ = true; + } + + private: + WriteItem* item_; + }; + + explicit WriteItem(WriteWrap* w) : w_(w), async_(false) { } ~WriteItem() { w_ = nullptr; } WriteWrap* w_; + bool async_; ListNode member_; }; + class CheckWriteItem { + public: + CheckWriteItem(WriteWrap* w, int status) : w_(w), status_(status) { + } + + ~CheckWriteItem() { + w_ = nullptr; + } + + static void CheckCb(uv_check_t* check); + + WriteWrap* w_; + int status_; + uv_check_t check_; + }; + TLSWrap(Environment* env, Kind kind, StreamBase* stream,