Skip to content

Commit 3eea551

Browse files
bnoordhuisMyles Borins
authored and
Myles Borins
committed
src: fix memory leak in WriteBuffers() error path
Pointed out by Coverity. Introduced in commit 05d30d5 from July 2015 ("fs: implemented WriteStream#writev"). WriteBuffers() leaked memory in the synchronous uv_fs_write() error path when trying to write > 1024 buffers. PR-URL: #7374 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent 23797eb commit 3eea551

File tree

2 files changed

+19
-20
lines changed

2 files changed

+19
-20
lines changed

src/node_file.cc

+5-20
Original file line numberDiff line numberDiff line change
@@ -930,38 +930,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
930930
int64_t pos = GET_OFFSET(args[2]);
931931
Local<Value> req = args[3];
932932

933-
uint32_t chunkCount = chunks->Length();
933+
MaybeStackBuffer<uv_buf_t> iovs(chunks->Length());
934934

935-
uv_buf_t s_iovs[1024]; // use stack allocation when possible
936-
uv_buf_t* iovs;
937-
938-
if (chunkCount > arraysize(s_iovs))
939-
iovs = new uv_buf_t[chunkCount];
940-
else
941-
iovs = s_iovs;
942-
943-
for (uint32_t i = 0; i < chunkCount; i++) {
935+
for (uint32_t i = 0; i < iovs.length(); i++) {
944936
Local<Value> chunk = chunks->Get(i);
945937

946-
if (!Buffer::HasInstance(chunk)) {
947-
if (iovs != s_iovs)
948-
delete[] iovs;
938+
if (!Buffer::HasInstance(chunk))
949939
return env->ThrowTypeError("Array elements all need to be buffers");
950-
}
951940

952941
iovs[i] = uv_buf_init(Buffer::Data(chunk), Buffer::Length(chunk));
953942
}
954943

955944
if (req->IsObject()) {
956-
ASYNC_CALL(write, req, fd, iovs, chunkCount, pos)
957-
if (iovs != s_iovs)
958-
delete[] iovs;
945+
ASYNC_CALL(write, req, fd, *iovs, iovs.length(), pos)
959946
return;
960947
}
961948

962-
SYNC_CALL(write, nullptr, fd, iovs, chunkCount, pos)
963-
if (iovs != s_iovs)
964-
delete[] iovs;
949+
SYNC_CALL(write, nullptr, fd, *iovs, iovs.length(), pos)
965950
args.GetReturnValue().Set(SYNC_RESULT);
966951
}
967952

src/util.h

+14
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,16 @@ class MaybeStackBuffer {
212212
return buf_;
213213
}
214214

215+
T& operator[](size_t index) {
216+
CHECK_LT(index, length());
217+
return buf_[index];
218+
}
219+
220+
const T& operator[](size_t index) const {
221+
CHECK_LT(index, length());
222+
return buf_[index];
223+
}
224+
215225
size_t length() const {
216226
return length_;
217227
}
@@ -263,6 +273,10 @@ class MaybeStackBuffer {
263273
buf_[0] = T();
264274
}
265275

276+
explicit MaybeStackBuffer(size_t storage) : MaybeStackBuffer() {
277+
AllocateSufficientStorage(storage);
278+
}
279+
266280
~MaybeStackBuffer() {
267281
if (buf_ != buf_st_)
268282
free(buf_);

0 commit comments

Comments
 (0)