Browse Source

src: fix memory leak in WriteBuffers() error path

Pointed out by Coverity.  Introduced in commit 05d30d53 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: https://github.com/nodejs/node/pull/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>
v7.x
Ben Noordhuis 9 years ago
parent
commit
c50e192204
  1. 25
      src/node_file.cc
  2. 14
      src/util.h

25
src/node_file.cc

@ -1079,38 +1079,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
int64_t pos = GET_OFFSET(args[2]); int64_t pos = GET_OFFSET(args[2]);
Local<Value> req = args[3]; 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 for (uint32_t i = 0; i < iovs.length(); i++) {
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++) {
Local<Value> chunk = chunks->Get(i); Local<Value> chunk = chunks->Get(i);
if (!Buffer::HasInstance(chunk)) { if (!Buffer::HasInstance(chunk))
if (iovs != s_iovs)
delete[] iovs;
return env->ThrowTypeError("Array elements all need to be buffers"); return env->ThrowTypeError("Array elements all need to be buffers");
}
iovs[i] = uv_buf_init(Buffer::Data(chunk), Buffer::Length(chunk)); iovs[i] = uv_buf_init(Buffer::Data(chunk), Buffer::Length(chunk));
} }
if (req->IsObject()) { if (req->IsObject()) {
ASYNC_CALL(write, req, UTF8, fd, iovs, chunkCount, pos) ASYNC_CALL(write, req, UTF8, fd, *iovs, iovs.length(), pos)
if (iovs != s_iovs)
delete[] iovs;
return; return;
} }
SYNC_CALL(write, nullptr, fd, iovs, chunkCount, pos) SYNC_CALL(write, nullptr, fd, *iovs, iovs.length(), pos)
if (iovs != s_iovs)
delete[] iovs;
args.GetReturnValue().Set(SYNC_RESULT); args.GetReturnValue().Set(SYNC_RESULT);
} }

14
src/util.h

@ -231,6 +231,16 @@ class MaybeStackBuffer {
return buf_; 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 { size_t length() const {
return length_; return length_;
} }
@ -282,6 +292,10 @@ class MaybeStackBuffer {
buf_[0] = T(); buf_[0] = T();
} }
explicit MaybeStackBuffer(size_t storage) : MaybeStackBuffer() {
AllocateSufficientStorage(storage);
}
~MaybeStackBuffer() { ~MaybeStackBuffer() {
if (buf_ != buf_st_) if (buf_ != buf_st_)
free(buf_); free(buf_);

Loading…
Cancel
Save