Skip to content

Commit df28a73

Browse files
committed
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: #1512
1 parent 5992187 commit df28a73

File tree

2 files changed

+59
-3
lines changed

2 files changed

+59
-3
lines changed

src/tls_wrap.cc

+28-2
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,38 @@ bool TLSWrap::InvokeQueued(int status) {
109109
WriteItemList queue;
110110
pending_write_items_.MoveBack(&queue);
111111
while (WriteItem* wi = queue.PopFront()) {
112-
wi->w_->Done(status);
112+
if (wi->async_) {
113+
wi->w_->Done(status);
114+
} else {
115+
CheckWriteItem* check = new CheckWriteItem(wi->w_, status);
116+
int err = uv_check_init(env()->event_loop(), &check->check_);
117+
check->check_.data = check;
118+
if (err == 0)
119+
err = uv_check_start(&check->check_, CheckWriteItem::CheckCb);
120+
121+
// No luck today, do it on next InvokeQueued
122+
if (err) {
123+
delete check;
124+
pending_write_items_.PushBack(wi);
125+
continue;
126+
}
127+
}
113128
delete wi;
114129
}
115130

116131
return true;
117132
}
118133

119134

135+
void TLSWrap::CheckWriteItem::CheckCb(uv_check_t* check) {
136+
CheckWriteItem* c = reinterpret_cast<CheckWriteItem*>(check->data);
137+
138+
c->w_->Done(c->status_);
139+
uv_close(reinterpret_cast<uv_handle_t*>(check), nullptr);
140+
delete c;
141+
}
142+
143+
120144
void TLSWrap::NewSessionDoneCb() {
121145
Cycle();
122146
}
@@ -556,7 +580,9 @@ int TLSWrap::DoWrite(WriteWrap* w,
556580
}
557581

558582
// Queue callback to execute it on next tick
559-
write_item_queue_.PushBack(new WriteItem(w));
583+
WriteItem* item = new WriteItem(w);
584+
WriteItem::SyncScope item_async(item);
585+
write_item_queue_.PushBack(item);
560586
w->Dispatched();
561587

562588
// Write queued data

src/tls_wrap.h

+31-1
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,46 @@ class TLSWrap : public crypto::SSLWrap<TLSWrap>,
6565
// Write callback queue's item
6666
class WriteItem {
6767
public:
68-
explicit WriteItem(WriteWrap* w) : w_(w) {
68+
class SyncScope {
69+
public:
70+
explicit SyncScope(WriteItem* item) : item_(item) {
71+
item_->async_ = false;
72+
}
73+
~SyncScope() {
74+
item_->async_ = true;
75+
}
76+
77+
private:
78+
WriteItem* item_;
79+
};
80+
81+
explicit WriteItem(WriteWrap* w) : w_(w), async_(false) {
6982
}
7083
~WriteItem() {
7184
w_ = nullptr;
7285
}
7386

7487
WriteWrap* w_;
88+
bool async_;
7589
ListNode<WriteItem> member_;
7690
};
7791

92+
class CheckWriteItem {
93+
public:
94+
CheckWriteItem(WriteWrap* w, int status) : w_(w), status_(status) {
95+
}
96+
97+
~CheckWriteItem() {
98+
w_ = nullptr;
99+
}
100+
101+
static void CheckCb(uv_check_t* check);
102+
103+
WriteWrap* w_;
104+
int status_;
105+
uv_check_t check_;
106+
};
107+
78108
TLSWrap(Environment* env,
79109
Kind kind,
80110
StreamBase* stream,

0 commit comments

Comments
 (0)