From f06debcf24509a2bd33ebae51aaeffdc675fc2be Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 16 Mar 2023 21:23:53 -0700 Subject: [PATCH 1/2] libsparse: Fix allocation failures on 32-bit systems. libsparse uses mapped files for length computation checks and writing output data. The platform-tools package for Windows is 32-bit, and if an embedded file in the stream is large enough, mapping will fail. In theory, this failure mode could happen on 64-bit systems as well. As a workaround, map files in chunks of 256MB instead. This is implemented by adding a new "fd_chunk" callback to the sparse ops struct. Bug: 273933042 Bug: 268872725 Test: fastboot update on Windows Change-Id: Ic40696b34a1d0951787c899db701fc2fa204eb18 --- libsparse/output_file.cpp | 102 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 98 insertions(+), 4 deletions(-) diff --git a/libsparse/output_file.cpp b/libsparse/output_file.cpp index cb5d73052..08312e4d3 100644 --- a/libsparse/output_file.cpp +++ b/libsparse/output_file.cpp @@ -58,6 +58,8 @@ #define container_of(inner, outer_t, elem) ((outer_t*)((char*)(inner)-offsetof(outer_t, elem))) +static constexpr size_t kMaxMmapSize = 256 * 1024 * 1024; + struct output_file_ops { int (*open)(struct output_file*, int fd); int (*skip)(struct output_file*, int64_t); @@ -71,6 +73,7 @@ struct sparse_file_ops { int (*write_fill_chunk)(struct output_file* out, uint64_t len, uint32_t fill_val); int (*write_skip_chunk)(struct output_file* out, uint64_t len); int (*write_end_chunk)(struct output_file* out); + int (*write_fd_chunk)(struct output_file* out, uint64_t len, int fd, int64_t offset); }; struct output_file { @@ -318,6 +321,26 @@ int read_all(int fd, void* buf, size_t len) { return 0; } +template +static bool write_fd_chunk_range(int fd, int64_t offset, uint64_t len, T callback) { + uint64_t bytes_written = 0; + int64_t current_offset = offset; + while (bytes_written < len) { + size_t mmap_size = std::min(static_cast(kMaxMmapSize), len - bytes_written); + auto m = android::base::MappedFile::FromFd(fd, current_offset, mmap_size, PROT_READ); + if (!m) { + error("failed to mmap region of length %zu", mmap_size); + return false; + } + if (!callback(m->data(), mmap_size)) { + return false; + } + bytes_written += mmap_size; + current_offset += mmap_size; + } + return true; +} + static int write_sparse_skip_chunk(struct output_file* out, uint64_t skip_len) { chunk_header_t chunk_header; int ret; @@ -424,6 +447,61 @@ static int write_sparse_data_chunk(struct output_file* out, uint64_t len, void* return 0; } +static int write_sparse_fd_chunk(struct output_file* out, uint64_t len, int fd, int64_t offset) { + chunk_header_t chunk_header; + uint64_t rnd_up_len, zero_len; + int ret; + + /* Round up the data length to a multiple of the block size */ + rnd_up_len = ALIGN(len, out->block_size); + zero_len = rnd_up_len - len; + + /* Finally we can safely emit a chunk of data */ + chunk_header.chunk_type = CHUNK_TYPE_RAW; + chunk_header.reserved1 = 0; + chunk_header.chunk_sz = rnd_up_len / out->block_size; + chunk_header.total_sz = CHUNK_HEADER_LEN + rnd_up_len; + ret = out->ops->write(out, &chunk_header, sizeof(chunk_header)); + + if (ret < 0) return -1; + bool ok = write_fd_chunk_range(fd, offset, len, [&ret, out](char* data, size_t size) -> bool { + ret = out->ops->write(out, data, size); + if (ret < 0) return false; + if (out->use_crc) { + out->crc32 = sparse_crc32(out->crc32, data, size); + } + return true; + }); + if (!ok) return -1; + if (zero_len) { + uint64_t len = zero_len; + uint64_t write_len; + while (len) { + write_len = std::min(len, (uint64_t)FILL_ZERO_BUFSIZE); + ret = out->ops->write(out, out->zero_buf, write_len); + if (ret < 0) { + return ret; + } + len -= write_len; + } + + if (out->use_crc) { + uint64_t len = zero_len; + uint64_t write_len; + while (len) { + write_len = std::min(len, (uint64_t)FILL_ZERO_BUFSIZE); + out->crc32 = sparse_crc32(out->crc32, out->zero_buf, write_len); + len -= write_len; + } + } + } + + out->cur_out_ptr += rnd_up_len; + out->chunk_cnt++; + + return 0; +} + int write_sparse_end_chunk(struct output_file* out) { chunk_header_t chunk_header; int ret; @@ -454,6 +532,7 @@ static struct sparse_file_ops sparse_file_ops = { .write_fill_chunk = write_sparse_fill_chunk, .write_skip_chunk = write_sparse_skip_chunk, .write_end_chunk = write_sparse_end_chunk, + .write_fd_chunk = write_sparse_fd_chunk, }; static int write_normal_data_chunk(struct output_file* out, uint64_t len, void* data) { @@ -495,6 +574,23 @@ static int write_normal_fill_chunk(struct output_file* out, uint64_t len, uint32 return 0; } +static int write_normal_fd_chunk(struct output_file* out, uint64_t len, int fd, int64_t offset) { + int ret; + uint64_t rnd_up_len = ALIGN(len, out->block_size); + + bool ok = write_fd_chunk_range(fd, offset, len, [&ret, out](char* data, size_t size) -> bool { + ret = out->ops->write(out, data, size); + return ret >= 0; + }); + if (!ok) return ret; + + if (rnd_up_len > len) { + ret = out->ops->skip(out, rnd_up_len - len); + } + + return ret; +} + static int write_normal_skip_chunk(struct output_file* out, uint64_t len) { return out->ops->skip(out, len); } @@ -508,6 +604,7 @@ static struct sparse_file_ops normal_file_ops = { .write_fill_chunk = write_normal_fill_chunk, .write_skip_chunk = write_normal_skip_chunk, .write_end_chunk = write_normal_end_chunk, + .write_fd_chunk = write_normal_fd_chunk, }; void output_file_close(struct output_file* out) { @@ -670,10 +767,7 @@ int write_fill_chunk(struct output_file* out, uint64_t len, uint32_t fill_val) { } int write_fd_chunk(struct output_file* out, uint64_t len, int fd, int64_t offset) { - auto m = android::base::MappedFile::FromFd(fd, offset, len, PROT_READ); - if (!m) return -errno; - - return out->sparse_ops->write_data_chunk(out, m->size(), m->data()); + return out->sparse_ops->write_fd_chunk(out, len, fd, offset); } /* Write a contiguous region of data blocks from a file */ From 74c7807af1fffdf81b93144389a10abfa5c969c1 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 16 Mar 2023 21:21:28 -0700 Subject: [PATCH 2/2] fastboot: Handle libsparse failures better. sparse_file_len is not actually infallible; on Windows it's pretty easy to make it fail by embedding large files in the stream. fastboot didn't handle this anywhere, leading to bad sparse images when libsparse runs out of address space. Bug: 273933042 Bug: 268872725 Test: fastboot flashall on Windows Change-Id: Ie68aed2f1970e820350d9f97aa89a6c0242229b8 --- fastboot/fastboot.cpp | 18 ++++++++---------- fastboot/task.cpp | 24 +++++++++++++++++++----- fastboot/task.h | 3 ++- fastboot/util.cpp | 12 ++++++++++++ fastboot/util.h | 1 + 5 files changed, 42 insertions(+), 16 deletions(-) diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp index 15d187437..3cb878d69 100644 --- a/fastboot/fastboot.cpp +++ b/fastboot/fastboot.cpp @@ -988,7 +988,7 @@ std::vector resparse_file(sparse_file* s, int64_t max_size) { } const int files = sparse_file_resparse(s, max_size, nullptr, 0); - if (files < 0) die("Failed to resparse"); + if (files < 0) die("Failed to compute resparse boundaries"); auto temp = std::make_unique(files); const int rv = sparse_file_resparse(s, max_size, temp.get(), files); @@ -1057,6 +1057,10 @@ static bool load_buf_fd(unique_fd fd, struct fastboot_buffer* buf) { if (sparse_file* s = sparse_file_import(fd.get(), false, false)) { buf->image_size = sparse_file_len(s, false, false); + if (buf->image_size < 0) { + LOG(ERROR) << "Could not compute length of sparse file"; + return false; + } sparse_file_destroy(s); } else { buf->image_size = sz; @@ -1166,15 +1170,6 @@ bool is_logical(const std::string& partition) { return fb->GetVar("is-logical:" + partition, &value) == fastboot::SUCCESS && value == "yes"; } -static std::string fb_fix_numeric_var(std::string var) { - // Some bootloaders (angler, for example), send spurious leading whitespace. - var = android::base::Trim(var); - // Some bootloaders (hammerhead, for example) use implicit hex. - // This code used to use strtol with base 16. - if (!android::base::StartsWith(var, "0x")) var = "0x" + var; - return var; -} - static uint64_t get_partition_size(const std::string& partition) { std::string partition_size_str; if (fb->GetVar("partition-size:" + partition, &partition_size_str) != fastboot::SUCCESS) { @@ -1247,6 +1242,9 @@ void flash_partition_files(const std::string& partition, const std::vectorFlashPartition(partition, s, sz, i + 1, files.size()); } } diff --git a/fastboot/task.cpp b/fastboot/task.cpp index c70139b9a..b21558c60 100644 --- a/fastboot/task.cpp +++ b/fastboot/task.cpp @@ -19,6 +19,8 @@ #include "filesystem.h" #include "super_flash_helper.h" +#include + using namespace std::string_literals; FlashTask::FlashTask(const std::string& slot, const std::string& pname) @@ -70,14 +72,18 @@ void RebootTask::Run() { FlashSuperLayoutTask::FlashSuperLayoutTask(const std::string& super_name, std::unique_ptr helper, - SparsePtr sparse_layout) + SparsePtr sparse_layout, uint64_t super_size) : super_name_(super_name), helper_(std::move(helper)), - sparse_layout_(std::move(sparse_layout)) {} + sparse_layout_(std::move(sparse_layout)), + super_size_(super_size) {} void FlashSuperLayoutTask::Run() { + // Use the reported super partition size as the upper limit, rather than + // sparse_file_len, which (1) can fail and (2) is kind of expensive, since + // it will map in all of the embedded fds. std::vector files; - if (int limit = get_sparse_limit(sparse_file_len(sparse_layout_.get(), false, false))) { + if (int limit = get_sparse_limit(super_size_)) { files = resparse_file(sparse_layout_.get(), limit); } else { files.emplace_back(std::move(sparse_layout_)); @@ -111,12 +117,19 @@ std::unique_ptr FlashSuperLayoutTask::Initialize( if (fp->fb->GetVar("super-partition-name", &super_name) != fastboot::SUCCESS) { super_name = "super"; } - std::string partition_size_str; + uint64_t partition_size; + std::string partition_size_str; if (fp->fb->GetVar("partition-size:" + super_name, &partition_size_str) != fastboot::SUCCESS) { LOG(VERBOSE) << "Cannot optimize super flashing: could not determine super partition"; return nullptr; } + partition_size_str = fb_fix_numeric_var(partition_size_str); + if (!android::base::ParseUint(partition_size_str, &partition_size)) { + LOG(VERBOSE) << "Could not parse " << super_name << " size: " << partition_size_str; + return nullptr; + } + std::unique_ptr helper = std::make_unique(*fp->source); if (!helper->Open(fd)) { return nullptr; @@ -140,7 +153,8 @@ std::unique_ptr FlashSuperLayoutTask::Initialize( }; os_images.erase(std::remove_if(os_images.begin(), os_images.end(), remove_if_callback), os_images.end()); - return std::make_unique(super_name, std::move(helper), std::move(s)); + return std::make_unique(super_name, std::move(helper), std::move(s), + partition_size); } UpdateSuperTask::UpdateSuperTask(FlashingPlan* fp) : fp_(fp) {} diff --git a/fastboot/task.h b/fastboot/task.h index 149c34cc5..c4914d1e5 100644 --- a/fastboot/task.h +++ b/fastboot/task.h @@ -57,7 +57,7 @@ class RebootTask : public Task { class FlashSuperLayoutTask : public Task { public: FlashSuperLayoutTask(const std::string& super_name, std::unique_ptr helper, - SparsePtr sparse_layout); + SparsePtr sparse_layout, uint64_t super_size); static std::unique_ptr Initialize(FlashingPlan* fp, std::vector& os_images); using ImageEntry = std::pair; @@ -67,6 +67,7 @@ class FlashSuperLayoutTask : public Task { const std::string super_name_; std::unique_ptr helper_; SparsePtr sparse_layout_; + uint64_t super_size_; }; class UpdateSuperTask : public Task { diff --git a/fastboot/util.cpp b/fastboot/util.cpp index ded54a5b9..e03012a05 100644 --- a/fastboot/util.cpp +++ b/fastboot/util.cpp @@ -33,6 +33,9 @@ #include #include +#include +#include + #include "util.h" using android::base::borrowed_fd; @@ -106,3 +109,12 @@ int64_t get_file_size(borrowed_fd fd) { } return sb.st_size; } + +std::string fb_fix_numeric_var(std::string var) { + // Some bootloaders (angler, for example), send spurious leading whitespace. + var = android::base::Trim(var); + // Some bootloaders (hammerhead, for example) use implicit hex. + // This code used to use strtol with base 16. + if (!android::base::StartsWith(var, "0x")) var = "0x" + var; + return var; +} diff --git a/fastboot/util.h b/fastboot/util.h index 290d0d5fe..fdbc1d699 100644 --- a/fastboot/util.h +++ b/fastboot/util.h @@ -30,6 +30,7 @@ bool should_flash_in_userspace(const android::fs_mgr::LpMetadata& metadata, const std::string& partition_name); bool is_sparse_file(android::base::borrowed_fd fd); int64_t get_file_size(android::base::borrowed_fd fd); +std::string fb_fix_numeric_var(std::string var); class ImageSource { public: