From 153fcf347cd6a53517f607ec1e6c7c14193640dd Mon Sep 17 00:00:00 2001
From: Peter Mattis <petermattis@gmail.com>
Date: Wed, 12 Dec 2018 11:28:25 -0500
Subject: [PATCH] c-deps: fix an infinite loop in MVCCScan

MVCCScan would previously loop forever if asked to scan more than 2GB of
data. The root problem was in the calculation of the next buffer size to
use in `chunkedBuffer::put` which was not accounting for an `int` being
a signed 32-bit value. Changed the logic in `chunkedBuffer::put` to use
a maximum buffer size of 128 MB to avoid hitting this problem and avoid
wasting excessive space wasteage if only a tiny bit of additional space
is needed.

Fixes #32896

Release note (bug fix): Fix an infinite loop in a low-level scanning
routine that could be hit in unusual circumstances.
---
 c-deps/libroach/CMakeLists.txt         |  1 +
 c-deps/libroach/chunked_buffer.cc      | 14 +++++++--
 c-deps/libroach/chunked_buffer_test.cc | 40 ++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 3 deletions(-)
 create mode 100644 c-deps/libroach/chunked_buffer_test.cc

diff --git a/c-deps/libroach/CMakeLists.txt b/c-deps/libroach/CMakeLists.txt
index 49522e5ca8d0..452c2449315d 100644
--- a/c-deps/libroach/CMakeLists.txt
+++ b/c-deps/libroach/CMakeLists.txt
@@ -91,6 +91,7 @@ enable_testing()
 # List of tests to build and run. Tests in `ccl/` are linked against roachccl, all others
 # are linked against roach only.
 set(tests
+  chunked_buffer_test.cc
   db_test.cc
   encoding_test.cc
   file_registry_test.cc
diff --git a/c-deps/libroach/chunked_buffer.cc b/c-deps/libroach/chunked_buffer.cc
index 525894d0d2ee..d2ff367a93f6 100644
--- a/c-deps/libroach/chunked_buffer.cc
+++ b/c-deps/libroach/chunked_buffer.cc
@@ -49,8 +49,12 @@ void chunkedBuffer::Clear() {
 // indicate that the required size of this buffer will soon be
 // len+next_size_hint, to prevent excessive resize operations.
 void chunkedBuffer::put(const char* data, int len, int next_size_hint) {
-  const int avail = bufs_.empty() ? 0 : (bufs_.back().len - (buf_ptr_ - bufs_.back().data));
-  if (len > avail) {
+  for (;;) {
+    const size_t avail = bufs_.empty() ? 0 : (bufs_.back().len - (buf_ptr_ - bufs_.back().data));
+    if (len <= avail) {
+      break;
+    }
+
     // If it's bigger than the last buf's capacity, we fill the last buf,
     // allocate a new one, and write the remainder to the new one.  Our new
     // buf's size will be the next power of two past the size of the last buf
@@ -59,8 +63,12 @@ void chunkedBuffer::put(const char* data, int len, int next_size_hint) {
     data += avail;
     len -= avail;
 
+    const int max_size = 128 << 20; // 128 MB
     int new_size = bufs_.empty() ? 16 : bufs_.back().len * 2;
-    for (; new_size < len + next_size_hint; new_size *= 2) {
+    for (; new_size < len + next_size_hint && new_size < max_size; new_size *= 2) {
+    }
+    if (new_size > max_size) {
+      new_size = max_size;
     }
 
     DBSlice new_buf;
diff --git a/c-deps/libroach/chunked_buffer_test.cc b/c-deps/libroach/chunked_buffer_test.cc
new file mode 100644
index 000000000000..4eb388549ba3
--- /dev/null
+++ b/c-deps/libroach/chunked_buffer_test.cc
@@ -0,0 +1,40 @@
+// Copyright 2018 The Cockroach Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied.  See the License for the specific language governing
+// permissions and limitations under the License.
+
+#include <gtest/gtest.h>
+#include "chunked_buffer.h"
+
+// Verify we can a chunked buffer can hold more than 2GB of data when
+// writing in pieces that are smaller than the maximum chunk size (128
+// MB). See #32896.
+TEST(ChunkedBuffer, PutSmall) {
+  const std::string data(1 << 20, '.'); // 1 MB
+  const int64_t total = 3LL << 30;      // 3 GB
+  cockroach::chunkedBuffer buf;
+  for (int64_t sum = 0; sum < total; sum += data.size()) {
+    buf.Put(data, data);
+  }
+}
+
+// Verify we can a chunked buffer can hold more than 2GB of data when
+// writing in pieces that are larger than the maximum chunk size (128
+// MB). See #32896.
+TEST(ChunkedBuffer, PutLarge) {
+  const std::string data(256 << 20, '.'); // 256 MB
+  const int64_t total = 3LL << 30;        // 3 GB
+  cockroach::chunkedBuffer buf;
+  for (int64_t sum = 0; sum < total; sum += data.size()) {
+    buf.Put(data, data);
+  }
+}