Commit 6fa560d
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
2 files changed
Lines changed: 19 additions & 20 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1079,38 +1079,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) { | |||
| 1079 | 1079 | int64_t pos = GET_OFFSET(args[2]); | |
| 1080 | 1080 | Local<Value> req = args[3]; | |
| 1081 | 1081 | ||
| 1082 | - uint32_t chunkCount = chunks->Length(); | ||
| 1082 | + MaybeStackBuffer<uv_buf_t> iovs(chunks->Length()); | ||
| 1083 | 1083 | ||
| 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++) { | ||
| 1093 | 1085 | Local<Value> chunk = chunks->Get(i); | |
| 1094 | 1086 | ||
| 1095 | - if (!Buffer::HasInstance(chunk)) { | ||
| 1096 | - if (iovs != s_iovs) | ||
| 1097 | - delete[] iovs; | ||
| 1087 | + if (!Buffer::HasInstance(chunk)) | ||
| 1098 | 1088 | return env->ThrowTypeError("Array elements all need to be buffers"); | |
| 1099 | - } | ||
| 1100 | 1089 | ||
| 1101 | 1090 | iovs[i] = uv_buf_init(Buffer::Data(chunk), Buffer::Length(chunk)); | |
| 1102 | 1091 | } | |
| 1103 | 1092 | ||
| 1104 | 1093 | 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) | ||
| 1108 | 1095 | return; | |
| 1109 | 1096 | } | |
| 1110 | 1097 | ||
| 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) | ||
| 1114 | 1099 | args.GetReturnValue().Set(SYNC_RESULT); | |
| 1115 | 1100 | } | |
| 1116 | 1101 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -231,6 +231,16 @@ class MaybeStackBuffer { | |||
| 231 | 231 | return buf_; | |
| 232 | 232 | } | |
| 233 | 233 | ||
| 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 | + | ||
| 234 | 244 | size_t length() const { | |
| 235 | 245 | return length_; | |
| 236 | 246 | } | |
@@ -282,6 +292,10 @@ class MaybeStackBuffer { | |||
| 282 | 292 | buf_[0] = T(); | |
| 283 | 293 | } | |
| 284 | 294 | ||
| 295 | + explicit MaybeStackBuffer(size_t storage) : MaybeStackBuffer() { | ||
| 296 | + AllocateSufficientStorage(storage); | ||
| 297 | + } | ||
| 298 | + | ||
| 285 | 299 | ~MaybeStackBuffer() { | |
| 286 | 300 | if (buf_ != buf_st_) | |
| 287 | 301 | free(buf_); | |
0 commit comments