logd: fix use after resize of contents_ vector
SerializedFlushToState::PopNextUnreadLog() was calling
AddMinHeapEntry() to replenish the element that was just popped off of
the heap, however AddMinHeapEntry() also manages reference counts for
the buffers, and this resulting in the following scenario:
PopNextUnreadLog() returns a pointer referencing log buffer #1
AddMinHeapEntry() sees that all logs from buffer #1 has been read, so
it decrements the reference count
The caller of PopNextUnreadLog() uses the result which references
invalid memory.
This calls CheckForNewLogs() within HasUnreadLogs() instead of
requiring a separate call, which fixes an additional issue where
continuing from the loop in SerializedLogBuffer::FlushTo() may not
pick up subsequent logs in a given log buffer, since CheckForNewLogs()
wouldn't be called. This was exacerbated by the above change.
This adds a test to check the reference counts for this case and fixes
an argument mismatch in SerializedFlushToStateTest.
This adds the corpus that surfaced the issue.
Bug: 159753229
Bug: 159783005
Test: these unit tests, run fuzzer without error
Change-Id: Ib2636dfc14293b7e2cd00876b9def6e9dbbff4ce
diff --git a/logd/SerializedFlushToStateTest.cpp b/logd/SerializedFlushToStateTest.cpp
index a1d21ac..f4515c8 100644
--- a/logd/SerializedFlushToStateTest.cpp
+++ b/logd/SerializedFlushToStateTest.cpp
@@ -89,7 +89,6 @@
for (uint32_t mask = 0; mask < max_mask; ++mask) {
auto state = SerializedFlushToState{sequence, mask};
state.InitializeLogs(log_chunks_);
- state.CheckForNewLogs();
TestReading(sequence, mask, state);
}
}
@@ -109,7 +108,6 @@
state.InitializeLogs(log_chunks_);
int loop_count = 0;
while (write_logs(loop_count++)) {
- state.CheckForNewLogs();
TestReading(sequence, mask, state);
sequence_numbers_per_buffer_.clear();
}
@@ -149,7 +147,7 @@
// Add a chunk with the given messages to the a given log buffer. Keep track of the sequence
// numbers for future validation. Optionally mark the block as having finished writing.
- void AddChunkWithMessages(int buffer, bool finish_writing,
+ void AddChunkWithMessages(bool finish_writing, int buffer,
const std::vector<std::string>& messages) {
auto chunk = SerializedLogChunk{kChunkSize};
for (const auto& message : messages) {
@@ -252,3 +250,41 @@
TestAllReadingWithFutureMessages(write_logs);
}
+
+TEST_F(SerializedFlushToStateTest, no_dangling_references) {
+ AddChunkWithMessages(true, 0, {"1st", "2nd"});
+ AddChunkWithMessages(true, 0, {"3rd", "4th"});
+
+ auto state = SerializedFlushToState{1, kLogMaskAll};
+ state.InitializeLogs(log_chunks_);
+
+ ASSERT_EQ(log_chunks_[0].size(), 2U);
+ auto first_chunk = log_chunks_[0].begin();
+ auto second_chunk = std::next(first_chunk);
+
+ ASSERT_TRUE(state.HasUnreadLogs());
+ auto first_log = state.PopNextUnreadLog();
+ EXPECT_STREQ(first_log.entry->msg(), "1st");
+ EXPECT_EQ(first_chunk->reader_ref_count(), 1U);
+ EXPECT_EQ(second_chunk->reader_ref_count(), 0U);
+
+ ASSERT_TRUE(state.HasUnreadLogs());
+ auto second_log = state.PopNextUnreadLog();
+ EXPECT_STREQ(second_log.entry->msg(), "2nd");
+ EXPECT_EQ(first_chunk->reader_ref_count(), 1U);
+ EXPECT_EQ(second_chunk->reader_ref_count(), 0U);
+
+ ASSERT_TRUE(state.HasUnreadLogs());
+ auto third_log = state.PopNextUnreadLog();
+ EXPECT_STREQ(third_log.entry->msg(), "3rd");
+ EXPECT_EQ(first_chunk->reader_ref_count(), 0U);
+ EXPECT_EQ(second_chunk->reader_ref_count(), 1U);
+
+ ASSERT_TRUE(state.HasUnreadLogs());
+ auto fourth_log = state.PopNextUnreadLog();
+ EXPECT_STREQ(fourth_log.entry->msg(), "4th");
+ EXPECT_EQ(first_chunk->reader_ref_count(), 0U);
+ EXPECT_EQ(second_chunk->reader_ref_count(), 1U);
+
+ EXPECT_FALSE(state.HasUnreadLogs());
+}
\ No newline at end of file