Create a minimal testcase to reproduce silent verity corruption
b/186196758 is triggered by the following sequence of events:
1. update_engine finish writing all install ops, emits kEndOfInstall
label
2. update_engine opens cow in append mode, invokes
InitialiazeAppend(kEndOfInstall)
3. update_engine writes verity data, invokes SnapshotWriter::Finalize()
4. update_engine repeats step 2, but does not write any data after
opening SnapshotWriter. Instead, it reads verity and make sure the hash
matches what's specified in OTA payload.
5. Reboot device, verity data corrupted, device rollback to slot _a.
This is because, during step 4, when calling
InitializeAppend(kEndOfInstall), the SnapshotWriter only reads up to the
given label. But OpenReader() completely disregards the resume label and
reads all ops. Therefore, update_engine sees the verity data, and
determines that everything is fine. However, when calling
SnapshotWriter::Finalize(), data after resume label are discarded,
therefore verity data is gone.
Test: th
Bug: 186196758
Change-Id: I0166271b64eb7b574434d617ce730f345ca93ff1
diff --git a/Android.bp b/Android.bp
index 11c03b4..d74e78f 100644
--- a/Android.bp
+++ b/Android.bp
@@ -792,6 +792,7 @@
"libcurl_http_fetcher_unittest.cc",
"payload_consumer/bzip_extent_writer_unittest.cc",
"payload_consumer/cached_file_descriptor_unittest.cc",
+ "payload_consumer/cow_writer_file_descriptor_unittest.cc",
"payload_consumer/certificate_parser_android_unittest.cc",
"payload_consumer/delta_performer_integration_test.cc",
"payload_consumer/delta_performer_unittest.cc",
diff --git a/payload_consumer/cow_writer_file_descriptor_unittest.cc b/payload_consumer/cow_writer_file_descriptor_unittest.cc
new file mode 100644
index 0000000..c596e3b
--- /dev/null
+++ b/payload_consumer/cow_writer_file_descriptor_unittest.cc
@@ -0,0 +1,120 @@
+//
+// Copyright (C) 2021 The Android Open Source Project
+//
+// 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 "update_engine/payload_consumer/cow_writer_file_descriptor.h"
+
+#include <cstring>
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include <android-base/unique_fd.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <libsnapshot/snapshot_writer.h>
+
+#include "update_engine/common/utils.h"
+
+namespace chromeos_update_engine {
+constexpr size_t BLOCK_SIZE = 4096;
+constexpr size_t PARTITION_SIZE = BLOCK_SIZE * 10;
+
+using android::base::unique_fd;
+using android::snapshot::CompressedSnapshotWriter;
+using android::snapshot::CowOptions;
+using android::snapshot::ISnapshotWriter;
+
+class CowWriterFileDescriptorUnittest : public ::testing::Test {
+ public:
+ void SetUp() override {
+ ASSERT_EQ(ftruncate64(cow_device_file_.fd(), PARTITION_SIZE), 0)
+ << "Failed to truncate cow_device file to " << PARTITION_SIZE
+ << strerror(errno);
+ ASSERT_EQ(ftruncate64(cow_source_file_.fd(), PARTITION_SIZE), 0)
+ << "Failed to truncate cow_source file to " << PARTITION_SIZE
+ << strerror(errno);
+ }
+
+ std::unique_ptr<CompressedSnapshotWriter> GetCowWriter() {
+ const CowOptions options{.block_size = BLOCK_SIZE, .compression = "gz"};
+ auto snapshot_writer = std::make_unique<CompressedSnapshotWriter>(options);
+ int fd = open(cow_device_file_.path().c_str(), O_RDWR);
+ EXPECT_NE(fd, -1);
+ EXPECT_TRUE(snapshot_writer->SetCowDevice(unique_fd{fd}));
+ snapshot_writer->SetSourceDevice(cow_source_file_.path());
+ return snapshot_writer;
+ }
+ CowWriterFileDescriptor GetCowFd() {
+ auto cow_writer = GetCowWriter();
+ return CowWriterFileDescriptor{std::move(cow_writer)};
+ }
+
+ ScopedTempFile cow_source_file_{"cow_source.XXXXXX", true};
+ ScopedTempFile cow_device_file_{"cow_device.XXXXXX", true};
+};
+
+TEST_F(CowWriterFileDescriptorUnittest, ReadAfterWrite) {
+ std::vector<unsigned char> buffer;
+ buffer.resize(BLOCK_SIZE);
+ std::fill(buffer.begin(), buffer.end(), 234);
+
+ std::vector<unsigned char> verity_data;
+ verity_data.resize(BLOCK_SIZE);
+ std::fill(verity_data.begin(), verity_data.end(), 0xAA);
+
+ auto cow_writer = GetCowWriter();
+ cow_writer->Initialize();
+
+ // Simulate Writing InstallOp data
+ ASSERT_TRUE(cow_writer->AddRawBlocks(0, buffer.data(), buffer.size()));
+ ASSERT_TRUE(cow_writer->AddZeroBlocks(1, 2));
+ ASSERT_TRUE(cow_writer->AddCopy(3, 1));
+ // Fake label to simulate "end of install"
+ ASSERT_TRUE(cow_writer->AddLabel(23));
+ ASSERT_TRUE(
+ cow_writer->AddRawBlocks(4, verity_data.data(), verity_data.size()));
+ ASSERT_TRUE(cow_writer->Finalize());
+
+ cow_writer = GetCowWriter();
+ ASSERT_NE(nullptr, cow_writer);
+ ASSERT_TRUE(cow_writer->InitializeAppend(23));
+ auto cow_fd =
+ std::make_unique<CowWriterFileDescriptor>(std::move(cow_writer));
+
+ ASSERT_EQ((ssize_t)BLOCK_SIZE * 4, cow_fd->Seek(BLOCK_SIZE * 4, SEEK_SET));
+ std::vector<unsigned char> read_back(4096);
+ ASSERT_EQ((ssize_t)read_back.size(),
+ cow_fd->Read(read_back.data(), read_back.size()));
+ ASSERT_EQ(verity_data, read_back);
+
+ // Since we didn't write anything to this instance of cow_fd, destructor
+ // should not call Finalize(). As finalize will drop ops after resume label,
+ // causing subsequent reads to fail.
+ cow_writer = GetCowWriter();
+ ASSERT_NE(nullptr, cow_writer);
+ ASSERT_TRUE(cow_writer->InitializeAppend(23));
+ cow_fd = std::make_unique<CowWriterFileDescriptor>(std::move(cow_writer));
+
+ ASSERT_EQ((ssize_t)BLOCK_SIZE * 4, cow_fd->Seek(BLOCK_SIZE * 4, SEEK_SET));
+ ASSERT_EQ((ssize_t)read_back.size(),
+ cow_fd->Read(read_back.data(), read_back.size()));
+ ASSERT_EQ(verity_data, read_back)
+ << "Could not read verity data afeter InitializeAppend() => Read() => "
+ "InitializeAppend() sequence. If no writes happened while CowWriterFd "
+ "is open, Finalize() should not be called.";
+}
+
+} // namespace chromeos_update_engine
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index d2a015d..586662d 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -488,7 +488,6 @@
}
TEST_F(FilesystemVerifierActionTest, ReadAfterWrite) {
- constexpr auto BLOCK_SIZE = 4096;
ScopedTempFile cow_device_file("cow_device.XXXXXX", true);
android::snapshot::CompressedSnapshotWriter snapshot_writer{
{.block_size = BLOCK_SIZE}};
@@ -507,6 +506,12 @@
ASSERT_TRUE(snapshot_writer.Finalize());
cow_reader = snapshot_writer.OpenReader();
ASSERT_NE(cow_reader, nullptr);
+ std::vector<unsigned char> read_back;
+ read_back.resize(buffer.size());
+ cow_reader->Seek(BLOCK_SIZE, SEEK_SET);
+ const auto bytes_read = cow_reader->Read(read_back.data(), read_back.size());
+ ASSERT_EQ((size_t)(bytes_read), BLOCK_SIZE);
+ ASSERT_EQ(read_back, buffer);
}
} // namespace chromeos_update_engine