summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--common/hash_calculator.cc5
-rw-r--r--common/hash_calculator.h1
-rw-r--r--common/utils.h8
-rw-r--r--payload_consumer/cow_writer_file_descriptor.cc17
-rw-r--r--payload_consumer/filesystem_verifier_action.cc23
-rw-r--r--payload_consumer/filesystem_verifier_action_unittest.cc72
6 files changed, 99 insertions, 27 deletions
diff --git a/common/hash_calculator.cc b/common/hash_calculator.cc
index d010a532..60812d56 100644
--- a/common/hash_calculator.cc
+++ b/common/hash_calculator.cc
@@ -95,6 +95,11 @@ bool HashCalculator::RawHashOfData(const brillo::Blob& data,
return RawHashOfBytes(data.data(), data.size(), out_hash);
}
+bool HashCalculator::RawHashOfFile(const string& name, brillo::Blob* out_hash) {
+ const auto file_size = utils::FileSize(name);
+ return RawHashOfFile(name, file_size, out_hash) == file_size;
+}
+
off_t HashCalculator::RawHashOfFile(const string& name,
off_t length,
brillo::Blob* out_hash) {
diff --git a/common/hash_calculator.h b/common/hash_calculator.h
index b7e4d86c..44261284 100644
--- a/common/hash_calculator.h
+++ b/common/hash_calculator.h
@@ -75,6 +75,7 @@ class HashCalculator {
static off_t RawHashOfFile(const std::string& name,
off_t length,
brillo::Blob* out_hash);
+ static bool RawHashOfFile(const std::string& name, brillo::Blob* out_hash);
private:
// If non-empty, the final raw hash. Will only be set to non-empty when
diff --git a/common/utils.h b/common/utils.h
index 5f6e4757..59f236ef 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -399,13 +399,19 @@ class ScopedTempFile {
// If |open_fd| is true, a writable file descriptor will be opened for this
// file.
- explicit ScopedTempFile(const std::string& pattern, bool open_fd = false) {
+ // If |truncate_size| is non-zero, truncate file to that size on creation.
+ explicit ScopedTempFile(const std::string& pattern,
+ bool open_fd = false,
+ size_t truncate_size = 0) {
CHECK(utils::MakeTempFile(pattern, &path_, open_fd ? &fd_ : nullptr));
unlinker_.reset(new ScopedPathUnlinker(path_));
if (open_fd) {
CHECK_GE(fd_, 0);
fd_closer_.reset(new ScopedFdCloser(&fd_));
}
+ if (truncate_size > 0) {
+ CHECK_EQ(0, truncate(path_.c_str(), truncate_size));
+ }
}
virtual ~ScopedTempFile() = default;
diff --git a/payload_consumer/cow_writer_file_descriptor.cc b/payload_consumer/cow_writer_file_descriptor.cc
index d8c7afb8..2de6664a 100644
--- a/payload_consumer/cow_writer_file_descriptor.cc
+++ b/payload_consumer/cow_writer_file_descriptor.cc
@@ -28,7 +28,10 @@ namespace chromeos_update_engine {
CowWriterFileDescriptor::CowWriterFileDescriptor(
std::unique_ptr<android::snapshot::ISnapshotWriter> cow_writer)
: cow_writer_(std::move(cow_writer)),
- cow_reader_(cow_writer_->OpenReader()) {}
+ cow_reader_(cow_writer_->OpenReader()) {
+ CHECK_NE(cow_writer_, nullptr);
+ CHECK_NE(cow_reader_, nullptr);
+}
bool CowWriterFileDescriptor::Open(const char* path, int flags, mode_t mode) {
LOG(ERROR) << "CowWriterFileDescriptor doesn't support Open()";
@@ -113,7 +116,17 @@ bool CowWriterFileDescriptor::Flush() {
bool CowWriterFileDescriptor::Close() {
if (cow_writer_) {
- TEST_AND_RETURN_FALSE(cow_writer_->Finalize());
+ // b/186196758
+ // 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. However,
+ // when calling SnapshotWriter::Finalize(), data after resume label are
+ // discarded, therefore verity data is gone. To prevent phantom reads, don't
+ // call Finalize() unless we actually write something.
+ if (dirty_) {
+ TEST_AND_RETURN_FALSE(cow_writer_->Finalize());
+ }
cow_writer_ = nullptr;
}
if (cow_reader_) {
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index 09dc638b..b14cbc8d 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -112,6 +112,14 @@ void FilesystemVerifierAction::Cleanup(ErrorCode code) {
// This memory is not used anymore.
buffer_.clear();
+ // If we didn't write verity, partitions were maped. Releaase resource now.
+ if (!install_plan_.write_verity &&
+ dynamic_control_->UpdateUsesSnapshotCompression()) {
+ LOG(INFO) << "Not writing verity and VABC is enabled, unmapping all "
+ "partitions";
+ dynamic_control_->UnmapAllPartitions();
+ }
+
if (cancelled_)
return;
if (code == ErrorCode::kSuccess && HasOutputPipe())
@@ -130,6 +138,21 @@ bool FilesystemVerifierAction::InitializeFdVABC() {
const InstallPlan::Partition& partition =
install_plan_.partitions[partition_index_];
+ if (!ShouldWriteVerity()) {
+ // In VABC, if we are not writing verity, just map all partitions,
+ // and read using regular fd on |postinstall_mount_device| .
+ // All read will go through snapuserd, which provides a consistent
+ // view: device will use snapuserd to read partition during boot.
+ // b/186196758
+ // Call UnmapAllPartitions() first, because if we wrote verity before, these
+ // writes won't be visible to previously opened snapuserd daemon. To ensure
+ // that we will see the most up to date data from partitions, call Unmap()
+ // then Map() to re-spin daemon.
+ dynamic_control_->UnmapAllPartitions();
+ dynamic_control_->MapAllPartitions();
+ return InitializeFd(partition.readonly_target_path);
+ }
+
// FilesystemVerifierAction need the read_fd_.
partition_fd_ =
dynamic_control_->OpenCowFd(partition.name, partition.source_path, true);
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index c1006842..d2a015d8 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -48,8 +48,23 @@ using testing::SetArgPointee;
namespace chromeos_update_engine {
class FilesystemVerifierActionTest : public ::testing::Test {
+ public:
+ static constexpr size_t BLOCK_SIZE = 4096;
+ static constexpr size_t PARTITION_SIZE = BLOCK_SIZE * 1024;
+
protected:
- void SetUp() override { loop_.SetAsCurrent(); }
+ void SetUp() override {
+ brillo::Blob part_data(PARTITION_SIZE);
+ test_utils::FillWithData(&part_data);
+ ASSERT_TRUE(utils::WriteFile(
+ source_part.path().c_str(), part_data.data(), part_data.size()));
+ // FillWithData() will will with different data next call. We want
+ // source/target partitions to contain different data for testing.
+ test_utils::FillWithData(&part_data);
+ ASSERT_TRUE(utils::WriteFile(
+ target_part.path().c_str(), part_data.data(), part_data.size()));
+ loop_.SetAsCurrent();
+ }
void TearDown() override {
EXPECT_EQ(0, brillo::MessageLoopRunMaxIterations(&loop_, 1));
@@ -62,11 +77,34 @@ class FilesystemVerifierActionTest : public ::testing::Test {
void BuildActions(const InstallPlan& install_plan,
DynamicPartitionControlInterface* dynamic_control);
+ InstallPlan::Partition* AddFakePartition(InstallPlan* install_plan,
+ std::string name = "fake_part") {
+ InstallPlan::Partition& part = install_plan->partitions.emplace_back();
+ part.name = name;
+ part.target_path = target_part.path();
+ part.readonly_target_path = part.target_path;
+ part.target_size = PARTITION_SIZE;
+ part.block_size = BLOCK_SIZE;
+ part.source_path = source_part.path();
+ EXPECT_TRUE(
+ HashCalculator::RawHashOfFile(source_part.path(), &part.source_hash));
+ EXPECT_TRUE(
+ HashCalculator::RawHashOfFile(target_part.path(), &part.target_hash));
+ return &part;
+ }
+
brillo::FakeMessageLoop loop_{nullptr};
ActionProcessor processor_;
DynamicPartitionControlStub dynamic_control_stub_;
+ static ScopedTempFile source_part;
+ static ScopedTempFile target_part;
};
+ScopedTempFile FilesystemVerifierActionTest::source_part{
+ "source_part.XXXXXX", false, PARTITION_SIZE};
+ScopedTempFile FilesystemVerifierActionTest::target_part{
+ "target_part.XXXXXX", false, PARTITION_SIZE};
+
class FilesystemVerifierActionTestDelegate : public ActionProcessorDelegate {
public:
FilesystemVerifierActionTestDelegate()
@@ -406,32 +444,27 @@ TEST_F(FilesystemVerifierActionTest, RunAsRootSkipWriteVerityTest) {
TEST_F(FilesystemVerifierActionTest, RunWithVABCNoVerity) {
InstallPlan install_plan;
- InstallPlan::Partition& part = install_plan.partitions.emplace_back();
- part.name = "fake_part";
- part.target_path = "/dev/fake_target_path";
- part.target_size = 4096 * 4096;
- part.block_size = 4096;
- part.source_path = "/dev/fake_source_path";
- part.fec_size = 0;
- part.hash_tree_size = 0;
- part.target_hash.clear();
- part.source_hash.clear();
+ auto part_ptr = AddFakePartition(&install_plan);
+ ASSERT_NE(part_ptr, nullptr);
+ InstallPlan::Partition& part = *part_ptr;
+ part.target_path = "Shouldn't attempt to open this path";
NiceMock<MockDynamicPartitionControl> dynamic_control;
- auto fake_fd = std::make_shared<FakeFileDescriptor>();
ON_CALL(dynamic_control, GetDynamicPartitionsFeatureFlag())
.WillByDefault(Return(FeatureFlag(FeatureFlag::Value::LAUNCH)));
ON_CALL(dynamic_control, UpdateUsesSnapshotCompression())
.WillByDefault(Return(true));
- ON_CALL(dynamic_control, OpenCowFd(_, _, _)).WillByDefault(Return(fake_fd));
ON_CALL(dynamic_control, IsDynamicPartition(part.name, _))
.WillByDefault(Return(true));
EXPECT_CALL(dynamic_control, UpdateUsesSnapshotCompression())
.Times(AtLeast(1));
+ // Since we are not writing verity, we should not attempt to OpenCowFd()
+ // reads should go through regular file descriptors on mapped partitions.
EXPECT_CALL(dynamic_control, OpenCowFd(part.name, {part.source_path}, _))
- .Times(1);
+ .Times(0);
+ EXPECT_CALL(dynamic_control, MapAllPartitions()).Times(AtLeast(1));
EXPECT_CALL(dynamic_control, ListDynamicPartitionsForSlot(_, _, _))
.WillRepeatedly(
DoAll(SetArgPointee<2, std::vector<std::string>>({part.name}),
@@ -451,16 +484,7 @@ TEST_F(FilesystemVerifierActionTest, RunWithVABCNoVerity) {
ASSERT_FALSE(processor_.IsRunning());
ASSERT_TRUE(delegate.ran());
- // Filesystem verifier will fail, because we set an empty hash
- ASSERT_EQ(ErrorCode::kNewRootfsVerificationError, delegate.code());
- const auto& read_pos = fake_fd->GetReadOps();
- size_t expected_offset = 0;
- for (const auto& [off, size] : read_pos) {
- ASSERT_EQ(off, expected_offset);
- expected_offset += size;
- }
- const auto actual_read_size = expected_offset;
- ASSERT_EQ(actual_read_size, part.target_size);
+ ASSERT_EQ(ErrorCode::kSuccess, delegate.code());
}
TEST_F(FilesystemVerifierActionTest, ReadAfterWrite) {
generated by cgit v1.2.3 (git 2.25.1) at 2025-10-03 18:58:31 +0000