Support verity writes in VABC am: 4f28a6c152

Original change: https://android-review.googlesource.com/c/platform/system/update_engine/+/1612456

Change-Id: I88da9dcdafcbf6e001d59a2628cd31279b83aeaf

7 files changed, 49 insertions, 46 deletions

diff --git a/download_action_android_unittest.cc b/download_action_android_unittest.cc
index b17550bf..fef2d24d 100644
--- a/download_action_android_unittest.cc
+++ b/download_action_android_unittest.cc
@@ -36,6 +36,7 @@
 #include "update_engine/common/utils.h"
 #include "update_engine/payload_consumer/install_plan.h"
 #include "update_engine/payload_consumer/payload_constants.h"
+#include "update_engine/payload_generator/annotated_operation.h"
 #include "update_engine/payload_generator/payload_file.h"
 #include "update_engine/payload_generator/payload_signer.h"
 
@@ -149,13 +150,16 @@ TEST_F(DownloadActionTest, CacheManifestValid) {
   payload.size = data.size();
   payload.payload_urls.emplace_back("http://fake_url.invalid");
   install_plan.is_resume = true;
+  auto& install_part = install_plan.partitions.emplace_back();
+  install_part.source_path = partition_file.path();
+  install_part.target_path = partition_file.path();
   action_pipe->set_contents(install_plan);
 
+  FakeHardware hardware;
   // takes ownership of passed in HttpFetcher
   auto download_action = std::make_unique<DownloadAction>(
-      &prefs, &boot_control, nullptr, http_fetcher, false /* interactive */);
+      &prefs, &boot_control, &hardware, http_fetcher, false /* interactive */);
 
-  FakeHardware hardware;
   auto delta_performer = std::make_unique<DeltaPerformer>(&prefs,
                                                           &boot_control,
                                                           &hardware,
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 82e589ec..a57169b5 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -584,6 +584,11 @@ bool DeltaPerformer::Write(const void* bytes, size_t count, ErrorCode* error) {
     CheckpointUpdateProgress(false);
   }
 
+  if (partition_writer_) {
+    TEST_AND_RETURN_FALSE(partition_writer_->FinishedInstallOps());
+  }
+  CloseCurrentPartition();
+
   // In major version 2, we don't add unused operation to the payload.
   // If we already extracted the signature we should skip this step.
   if (manifest_.has_signatures_offset() && manifest_.has_signatures_size() &&
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index 4efbe41b..09dc638b 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -32,8 +32,10 @@
 #include <base/strings/string_util.h>
 #include <brillo/data_encoding.h>
 #include <brillo/message_loops/message_loop.h>
+#include <brillo/secure_blob.h>
 #include <brillo/streams/file_stream.h>
 
+#include "payload_generator/delta_diff_generator.h"
 #include "update_engine/common/utils.h"
 #include "update_engine/payload_consumer/file_descriptor.h"
 
@@ -106,8 +108,7 @@ void FilesystemVerifierAction::TerminateProcessing() {
 }
 
 void FilesystemVerifierAction::Cleanup(ErrorCode code) {
-  read_fd_.reset();
-  write_fd_.reset();
+  partition_fd_.reset();
   // This memory is not used anymore.
   buffer_.clear();
 
@@ -129,36 +130,30 @@ bool FilesystemVerifierAction::InitializeFdVABC() {
   const InstallPlan::Partition& partition =
       install_plan_.partitions[partition_index_];
 
-  read_fd_ =
+  // FilesystemVerifierAction need the read_fd_.
+  partition_fd_ =
       dynamic_control_->OpenCowFd(partition.name, partition.source_path, true);
-  if (!read_fd_) {
+  if (!partition_fd_) {
     LOG(ERROR) << "OpenCowReader(" << partition.name << ", "
                << partition.source_path << ") failed.";
     return false;
   }
   partition_size_ = partition.target_size;
-  // TODO(b/173432386): Support Verity writes for VABC.
-  CHECK_EQ(partition.fec_size, 0U);
-  CHECK_EQ(partition.hash_tree_size, 0U);
   return true;
 }
 
 bool FilesystemVerifierAction::InitializeFd(const std::string& part_path) {
-  read_fd_ = FileDescriptorPtr(new EintrSafeFileDescriptor());
-  if (!read_fd_->Open(part_path.c_str(), O_RDONLY)) {
+  partition_fd_ = FileDescriptorPtr(new EintrSafeFileDescriptor());
+  const bool write_verity = ShouldWriteVerity();
+  int flags = write_verity ? O_RDWR : O_RDONLY;
+  if (!utils::SetBlockDeviceReadOnly(part_path, !write_verity)) {
+    LOG(WARNING) << "Failed to set block device " << part_path << " as "
+                 << (write_verity ? "writable" : "readonly");
+  }
+  if (!partition_fd_->Open(part_path.c_str(), flags)) {
     LOG(ERROR) << "Unable to open " << part_path << " for reading.";
     return false;
   }
-
-  // Can't re-use |read_fd_|, as verity writer may call `seek` to modify state
-  // of a file descriptor.
-  if (ShouldWriteVerity()) {
-    write_fd_ = FileDescriptorPtr(new EintrSafeFileDescriptor());
-    if (!write_fd_->Open(part_path.c_str(), O_RDWR)) {
-      LOG(ERROR) << "Unable to open " << part_path << " for Read/Write.";
-      return false;
-    }
-  }
   return true;
 }
 
@@ -238,9 +233,12 @@ void FilesystemVerifierAction::StartPartitionHashing() {
   }
   if (ShouldWriteVerity()) {
     if (!verity_writer_->Init(partition)) {
+      LOG(INFO) << "Verity writes enabled on partition " << partition.name;
       Cleanup(ErrorCode::kVerityCalculationError);
       return;
     }
+  } else {
+    LOG(INFO) << "Verity writes disabled on partition " << partition.name;
   }
 
   // Start the first read.
@@ -257,19 +255,19 @@ bool FilesystemVerifierAction::ShouldWriteVerity() {
 
 void FilesystemVerifierAction::ReadVerityAndFooter() {
   if (ShouldWriteVerity()) {
-    if (!verity_writer_->Finalize(read_fd_, write_fd_)) {
+    if (!verity_writer_->Finalize(partition_fd_, partition_fd_)) {
       LOG(ERROR) << "Failed to write hashtree/FEC data.";
       Cleanup(ErrorCode::kFilesystemVerifierError);
       return;
     }
   }
+  // Since we handed our |read_fd_| to verity_writer_ during |Finalize()|
+  // call, fd's position could have been changed. Re-seek.
+  partition_fd_->Seek(filesystem_data_end_, SEEK_SET);
   auto bytes_to_read = partition_size_ - filesystem_data_end_;
-  // Since we handed our |read_fd_| to verity_writer_ during |Finalize()| call,
-  // fd's position could have been changed. Re-seek.
-  read_fd_->Seek(filesystem_data_end_, SEEK_SET);
   while (bytes_to_read > 0) {
     const auto read_size = std::min<size_t>(buffer_.size(), bytes_to_read);
-    auto bytes_read = read_fd_->Read(buffer_.data(), read_size);
+    auto bytes_read = partition_fd_->Read(buffer_.data(), read_size);
     if (bytes_read <= 0) {
       PLOG(ERROR) << "Failed to read hash tree " << bytes_read;
       Cleanup(ErrorCode::kFilesystemVerifierError);
@@ -296,10 +294,11 @@ void FilesystemVerifierAction::ScheduleFileSystemRead() {
     ReadVerityAndFooter();
     return;
   }
-
-  auto bytes_read = read_fd_->Read(buffer_.data(), bytes_to_read);
+  partition_fd_->Seek(offset_, SEEK_SET);
+  auto bytes_read = partition_fd_->Read(buffer_.data(), bytes_to_read);
   if (bytes_read < 0) {
-    LOG(ERROR) << "Unable to schedule an asynchronous read from the stream.";
+    LOG(ERROR) << "Unable to schedule an asynchronous read from the stream. "
+               << bytes_read;
     Cleanup(ErrorCode::kError);
   } else {
     // We could just invoke |OnReadDoneCallback()|, it works. But |PostTask|
@@ -424,11 +423,8 @@ void FilesystemVerifierAction::FinishPartitionHashing() {
   // Start hashing the next partition, if any.
   hasher_.reset();
   buffer_.clear();
-  if (read_fd_) {
-    read_fd_.reset();
-  }
-  if (write_fd_) {
-    write_fd_.reset();
+  if (partition_fd_) {
+    partition_fd_.reset();
   }
   StartPartitionHashing();
 }
diff --git a/payload_consumer/filesystem_verifier_action.h b/payload_consumer/filesystem_verifier_action.h
index 1f527c92..78634cb3 100644
--- a/payload_consumer/filesystem_verifier_action.h
+++ b/payload_consumer/filesystem_verifier_action.h
@@ -126,14 +126,8 @@ class FilesystemVerifierAction : public InstallPlanAction {
   size_t partition_index_{0};
 
   // If not null, the FileDescriptor used to read from the device.
-  // |read_fd_| and |write_fd_| will be initialized when we begin hashing a
-  // partition. They will be deallocated once we encounter an error or
-  // successfully finished hashing.
-  FileDescriptorPtr read_fd_;
-  // If not null, the FileDescriptor used to write to the device.
-  // For VABC, this will be different from |read_fd_|. For other cases
-  // this can be the same as |read_fd_|.
-  FileDescriptorPtr write_fd_;
+  // verity writer might attempt to write to this fd, if verity is enabled.
+  FileDescriptorPtr partition_fd_;
 
   // Buffer for storing data we read.
   brillo::Blob buffer_;
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index 60e36cc2..2cad5232 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -84,7 +84,8 @@ class FilesystemVerifierActionTestDelegate : public ActionProcessorDelegate {
     if (action->Type() == FilesystemVerifierAction::StaticType()) {
       ran_ = true;
       code_ = code;
-      EXPECT_FALSE(static_cast<FilesystemVerifierAction*>(action)->read_fd_);
+      EXPECT_FALSE(
+          static_cast<FilesystemVerifierAction*>(action)->partition_fd_);
     } else if (action->Type() ==
                ObjectCollectorAction<InstallPlan>::StaticType()) {
       auto collector_action =
diff --git a/payload_consumer/verity_writer_android.cc b/payload_consumer/verity_writer_android.cc
index 4b23b831..e2fab7dd 100644
--- a/payload_consumer/verity_writer_android.cc
+++ b/payload_consumer/verity_writer_android.cc
@@ -43,9 +43,6 @@ std::unique_ptr<VerityWriterInterface> CreateVerityWriter() {
 bool VerityWriterAndroid::Init(const InstallPlan::Partition& partition) {
   partition_ = &partition;
 
-  if (partition_->hash_tree_size != 0 || partition_->fec_size != 0) {
-    utils::SetBlockDeviceReadOnly(partition_->target_path, false);
-  }
   if (partition_->hash_tree_size != 0) {
     auto hash_function =
         HashTreeBuilder::HashFunction(partition_->hash_tree_algorithm);
diff --git a/payload_generator/delta_diff_generator.cc b/payload_generator/delta_diff_generator.cc
index a87dabf1..ab0f0364 100644
--- a/payload_generator/delta_diff_generator.cc
+++ b/payload_generator/delta_diff_generator.cc
@@ -137,6 +137,12 @@ class PartitionProcessor : public base::DelegateSimpleThread::Delegate {
         {cow_merge_sequence_->begin(), cow_merge_sequence_->end()},
         config_.block_size,
         config_.target.dynamic_partition_metadata->vabc_compression_param());
+    if (!new_part_.disable_fec_computation) {
+      *cow_size_ +=
+          new_part_.verity.fec_extent.num_blocks() * config_.block_size;
+    }
+    *cow_size_ +=
+        new_part_.verity.hash_tree_extent.num_blocks() * config_.block_size;
     LOG(INFO) << "Estimated COW size for partition: " << new_part_.name << " "
               << *cow_size_;
   }