src: fix memory leak in WriteBuffers() error path

bnoordhuis · Fishrock123 · commit 6fa560dce962 · 2016-07-05T22:36:46.000+02:00
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>
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -1079,38 +1079,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
   int64_t pos = GET_OFFSET(args[2]);
   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
-  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<Value> 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, UTF8, fd, iovs, chunkCount, pos)
-    if (iovs != s_iovs)
-      delete[] iovs;
+    ASYNC_CALL(write, req, UTF8, 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
@@ -231,6 +231,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_;
   }
@@ -282,6 +292,10 @@ class MaybeStackBuffer {
     buf_[0] = T();
   }
 
+  explicit MaybeStackBuffer(size_t storage) : MaybeStackBuffer() {
+    AllocateSufficientStorage(storage);
+  }
+
   ~MaybeStackBuffer() {
     if (buf_ != buf_st_)
       free(buf_);
