Commit 3eea551
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 23797eb commit 3eea551
2 files changed
Lines changed: 19 additions & 20 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -930,38 +930,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) { | |||
| 930 | 930 | int64_t pos = GET_OFFSET(args[2]); | |
| 931 | 931 | Local<Value> req = args[3]; | |
| 932 | 932 | ||
| 933 | - uint32_t chunkCount = chunks->Length(); | ||
| 933 | + MaybeStackBuffer<uv_buf_t> iovs(chunks->Length()); | ||
| 934 | 934 | ||
| 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++) { | ||
| 944 | 936 | Local<Value> chunk = chunks->Get(i); | |
| 945 | 937 | ||
| 946 | - if (!Buffer::HasInstance(chunk)) { | ||
| 947 | - if (iovs != s_iovs) | ||
| 948 | - delete[] iovs; | ||
| 938 | + if (!Buffer::HasInstance(chunk)) | ||
| 949 | 939 | return env->ThrowTypeError("Array elements all need to be buffers"); | |
| 950 | - } | ||
| 951 | 940 | ||
| 952 | 941 | iovs[i] = uv_buf_init(Buffer::Data(chunk), Buffer::Length(chunk)); | |
| 953 | 942 | } | |
| 954 | 943 | ||
| 955 | 944 | 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) | ||
| 959 | 946 | return; | |
| 960 | 947 | } | |
| 961 | 948 | ||
| 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) | ||
| 965 | 950 | args.GetReturnValue().Set(SYNC_RESULT); | |
| 966 | 951 | } | |
| 967 | 952 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -212,6 +212,16 @@ class MaybeStackBuffer { | |||
| 212 | 212 | return buf_; | |
| 213 | 213 | } | |
| 214 | 214 | ||
| 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 | + | ||
| 215 | 225 | size_t length() const { | |
| 216 | 226 | return length_; | |
| 217 | 227 | } | |
@@ -263,6 +273,10 @@ class MaybeStackBuffer { | |||
| 263 | 273 | buf_[0] = T(); | |
| 264 | 274 | } | |
| 265 | 275 | ||
| 276 | + explicit MaybeStackBuffer(size_t storage) : MaybeStackBuffer() { | ||
| 277 | + AllocateSufficientStorage(storage); | ||
| 278 | + } | ||
| 279 | + | ||
| 266 | 280 | ~MaybeStackBuffer() { | |
| 267 | 281 | if (buf_ != buf_st_) | |
| 268 | 282 | free(buf_); | |
0 commit comments