Skip to content

Commit 6fa560d

Browse files
bnoordhuisFishrock123
authored andcommitted
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 <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>
1 parent ce039c3 commit 6fa560d

File tree

2 files changed

+19
-20
lines changed

2 files changed

+19
-20
lines changed

src/node_file.cc

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,38 +1079,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
10791079
int64_t pos = GET_OFFSET(args[2]);
10801080
Local<Value> req = args[3];
10811081

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

1084-
uv_buf_t s_iovs[1024]; // use stack allocation when possible
1085-
uv_buf_t* iovs;
1086-
1087-
if (chunkCount > arraysize(s_iovs))
1088-
iovs = new uv_buf_t[chunkCount];
1089-
else
1090-
iovs = s_iovs;
1091-
1092-
for (uint32_t i = 0; i < chunkCount; i++) {
1084+
for (uint32_t i = 0; i < iovs.length(); i++) {
10931085
Local<Value> chunk = chunks->Get(i);
10941086

1095-
if (!Buffer::HasInstance(chunk)) {
1096-
if (iovs != s_iovs)
1097-
delete[] iovs;
1087+
if (!Buffer::HasInstance(chunk))
10981088
return env->ThrowTypeError("Array elements all need to be buffers");
1099-
}
11001089

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

11041093
if (req->IsObject()) {
1105-
ASYNC_CALL(write, req, UTF8, fd, iovs, chunkCount, pos)
1106-
if (iovs != s_iovs)
1107-
delete[] iovs;
1094+
ASYNC_CALL(write, req, UTF8, fd, *iovs, iovs.length(), pos)
11081095
return;
11091096
}
11101097

1111-
SYNC_CALL(write, nullptr, fd, iovs, chunkCount, pos)
1112-
if (iovs != s_iovs)
1113-
delete[] iovs;
1098+
SYNC_CALL(write, nullptr, fd, *iovs, iovs.length(), pos)
11141099
args.GetReturnValue().Set(SYNC_RESULT);
11151100
}
11161101

src/util.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,16 @@ class MaybeStackBuffer {
231231
return buf_;
232232
}
233233

234+
T& operator[](size_t index) {
235+
CHECK_LT(index, length());
236+
return buf_[index];
237+
}
238+
239+
const T& operator[](size_t index) const {
240+
CHECK_LT(index, length());
241+
return buf_[index];
242+
}
243+
234244
size_t length() const {
235245
return length_;
236246
}
@@ -282,6 +292,10 @@ class MaybeStackBuffer {
282292
buf_[0] = T();
283293
}
284294

295+
explicit MaybeStackBuffer(size_t storage) : MaybeStackBuffer() {
296+
AllocateSufficientStorage(storage);
297+
}
298+
285299
~MaybeStackBuffer() {
286300
if (buf_ != buf_st_)
287301
free(buf_);

0 commit comments

Comments
 (0)