utils/chunked_managed_vector: Fix sigsegv during reserve()

Fixes the case of make_room() invoked with last_chunk_capacity_deficit but _size not in the last reserved chunk. Found during code review, no user impact. Fixes scylladb#10364. Message-Id: <[email protected]>
sarvex · Apr 12, 2022 · 0c36581 · 0c36581
1 parent 01eeb33
commit 0c36581
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 1 deletion.
diff --git a/test/boost/chunked_managed_vector_test.cc b/test/boost/chunked_managed_vector_test.cc
@@ -287,3 +287,45 @@ SEASTAR_TEST_CASE(test_correctness_when_crossing_chunk_boundary) {
     });
     return make_ready_future<>();
 }
+
+// Tests the case of make_room() invoked with last_chunk_capacity_deficit but _size not in
+// the last reserved chunk.
+SEASTAR_TEST_CASE(test_shrinking_and_expansion_involving_chunk_boundary) {
+    region region;
+    allocating_section as;
+
+    with_allocator(region.allocator(), [&] {
+        lsa::chunked_managed_vector<managed_ref<uint64_t>> v;
+
+        // Fill two chunks
+        v.reserve(2000);
+        for (uint64_t i = 0; i < 2000; ++i) {
+            as(region, [&] {
+                v.emplace_back(make_managed<uint64_t>(i));
+            });
+        }
+
+        // Make the last chunk smaller than max size to trigger the last_chunk_capacity_deficit path in make_room()
+        v.shrink_to_fit();
+
+        // Leave the last chunk reserved but empty
+        for (uint64_t i = 0; i < 1000; ++i) {
+            v.pop_back();
+        }
+
+        // Try to reserve more than the currently reserved capacity and trigger last_chunk_capacity_deficit path
+        // with _size not in the last chunk. Should not sigsegv.
+        v.reserve(8000);
+
+        for (uint64_t i = 0; i < 2000; ++i) {
+            as(region, [&] {
+                v.emplace_back(make_managed<uint64_t>(i));
+            });
+        }
+
+        v.clear_and_release();
+    });
+
+    return make_ready_future<>();
+}
+
diff --git a/utils/lsa/chunked_managed_vector.hh b/utils/lsa/chunked_managed_vector.hh
@@ -385,7 +385,9 @@ chunked_managed_vector<T>::make_room(size_t n, bool stop_after_one) {
         auto new_last_chunk_capacity = last_chunk_capacity + capacity_increase;
         // FIXME: realloc? maybe not worth the complication; only works for PODs
         auto new_last_chunk = new_chunk(new_last_chunk_capacity);
-        migrate(addr(_capacity - last_chunk_capacity), addr(_size), new_last_chunk);
+        if (_size > _capacity - last_chunk_capacity) {
+            migrate(addr(_capacity - last_chunk_capacity), addr(_size), new_last_chunk);
+        }
         _chunks.back() = std::move(new_last_chunk);
         _capacity += capacity_increase;
     }