Skip to content

Commit

Permalink
src: fix memory leak in WriteBuffers() error path
Browse files Browse the repository at this point in the history
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 <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
bnoordhuis authored and Myles Borins committed Jul 12, 2016
1 parent a81102a commit a2cabe0
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 20 deletions.
25 changes: 5 additions & 20 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -930,38 +930,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
int64_t pos = GET_OFFSET(args[2]);
Local<Value> req = args[3];

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

uv_buf_t s_iovs[1024]; // use stack allocation when possible
uv_buf_t* iovs;

if (chunkCount > arraysize(s_iovs))
iovs = new uv_buf_t[chunkCount];
else
iovs = s_iovs;

for (uint32_t i = 0; i < chunkCount; i++) {
for (uint32_t i = 0; i < iovs.length(); i++) {
Local<Value> chunk = chunks->Get(i);

if (!Buffer::HasInstance(chunk)) {
if (iovs != s_iovs)
delete[] iovs;
if (!Buffer::HasInstance(chunk))
return env->ThrowTypeError("Array elements all need to be buffers");
}

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

if (req->IsObject()) {
ASYNC_CALL(write, req, fd, iovs, chunkCount, pos)
if (iovs != s_iovs)
delete[] iovs;
ASYNC_CALL(write, req, fd, *iovs, iovs.length(), pos)
return;
}

SYNC_CALL(write, nullptr, fd, iovs, chunkCount, pos)
if (iovs != s_iovs)
delete[] iovs;
SYNC_CALL(write, nullptr, fd, *iovs, iovs.length(), pos)
args.GetReturnValue().Set(SYNC_RESULT);
}

Expand Down
14 changes: 14 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,16 @@ class MaybeStackBuffer {
return buf_;
}

T& operator[](size_t index) {
CHECK_LT(index, length());
return buf_[index];
}

const T& operator[](size_t index) const {
CHECK_LT(index, length());
return buf_[index];
}

size_t length() const {
return length_;
}
Expand Down Expand Up @@ -263,6 +273,10 @@ class MaybeStackBuffer {
buf_[0] = T();
}

explicit MaybeStackBuffer(size_t storage) : MaybeStackBuffer() {
AllocateSufficientStorage(storage);
}

~MaybeStackBuffer() {
if (buf_ != buf_st_)
free(buf_);
Expand Down

0 comments on commit a2cabe0

Please sign in to comment.