From 2adbece108c0f29dd417216173176c016440e1e5 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 9 Sep 2020 20:13:18 +0000 Subject: logd: Fix ClearUidLogs() when writer_active_ is true Previously ClearUidLogs() would Compress() the log buffer in all cases, however that is the wrong behavior when writer_active_ is true and would leave the SerializedLogChunk object in an invalid state. If more logs are written to the log, then write_offset() will be higher than the compressed size of the log, violating a CHECK() when later decompressing the log. This change does not call Compress() in ClearUidLogs() if writer_active_ is true. It upgrades a check in Compress() from a simple if statement to a CHECK() to prevent against this happening in the future. It adds a test that exercises the previously failing path. Bug: 166187079 Test: unit tests Change-Id: Ic5fbcf16f724af1c20170b8f4e6e2daadf6a9529 --- logd/SerializedLogChunk.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'logd/SerializedLogChunk.cpp') diff --git a/logd/SerializedLogChunk.cpp b/logd/SerializedLogChunk.cpp index de641d62a..e4d89451d 100644 --- a/logd/SerializedLogChunk.cpp +++ b/logd/SerializedLogChunk.cpp @@ -25,12 +25,10 @@ SerializedLogChunk::~SerializedLogChunk() { } void SerializedLogChunk::Compress() { - if (compressed_log_.size() == 0) { - CompressionEngine::GetInstance().Compress(contents_, write_offset_, compressed_log_); - LOG(INFO) << "Compressed Log, buffer max size: " << contents_.size() - << " size used: " << write_offset_ - << " compressed size: " << compressed_log_.size(); - } + CHECK_EQ(compressed_log_.size(), 0U); + CompressionEngine::GetInstance().Compress(contents_, write_offset_, compressed_log_); + LOG(INFO) << "Compressed Log, buffer max size: " << contents_.size() + << " size used: " << write_offset_ << " compressed size: " << compressed_log_.size(); } // TODO: Develop a better reference counting strategy to guard against the case where the writer is @@ -89,9 +87,11 @@ bool SerializedLogChunk::ClearUidLogs(uid_t uid, log_id_t log_id, LogStatistics* // Clear the old compressed logs and set write_offset_ appropriately to compress the new // partially cleared log. if (new_write_offset != write_offset_) { - compressed_log_.Resize(0); write_offset_ = new_write_offset; - Compress(); + if (!writer_active_) { + compressed_log_.Resize(0); + Compress(); + } } DecReaderRefCount(); -- cgit v1.2.3