From a2cabe06f8110cb33391c18fef8c9b14feb07304 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 22 Jun 2016 14:56:18 +0200 Subject: [PATCH] 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 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Michael Dawson --- src/node_file.cc | 25 +++++-------------------- src/util.h | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index 59b40f94380bdb..fadf4cef93cd61 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -930,38 +930,23 @@ static void WriteBuffers(const FunctionCallbackInfo& args) { int64_t pos = GET_OFFSET(args[2]); Local req = args[3]; - uint32_t chunkCount = chunks->Length(); + MaybeStackBuffer 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 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); } diff --git a/src/util.h b/src/util.h index 8c2b0a1be4c47a..1973f268e4ecac 100644 --- a/src/util.h +++ b/src/util.h @@ -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_; } @@ -263,6 +273,10 @@ class MaybeStackBuffer { buf_[0] = T(); } + explicit MaybeStackBuffer(size_t storage) : MaybeStackBuffer() { + AllocateSufficientStorage(storage); + } + ~MaybeStackBuffer() { if (buf_ != buf_st_) free(buf_);