Do not skip copying in place block if source corrupted am: 7dd5a5e840 am: 0c3d282535
Original change: https://android-review.googlesource.com/c/platform/system/update_engine/+/2585937
Change-Id: Ic4a8b998838e8585e431792950e960419804de59
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/common/hash_calculator.h b/common/hash_calculator.h
index dd7b2e8..36bfcc8 100644
--- a/common/hash_calculator.h
+++ b/common/hash_calculator.h
@@ -90,7 +90,7 @@
bool valid_;
// The hash state used by OpenSSL
- SHA256_CTX ctx_;
+ SHA256_CTX ctx_{};
DISALLOW_COPY_AND_ASSIGN(HashCalculator);
};
diff --git a/payload_consumer/fec_file_descriptor.cc b/payload_consumer/fec_file_descriptor.cc
index 3fee196..56edc24 100644
--- a/payload_consumer/fec_file_descriptor.cc
+++ b/payload_consumer/fec_file_descriptor.cc
@@ -34,7 +34,7 @@
return false;
}
- fec_status status;
+ fec_status status{};
if (!fh_.get_status(status)) {
LOG(ERROR) << "Couldn't load ECC status";
fh_.close();
diff --git a/payload_consumer/partition_writer.cc b/payload_consumer/partition_writer.cc
index d7d8bea..2ec05f5 100644
--- a/payload_consumer/partition_writer.cc
+++ b/payload_consumer/partition_writer.cc
@@ -32,18 +32,14 @@
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
-#include "update_engine/common/terminator.h"
+#include "update_engine/common/error_code.h"
#include "update_engine/common/utils.h"
-#include "update_engine/payload_consumer/bzip_extent_writer.h"
#include "update_engine/payload_consumer/cached_file_descriptor.h"
-#include "update_engine/payload_consumer/extent_reader.h"
#include "update_engine/payload_consumer/extent_writer.h"
#include "update_engine/payload_consumer/file_descriptor_utils.h"
#include "update_engine/payload_consumer/install_operation_executor.h"
#include "update_engine/payload_consumer/install_plan.h"
#include "update_engine/payload_consumer/mount_history.h"
-#include "update_engine/payload_consumer/payload_constants.h"
-#include "update_engine/payload_consumer/xz_extent_writer.h"
#include "update_engine/payload_generator/extent_utils.h"
namespace chromeos_update_engine {
@@ -232,22 +228,23 @@
// decide it the operation should be skipped.
const PartitionUpdate& partition = partition_update_;
- InstallOperation buf;
- const bool should_optimize = dynamic_control_->OptimizeOperation(
- partition.partition_name(), operation, &buf);
- const InstallOperation& optimized = should_optimize ? buf : operation;
-
// Invoke ChooseSourceFD with original operation, so that it can properly
// verify source hashes. Optimized operation might contain a smaller set of
// extents, or completely empty.
auto source_fd = ChooseSourceFD(operation, error);
- if (source_fd == nullptr) {
- LOG(ERROR) << "Unrecoverable source hash mismatch found on partition "
- << partition.partition_name()
- << " extents: " << ExtentsToString(operation.src_extents());
+ if (*error != ErrorCode::kSuccess || source_fd == nullptr) {
+ LOG(WARNING) << "Source hash mismatch detected for extents "
+ << operation.src_extents() << " on partition "
+ << partition.partition_name() << " @ " << source_path_;
+
return false;
}
+ InstallOperation buf;
+ const bool should_optimize = dynamic_control_->OptimizeOperation(
+ partition.partition_name(), operation, &buf);
+ const InstallOperation& optimized = should_optimize ? buf : operation;
+
auto writer = CreateBaseExtentWriter();
return install_op_executor_.ExecuteSourceCopyOperation(
optimized, std::move(writer), source_fd);
@@ -340,8 +337,9 @@
// Log remount history if this device is an ext4 partition.
LogMountHistory(source_fd);
-
- *error = ErrorCode::kDownloadStateInitializationError;
+ if (error) {
+ *error = ErrorCode::kDownloadStateInitializationError;
+ }
return false;
}
return true;
diff --git a/payload_consumer/partition_writer_unittest.cc b/payload_consumer/partition_writer_unittest.cc
index 4910594..32324b6 100644
--- a/payload_consumer/partition_writer_unittest.cc
+++ b/payload_consumer/partition_writer_unittest.cc
@@ -210,8 +210,7 @@
op.set_src_sha256_hash(src_hash.data(), src_hash.size());
ErrorCode error = ErrorCode::kSuccess;
- ASSERT_EQ(writer_.verified_source_fd_.source_ecc_fd_,
- writer_.ChooseSourceFD(op, &error));
+ ASSERT_NE(writer_.ChooseSourceFD(op, &error), nullptr);
ASSERT_EQ(ErrorCode::kSuccess, error);
// Verify that the fake_fec was actually used.
ASSERT_EQ(1U, fake_fec->GetReadOps().size());
diff --git a/payload_consumer/vabc_partition_writer.cc b/payload_consumer/vabc_partition_writer.cc
index 17b7d50..b00ff70 100644
--- a/payload_consumer/vabc_partition_writer.cc
+++ b/payload_consumer/vabc_partition_writer.cc
@@ -95,7 +95,7 @@
}
copy_blocks_.AddExtent(cow_op.dst_extent());
}
- LOG(INFO) << "Partition `" << partition_update.partition_name() << " has "
+ LOG(INFO) << "Partition `" << partition_update.partition_name() << "` has "
<< copy_blocks_.blocks() << " copy blocks";
}
diff --git a/payload_consumer/verified_source_fd.cc b/payload_consumer/verified_source_fd.cc
index 002bd07..f35b6a9 100644
--- a/payload_consumer/verified_source_fd.cc
+++ b/payload_consumer/verified_source_fd.cc
@@ -26,11 +26,17 @@
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
+#include "update_engine/common/error_code.h"
+#include "update_engine/common/hash_calculator.h"
#include "update_engine/common/utils.h"
-#include "update_engine/payload_consumer/fec_file_descriptor.h"
+#include "update_engine/payload_consumer/extent_writer.h"
+#include "update_engine/payload_consumer/file_descriptor.h"
#include "update_engine/payload_consumer/file_descriptor_utils.h"
-#include "update_engine/payload_consumer/mount_history.h"
#include "update_engine/payload_consumer/partition_writer.h"
+#include "update_engine/update_metadata.pb.h"
+#if USE_FEC
+#include "update_engine/payload_consumer/fec_file_descriptor.h"
+#endif
namespace chromeos_update_engine {
using std::string;
@@ -45,7 +51,7 @@
return false;
#if USE_FEC
- FileDescriptorPtr fd(new FecFileDescriptor());
+ auto fd = std::make_shared<FecFileDescriptor>();
if (!fd->Open(source_path_.c_str(), O_RDONLY, 0)) {
PLOG(ERROR) << "Unable to open ECC source partition " << source_path_;
source_ecc_open_failure_ = true;
@@ -60,12 +66,25 @@
return !source_ecc_open_failure_;
}
+bool VerifiedSourceFd::WriteBackCorrectedSourceBlocks(
+ const std::vector<unsigned char>& source_data,
+ const google::protobuf::RepeatedPtrField<Extent>& extents) {
+ auto fd = std::make_shared<EintrSafeFileDescriptor>();
+ TEST_AND_RETURN_FALSE_ERRNO(fd->Open(source_path_.c_str(), O_RDWR));
+ DirectExtentWriter writer(fd);
+ TEST_AND_RETURN_FALSE(writer.Init(extents, block_size_));
+ return writer.Write(source_data.data(), source_data.size());
+}
+
FileDescriptorPtr VerifiedSourceFd::ChooseSourceFD(
const InstallOperation& operation, ErrorCode* error) {
if (source_fd_ == nullptr) {
LOG(ERROR) << "ChooseSourceFD fail: source_fd_ == nullptr";
return nullptr;
}
+ if (error) {
+ *error = ErrorCode::kSuccess;
+ }
if (!operation.has_src_sha256_hash()) {
// When the operation doesn't include a source hash, we attempt the error
// corrected device first since we can't verify the block in the raw device
@@ -74,6 +93,9 @@
if (OpenCurrentECCPartition() &&
fd_utils::ReadAndHashExtents(
source_ecc_fd_, operation.src_extents(), block_size_, nullptr)) {
+ if (error) {
+ *error = ErrorCode::kDownloadOperationHashMissingError;
+ }
return source_ecc_fd_;
}
return source_fd_;
@@ -87,6 +109,9 @@
source_hash == expected_source_hash) {
return source_fd_;
}
+ if (error) {
+ *error = ErrorCode::kDownloadOperationHashMismatch;
+ }
// We fall back to use the error corrected device if the hash of the raw
// device doesn't match or there was an error reading the source partition.
if (!OpenCurrentECCPartition()) {
@@ -103,11 +128,23 @@
<< base::HexEncode(expected_source_hash.data(),
expected_source_hash.size());
- if (fd_utils::ReadAndHashExtents(
- source_ecc_fd_, operation.src_extents(), block_size_, &source_hash) &&
- PartitionWriter::ValidateSourceHash(
+ std::vector<unsigned char> source_data;
+ if (!utils::ReadExtents(
+ source_ecc_fd_, operation.src_extents(), &source_data, block_size_)) {
+ return nullptr;
+ }
+ if (!HashCalculator::RawHashOfData(source_data, &source_hash)) {
+ return nullptr;
+ }
+ if (PartitionWriter::ValidateSourceHash(
source_hash, operation, source_ecc_fd_, error)) {
source_ecc_recovered_failures_++;
+ if (WriteBackCorrectedSourceBlocks(source_data, operation.src_extents())) {
+ if (error) {
+ *error = ErrorCode::kSuccess;
+ }
+ return source_fd_;
+ }
return source_ecc_fd_;
}
return nullptr;
diff --git a/payload_consumer/verified_source_fd.h b/payload_consumer/verified_source_fd.h
index f7d0620..6d859b9 100644
--- a/payload_consumer/verified_source_fd.h
+++ b/payload_consumer/verified_source_fd.h
@@ -39,6 +39,9 @@
[[nodiscard]] bool Open();
private:
+ bool WriteBackCorrectedSourceBlocks(
+ const std::vector<unsigned char>& source_data,
+ const google::protobuf::RepeatedPtrField<Extent>& extents);
bool OpenCurrentECCPartition();
const size_t block_size_;
const std::string source_path_;