Recover source hash failures on SOURCE_COPY operations. am: 51c264e33e
am: 4630ec1601
Change-Id: I39abd31949bb64582dc2e61bc41857d22c4881f0
diff --git a/Android.mk b/Android.mk
index 529c1f4..07bf16a 100644
--- a/Android.mk
+++ b/Android.mk
@@ -22,6 +22,7 @@
# by setting BRILLO_USE_* values. Note that we define local variables like
# local_use_* to prevent leaking our default setting for other packages.
local_use_binder := $(if $(BRILLO_USE_BINDER),$(BRILLO_USE_BINDER),1)
+local_use_fec := 1
local_use_hwid_override := \
$(if $(BRILLO_USE_HWID_OVERRIDE),$(BRILLO_USE_HWID_OVERRIDE),0)
local_use_mtd := $(if $(BRILLO_USE_MTD),$(BRILLO_USE_MTD),0)
@@ -35,6 +36,7 @@
-DUSE_BINDER=$(local_use_binder) \
-DUSE_CHROME_NETWORK_PROXY=$(local_use_chrome_network_proxy) \
-DUSE_CHROME_KIOSK_APP=$(local_use_chrome_kiosk_app) \
+ -DUSE_FEC=$(local_use_fec) \
-DUSE_HWID_OVERRIDE=$(local_use_hwid_override) \
-DUSE_MTD=$(local_use_mtd) \
-DUSE_OMAHA=$(local_use_omaha) \
@@ -148,6 +150,13 @@
payload_consumer/postinstall_runner_action.cc \
payload_consumer/xz_extent_writer.cc
+ifeq ($(local_use_fec),1)
+ue_libpayload_consumer_src_files += \
+ payload_consumer/fec_file_descriptor.cc
+ue_libpayload_consumer_exported_shared_libraries += \
+ libfec
+endif # local_use_fec == 1
+
ifeq ($(HOST_OS),linux)
# Build for the host.
include $(CLEAR_VARS)
@@ -510,6 +519,21 @@
libmodpb64 \
libgtest_prod
+ifeq ($(local_use_fec),1)
+# The static library "libfec" depends on a bunch of other static libraries, but
+# such dependency is not handled by the build system, so we need to add them
+# here.
+LOCAL_STATIC_LIBRARIES += \
+ libext4_utils \
+ libsquashfs_utils \
+ libcutils \
+ libcrypto_utils \
+ libcrypto \
+ libcutils \
+ libbase \
+ libfec_rs
+endif # local_use_fec == 1
+
ifeq ($(strip $(PRODUCT_STATIC_BOOT_CONTROL_HAL)),)
# No static boot_control HAL defined, so no sideload support. We use a fake
# boot_control HAL to allow compiling update_engine_sideload for test purposes.
@@ -737,6 +761,11 @@
$(ue_libpayload_consumer_exported_shared_libraries) \
$(ue_libpayload_generator_exported_shared_libraries)
LOCAL_SRC_FILES := $(ue_delta_generator_src_files)
+# libfec, when built for the host on linux uses integer sanitize, which requires
+# any module that links against it to also include this option when linking,
+# otherwise you will have a missing symbol error at runtime:
+# undefined symbol: __ubsan_handle_add_overflow_abort
+LOCAL_SANITIZE := integer
include $(BUILD_HOST_EXECUTABLE)
endif # HOST_OS == linux
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index a80092b..042861f 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -48,11 +48,14 @@
#include "update_engine/payload_consumer/download_action.h"
#include "update_engine/payload_consumer/extent_reader.h"
#include "update_engine/payload_consumer/extent_writer.h"
+#if USE_FEC
+#include "update_engine/payload_consumer/fec_file_descriptor.h"
+#endif // USE_FEC
#include "update_engine/payload_consumer/file_descriptor_utils.h"
#include "update_engine/payload_consumer/mount_history.h"
#if USE_MTD
#include "update_engine/payload_consumer/mtd_file_descriptor.h"
-#endif
+#endif // USE_MTD
#include "update_engine/payload_consumer/payload_constants.h"
#include "update_engine/payload_consumer/payload_verifier.h"
#include "update_engine/payload_consumer/xz_extent_writer.h"
@@ -327,6 +330,14 @@
err = 1;
}
source_fd_.reset();
+ if (source_ecc_fd_ && !source_ecc_fd_->Close()) {
+ err = errno;
+ PLOG(ERROR) << "Error closing ECC source partition";
+ if (!err)
+ err = 1;
+ }
+ source_ecc_fd_.reset();
+ source_ecc_open_failure_ = false;
source_path_.clear();
if (target_fd_ && !target_fd_->Close()) {
@@ -393,6 +404,46 @@
return true;
}
+bool DeltaPerformer::OpenCurrentECCPartition() {
+ if (source_ecc_fd_)
+ return true;
+
+ if (source_ecc_open_failure_)
+ return false;
+
+ if (current_partition_ >= partitions_.size())
+ return false;
+
+ // No support for ECC in minor version 1 or full payloads.
+ if (payload_->type == InstallPayloadType::kFull ||
+ GetMinorVersion() == kInPlaceMinorPayloadVersion)
+ return false;
+
+#if USE_FEC
+ const PartitionUpdate& partition = partitions_[current_partition_];
+ size_t num_previous_partitions =
+ install_plan_->partitions.size() - partitions_.size();
+ const InstallPlan::Partition& install_part =
+ install_plan_->partitions[num_previous_partitions + current_partition_];
+ string path = install_part.source_path;
+ FileDescriptorPtr fd(new FecFileDescriptor());
+ if (!fd->Open(path.c_str(), O_RDONLY, 0)) {
+ PLOG(ERROR) << "Unable to open ECC source partition "
+ << partition.partition_name() << " on slot "
+ << BootControlInterface::SlotName(install_plan_->source_slot)
+ << ", file " << path;
+ source_ecc_open_failure_ = true;
+ return false;
+ }
+ source_ecc_fd_ = fd;
+#else
+ // No support for ECC compiled.
+ source_ecc_open_failure_ = true;
+#endif // USE_FEC
+
+ return !source_ecc_open_failure_;
+}
+
namespace {
void LogPartitionInfoHash(const PartitionInfo& info, const string& tag) {
@@ -1025,19 +1076,70 @@
if (operation.has_dst_length())
TEST_AND_RETURN_FALSE(operation.dst_length() % block_size_ == 0);
- brillo::Blob source_hash;
- TEST_AND_RETURN_FALSE(fd_utils::CopyAndHashExtents(source_fd_,
- operation.src_extents(),
- target_fd_,
- operation.dst_extents(),
- block_size_,
- &source_hash));
-
if (operation.has_src_sha256_hash()) {
- TEST_AND_RETURN_FALSE(
- ValidateSourceHash(source_hash, operation, source_fd_, error));
- }
+ brillo::Blob source_hash;
+ brillo::Blob expected_source_hash(operation.src_sha256_hash().begin(),
+ operation.src_sha256_hash().end());
+ // 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.
+ // Note that this code will also fall back if writing the target partition
+ // fails.
+ bool read_ok = fd_utils::CopyAndHashExtents(source_fd_,
+ operation.src_extents(),
+ target_fd_,
+ operation.dst_extents(),
+ block_size_,
+ &source_hash);
+ if (read_ok && expected_source_hash == source_hash)
+ return true;
+
+ if (!OpenCurrentECCPartition()) {
+ // The following function call will return false since the source hash
+ // mismatches, but we still want to call it so it prints the appropriate
+ // log message.
+ return ValidateSourceHash(source_hash, operation, source_fd_, error);
+ }
+
+ LOG(WARNING) << "Source hash from RAW device mismatched: found "
+ << base::HexEncode(source_hash.data(), source_hash.size())
+ << ", expected "
+ << base::HexEncode(expected_source_hash.data(),
+ expected_source_hash.size());
+
+ TEST_AND_RETURN_FALSE(fd_utils::CopyAndHashExtents(source_ecc_fd_,
+ operation.src_extents(),
+ target_fd_,
+ operation.dst_extents(),
+ block_size_,
+ &source_hash));
+ TEST_AND_RETURN_FALSE(
+ ValidateSourceHash(source_hash, operation, source_ecc_fd_, error));
+ // At this point reading from the the error corrected device worked, but
+ // reading from the raw device failed, so this is considered a recovered
+ // failure.
+ source_ecc_recovered_failures_++;
+ } else {
+ // 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
+ // at this point, but we fall back to the raw device since the error
+ // corrected device can be shorter or not available.
+ if (OpenCurrentECCPartition() &&
+ fd_utils::CopyAndHashExtents(source_ecc_fd_,
+ operation.src_extents(),
+ target_fd_,
+ operation.dst_extents(),
+ block_size_,
+ nullptr)) {
+ return true;
+ }
+ TEST_AND_RETURN_FALSE(fd_utils::CopyAndHashExtents(source_fd_,
+ operation.src_extents(),
+ target_fd_,
+ operation.dst_extents(),
+ block_size_,
+ nullptr));
+ }
return true;
}
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 36c6e34..b156e2f 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -99,6 +99,10 @@
// work. Returns whether the required file descriptors were successfully open.
bool OpenCurrentPartition();
+ // Attempt to open the error-corrected device for the current partition.
+ // Returns whether the operation succeeded.
+ bool OpenCurrentECCPartition();
+
// Closes the current partition file descriptors if open. Returns 0 on success
// or -errno on error.
int CloseCurrentPartition();
@@ -283,6 +287,22 @@
// partition when using a delta payload.
FileDescriptorPtr source_fd_{nullptr};
+ // File descriptor of the error corrected source partition. Only set while
+ // updating partition using a delta payload for a partition where error
+ // correction is available. The size of the error corrected device is smaller
+ // than the underlying raw device, since it doesn't include the error
+ // correction blocks.
+ FileDescriptorPtr source_ecc_fd_{nullptr};
+
+ // The total number of operations that failed source hash verification but
+ // passed after falling back to the error-corrected |source_ecc_fd_| device.
+ uint64_t source_ecc_recovered_failures_{0};
+
+ // Whether opening the current partition as an error-corrected device failed.
+ // Used to avoid re-opening the same source partition if it is not actually
+ // error corrected.
+ bool source_ecc_open_failure_{false};
+
// File descriptor of the target partition. Only set while performing the
// operations of a given partition.
FileDescriptorPtr target_fd_{nullptr};
diff --git a/payload_consumer/delta_performer_unittest.cc b/payload_consumer/delta_performer_unittest.cc
index 99c19f9..921df99 100644
--- a/payload_consumer/delta_performer_unittest.cc
+++ b/payload_consumer/delta_performer_unittest.cc
@@ -39,6 +39,7 @@
#include "update_engine/common/fake_prefs.h"
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
+#include "update_engine/payload_consumer/fake_file_descriptor.h"
#include "update_engine/payload_consumer/mock_download_action.h"
#include "update_engine/payload_consumer/payload_constants.h"
#include "update_engine/payload_generator/bzip.h"
@@ -227,6 +228,24 @@
return payload_data;
}
+ brillo::Blob GenerateSourceCopyPayload(const brillo::Blob& copied_data,
+ bool add_hash) {
+ PayloadGenerationConfig config;
+ const uint64_t kDefaultBlockSize = config.block_size;
+ EXPECT_EQ(0U, copied_data.size() % kDefaultBlockSize);
+ uint64_t num_blocks = copied_data.size() / kDefaultBlockSize;
+ AnnotatedOperation aop;
+ *(aop.op.add_src_extents()) = ExtentForRange(0, num_blocks);
+ *(aop.op.add_dst_extents()) = ExtentForRange(0, num_blocks);
+ aop.op.set_type(InstallOperation::SOURCE_COPY);
+ brillo::Blob src_hash;
+ EXPECT_TRUE(HashCalculator::RawHashOfData(copied_data, &src_hash));
+ if (add_hash)
+ aop.op.set_src_sha256_hash(src_hash.data(), src_hash.size());
+
+ return GeneratePayload(brillo::Blob(), {aop}, false);
+ }
+
// Apply |payload_data| on partition specified in |source_path|.
// Expect result of performer_.Write() to be |expect_success|.
// Returns the result of the payload application.
@@ -376,6 +395,23 @@
EXPECT_EQ(payload_.metadata_size, performer_.metadata_size_);
}
+ // Helper function to pretend that the ECC file descriptor was already opened.
+ // Returns a pointer to the created file descriptor.
+ FakeFileDescriptor* SetFakeECCFile(size_t size) {
+ EXPECT_FALSE(performer_.source_ecc_fd_) << "source_ecc_fd_ already open.";
+ FakeFileDescriptor* ret = new FakeFileDescriptor();
+ fake_ecc_fd_.reset(ret);
+ // Call open to simulate it was already opened.
+ ret->Open("", 0);
+ ret->SetFileSize(size);
+ performer_.source_ecc_fd_ = fake_ecc_fd_;
+ return ret;
+ }
+
+ uint64_t GetSourceEccRecoveredFailures() const {
+ return performer_.source_ecc_recovered_failures_;
+ }
+
void SetSupportedMajorVersion(uint64_t major_version) {
performer_.supported_major_version_ = major_version;
}
@@ -385,6 +421,7 @@
FakeBootControl fake_boot_control_;
FakeHardware fake_hardware_;
MockDownloadActionDelegate mock_delegate_;
+ FileDescriptorPtr fake_ecc_fd_;
DeltaPerformer performer_{&prefs_,
&fake_boot_control_,
&fake_hardware_,
@@ -593,6 +630,60 @@
EXPECT_EQ(actual_data, ApplyPayload(payload_data, source_path, false));
}
+// Test that the error-corrected file descriptor is used to read the partition
+// since the source partition doesn't match the operation hash.
+TEST_F(DeltaPerformerTest, ErrorCorrectionSourceCopyWhenNoHashFallbackTest) {
+ const size_t kCopyOperationSize = 4 * 4096;
+ string source_path;
+ EXPECT_TRUE(utils::MakeTempFile("Source-XXXXXX", &source_path, nullptr));
+ ScopedPathUnlinker path_unlinker(source_path);
+ // Write invalid data to the source image, which doesn't match the expected
+ // hash.
+ brillo::Blob invalid_data(kCopyOperationSize, 0x55);
+ EXPECT_TRUE(utils::WriteFile(
+ source_path.c_str(), invalid_data.data(), invalid_data.size()));
+
+ // Setup the fec file descriptor as the fake stream, which matches
+ // |expected_data|.
+ FakeFileDescriptor* fake_fec = SetFakeECCFile(kCopyOperationSize);
+ brillo::Blob expected_data = FakeFileDescriptorData(kCopyOperationSize);
+
+ brillo::Blob payload_data = GenerateSourceCopyPayload(expected_data, true);
+ EXPECT_EQ(expected_data, ApplyPayload(payload_data, source_path, true));
+ // Verify that the fake_fec was actually used.
+ EXPECT_EQ(1U, fake_fec->GetReadOps().size());
+ EXPECT_EQ(1U, GetSourceEccRecoveredFailures());
+}
+
+// Test that the error-corrected file descriptor is used to read a partition
+// when no hash is available for SOURCE_COPY but it falls back to the normal
+// file descriptor when the size of the error corrected one is too small.
+TEST_F(DeltaPerformerTest, ErrorCorrectionSourceCopyFallbackTest) {
+ const size_t kCopyOperationSize = 4 * 4096;
+ string source_path;
+ EXPECT_TRUE(utils::MakeTempFile("Source-XXXXXX", &source_path, nullptr));
+ ScopedPathUnlinker path_unlinker(source_path);
+ // Setup the source path with the right expected data.
+ brillo::Blob expected_data = FakeFileDescriptorData(kCopyOperationSize);
+ EXPECT_TRUE(utils::WriteFile(
+ source_path.c_str(), expected_data.data(), expected_data.size()));
+
+ // Setup the fec file descriptor as the fake stream, with smaller data than
+ // the expected.
+ FakeFileDescriptor* fake_fec = SetFakeECCFile(kCopyOperationSize / 2);
+
+ // The payload operation doesn't include an operation hash.
+ brillo::Blob payload_data = GenerateSourceCopyPayload(expected_data, false);
+ EXPECT_EQ(expected_data, ApplyPayload(payload_data, source_path, true));
+ // Verify that the fake_fec was attempted to be used. Since the file
+ // descriptor is shorter it can actually do more than one read to realize it
+ // reached the EOF.
+ EXPECT_LE(1U, fake_fec->GetReadOps().size());
+ // This fallback doesn't count as an error-corrected operation since the
+ // operation hash was not available.
+ EXPECT_EQ(0U, GetSourceEccRecoveredFailures());
+}
+
TEST_F(DeltaPerformerTest, ExtentsToByteStringTest) {
uint64_t test[] = {1, 1, 4, 2, 0, 1};
static_assert(arraysize(test) % 2 == 0, "Array size uneven");
diff --git a/payload_consumer/fake_file_descriptor.cc b/payload_consumer/fake_file_descriptor.cc
index d54856b..63af181 100644
--- a/payload_consumer/fake_file_descriptor.cc
+++ b/payload_consumer/fake_file_descriptor.cc
@@ -73,4 +73,12 @@
return offset_;
}
+brillo::Blob FakeFileDescriptorData(size_t size) {
+ brillo::Blob ret(size);
+ FakeFileDescriptor fd;
+ fd.SetFileSize(size);
+ fd.Read(ret.data(), size);
+ return ret;
+}
+
} // namespace chromeos_update_engine
diff --git a/payload_consumer/fake_file_descriptor.h b/payload_consumer/fake_file_descriptor.h
index f17820b..c9fea7d 100644
--- a/payload_consumer/fake_file_descriptor.h
+++ b/payload_consumer/fake_file_descriptor.h
@@ -22,6 +22,8 @@
#include <utility>
#include <vector>
+#include <brillo/secure_blob.h>
+
#include "update_engine/payload_consumer/file_descriptor.h"
namespace chromeos_update_engine {
@@ -121,6 +123,9 @@
DISALLOW_COPY_AND_ASSIGN(FakeFileDescriptor);
};
+// Return a blob with the first |size| bytes of a FakeFileDescriptor stream.
+brillo::Blob FakeFileDescriptorData(size_t size);
+
} // namespace chromeos_update_engine
#endif // UPDATE_ENGINE_PAYLOAD_CONSUMER_FAKE_FILE_DESCRIPTOR_H_
diff --git a/payload_consumer/fec_file_descriptor.cc b/payload_consumer/fec_file_descriptor.cc
new file mode 100644
index 0000000..de22cf3
--- /dev/null
+++ b/payload_consumer/fec_file_descriptor.cc
@@ -0,0 +1,78 @@
+//
+// Copyright (C) 2017 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/fec_file_descriptor.h"
+
+namespace chromeos_update_engine {
+
+bool FecFileDescriptor::Open(const char* path, int flags) {
+ return Open(path, flags, 0600);
+}
+
+bool FecFileDescriptor::Open(const char* path, int flags, mode_t mode) {
+ if (!fh_.open(path, flags, mode))
+ return false;
+
+ if (!fh_.has_ecc()) {
+ LOG(ERROR) << "No ECC data in the passed file";
+ fh_.close();
+ return false;
+ }
+
+ fec_status status;
+ if (!fh_.get_status(status)) {
+ LOG(ERROR) << "Couldn't load ECC status";
+ fh_.close();
+ return false;
+ }
+
+ dev_size_ = status.data_size;
+ return true;
+}
+
+ssize_t FecFileDescriptor::Read(void* buf, size_t count) {
+ return fh_.read(buf, count);
+}
+
+ssize_t FecFileDescriptor::Write(const void* buf, size_t count) {
+ errno = EROFS;
+ return -1;
+}
+
+off64_t FecFileDescriptor::Seek(off64_t offset, int whence) {
+ if (fh_.seek(offset, whence)) {
+ return offset;
+ }
+ return -1;
+}
+
+uint64_t FecFileDescriptor::BlockDevSize() {
+ return dev_size_;
+}
+
+bool FecFileDescriptor::BlkIoctl(int request,
+ uint64_t start,
+ uint64_t length,
+ int* result) {
+ // No IOCTL pass-through in this mode.
+ return false;
+}
+
+bool FecFileDescriptor::Close() {
+ return fh_.close();
+}
+
+} // namespace chromeos_update_engine
diff --git a/payload_consumer/fec_file_descriptor.h b/payload_consumer/fec_file_descriptor.h
new file mode 100644
index 0000000..e7f2e40
--- /dev/null
+++ b/payload_consumer/fec_file_descriptor.h
@@ -0,0 +1,65 @@
+//
+// Copyright (C) 2017 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.
+//
+
+#ifndef UPDATE_ENGINE_PAYLOAD_CONSUMER_FEC_FILE_DESCRIPTOR_H_
+#define UPDATE_ENGINE_PAYLOAD_CONSUMER_FEC_FILE_DESCRIPTOR_H_
+
+#include <fec/io.h>
+
+#include "update_engine/payload_consumer/file_descriptor.h"
+
+// A FileDescriptor implementation with error correction based on the "libfec"
+// library. The libfec on the running system allows to parse the error
+// correction blocks stored in partitions that have verity and error correction
+// enabled. This information is present in the raw block device, but of course
+// not available via the dm-verity block device.
+
+namespace chromeos_update_engine {
+
+// An error corrected file based on FEC.
+class FecFileDescriptor : public FileDescriptor {
+ public:
+ FecFileDescriptor() = default;
+ ~FecFileDescriptor() = default;
+
+ // Interface methods.
+ bool Open(const char* path, int flags, mode_t mode) override;
+ bool Open(const char* path, int flags) override;
+ ssize_t Read(void* buf, size_t count) override;
+ ssize_t Write(const void* buf, size_t count) override;
+ off64_t Seek(off64_t offset, int whence) override;
+ uint64_t BlockDevSize() override;
+ bool BlkIoctl(int request,
+ uint64_t start,
+ uint64_t length,
+ int* result) override;
+ bool Flush() override { return true; }
+ bool Close() override;
+ bool IsSettingErrno() override { return true; }
+ bool IsOpen() override {
+ // The bool operator on the fec::io class tells whether the internal
+ // handle is open.
+ return static_cast<bool>(fh_);
+ }
+
+ protected:
+ fec::io fh_;
+ uint64_t dev_size_{0};
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_PAYLOAD_CONSUMER_FEC_FILE_DESCRIPTOR_H_
diff --git a/update_engine.gyp b/update_engine.gyp
index ba46266..7cb2a7d 100644
--- a/update_engine.gyp
+++ b/update_engine.gyp
@@ -50,6 +50,7 @@
'_POSIX_C_SOURCE=199309L',
'USE_BINDER=<(USE_binder)',
'USE_DBUS=<(USE_dbus)',
+ 'USE_FEC=0',
'USE_HWID_OVERRIDE=<(USE_hwid_override)',
'USE_CHROME_KIOSK_APP=<(USE_chrome_kiosk_app)',
'USE_CHROME_NETWORK_PROXY=<(USE_chrome_network_proxy)',