From d2aecd465b7fe24f8302c85f3e062b21b9762d0f Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 23 Mar 2017 14:43:44 -0700 Subject: updater: Clean up LoadSrcTgtVersion2(). Rename to LoadSourceBlocks() by moving the target blocks parsing part into the caller. This allows detecting whether the target blocks have already had the expected data before loading the source blocks. It doesn't affect anything when applying an update package for the first time, but it skips loading the unneeded source blocks when resuming an update. It additionally avoids unnecessarily dumping the "corrupt" source/stash blocks when resuming an update. Bug: 33694730 Test: Apply an incremental update with the new updater. Test: Resume an incremental update with the new updater. Change-Id: I794fd0d1045be7b3b7f8619285dc0dade01398d0 --- updater/blockimg.cpp | 289 +++++++++++++++++++++++++-------------------------- 1 file changed, 143 insertions(+), 146 deletions(-) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index c614ccc47..1cad6da92 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -696,7 +696,7 @@ static int LoadStash(CommandParameters& params, const std::string& id, bool veri } static int WriteStash(const std::string& base, const std::string& id, int blocks, - std::vector& buffer, bool checkspace, bool *exists) { + std::vector& buffer, bool checkspace, bool* exists) { if (base.empty()) { return -1; } @@ -883,96 +883,81 @@ static void MoveRange(std::vector& dest, const RangeSet& locs, } } -// Do a source/target load for move/bsdiff/imgdiff in version 2. -// We expect to parse the remainder of the parameter tokens as one of: -// -// -// (loads data from source image only) -// -// - <[stash_id:stash_range] ...> -// (loads data from stashes only) -// -// <[stash_id:stash_range] ...> -// (loads data from both source image and stashes) -// -// On return, params.buffer is filled with the loaded source data (rearranged and combined with -// stashed data as necessary). buffer may be reallocated if needed to accommodate the source data. -// *tgt is the target RangeSet. Any stashes required are loaded using LoadStash. - -static int LoadSrcTgtVersion2(CommandParameters& params, RangeSet& tgt, size_t& src_blocks, - bool* overlap) { +/** + * We expect to parse the remainder of the parameter tokens as one of: + * + * + * (loads data from source image only) + * + * - <[stash_id:stash_range] ...> + * (loads data from stashes only) + * + * <[stash_id:stash_range] ...> + * (loads data from both source image and stashes) + * + * On return, params.buffer is filled with the loaded source data (rearranged and combined with + * stashed data as necessary). buffer may be reallocated if needed to accommodate the source data. + * tgt is the target RangeSet for detecting overlaps. Any stashes required are loaded using + * LoadStash. + */ +static int LoadSourceBlocks(CommandParameters& params, const RangeSet& tgt, size_t* src_blocks, + bool* overlap) { + CHECK(src_blocks != nullptr); + CHECK(overlap != nullptr); + + // + const std::string& token = params.tokens[params.cpos++]; + if (!android::base::ParseUint(token, src_blocks)) { + LOG(ERROR) << "invalid src_block_count \"" << token << "\""; + return -1; + } - // At least it needs to provide three parameters: , - // and "-"/. - if (params.cpos + 2 >= params.tokens.size()) { - LOG(ERROR) << "invalid parameters"; - return -1; - } + allocate(*src_blocks * BLOCKSIZE, params.buffer); - // - tgt = parse_range(params.tokens[params.cpos++]); + // "-" or [] + if (params.tokens[params.cpos] == "-") { + // no source ranges, only stashes + params.cpos++; + } else { + RangeSet src = parse_range(params.tokens[params.cpos++]); + *overlap = range_overlaps(src, tgt); - // - const std::string& token = params.tokens[params.cpos++]; - if (!android::base::ParseUint(token.c_str(), &src_blocks)) { - LOG(ERROR) << "invalid src_block_count \"" << token << "\""; - return -1; + if (ReadBlocks(src, params.buffer, params.fd) == -1) { + return -1; } - allocate(src_blocks * BLOCKSIZE, params.buffer); - - // "-" or [] - if (params.tokens[params.cpos] == "-") { - // no source ranges, only stashes - params.cpos++; - } else { - RangeSet src = parse_range(params.tokens[params.cpos++]); - int res = ReadBlocks(src, params.buffer, params.fd); - - if (overlap) { - *overlap = range_overlaps(src, tgt); - } - - if (res == -1) { - return -1; - } - - if (params.cpos >= params.tokens.size()) { - // no stashes, only source range - return 0; - } - - RangeSet locs = parse_range(params.tokens[params.cpos++]); - MoveRange(params.buffer, locs, params.buffer); + if (params.cpos >= params.tokens.size()) { + // no stashes, only source range + return 0; } - // <[stash_id:stash_range]> - while (params.cpos < params.tokens.size()) { - // Each word is a an index into the stash table, a colon, and - // then a rangeset describing where in the source block that - // stashed data should go. - std::vector tokens = android::base::Split(params.tokens[params.cpos++], ":"); - if (tokens.size() != 2) { - LOG(ERROR) << "invalid parameter"; - return -1; - } - - std::vector stash; - int res = LoadStash(params, tokens[0], false, nullptr, stash, true); - - if (res == -1) { - // These source blocks will fail verification if used later, but we - // will let the caller decide if this is a fatal failure - LOG(ERROR) << "failed to load stash " << tokens[0]; - continue; - } + RangeSet locs = parse_range(params.tokens[params.cpos++]); + MoveRange(params.buffer, locs, params.buffer); + } - RangeSet locs = parse_range(tokens[1]); + // <[stash_id:stash_range]> + while (params.cpos < params.tokens.size()) { + // Each word is a an index into the stash table, a colon, and then a RangeSet describing where + // in the source block that stashed data should go. + std::vector tokens = android::base::Split(params.tokens[params.cpos++], ":"); + if (tokens.size() != 2) { + LOG(ERROR) << "invalid parameter"; + return -1; + } - MoveRange(params.buffer, locs, stash); + std::vector stash; + if (LoadStash(params, tokens[0], false, nullptr, stash, true) == -1) { + // These source blocks will fail verification if used later, but we + // will let the caller decide if this is a fatal failure + LOG(ERROR) << "failed to load stash " << tokens[0]; + continue; } - return 0; + RangeSet locs = parse_range(tokens[1]); + MoveRange(params.buffer, locs, stash); + } + + return 0; } /** @@ -989,9 +974,8 @@ static int LoadSrcTgtVersion2(CommandParameters& params, RangeSet& tgt, size_t& * <[stash_id:stash_range] ...> * (loads data from both source image and stashes) * - * Parameters are the same as for LoadSrcTgtVersion2, except for 'onehash', which tells the function - * whether to expect separate source and targe block hashes, or if they are both the same and only - * one hash should be expected, and 'isunresumable', which receives a non-zero value if block + * 'onehash' tells whether to expect separate source and targe block hashes, or if they are both the + * same and only one hash should be expected. params.isunresumable will be set to true if block * verification fails in a way that the update cannot be resumed anymore. * * If the function is unable to load the necessary blocks or their contents don't match the hashes, @@ -1002,87 +986,100 @@ static int LoadSrcTgtVersion2(CommandParameters& params, RangeSet& tgt, size_t& * * If the return value is 0, source blocks have expected content and the command can be performed. */ -static int LoadSrcTgtVersion3(CommandParameters& params, RangeSet& tgt, size_t& src_blocks, - bool onehash, bool& overlap) { - if (params.cpos >= params.tokens.size()) { - LOG(ERROR) << "missing source hash"; - return -1; - } +static int LoadSrcTgtVersion3(CommandParameters& params, RangeSet& tgt, size_t* src_blocks, + bool onehash, bool* overlap) { + CHECK(src_blocks != nullptr); + CHECK(overlap != nullptr); - std::string srchash = params.tokens[params.cpos++]; - std::string tgthash; + if (params.cpos >= params.tokens.size()) { + LOG(ERROR) << "missing source hash"; + return -1; + } - if (onehash) { - tgthash = srchash; - } else { - if (params.cpos >= params.tokens.size()) { - LOG(ERROR) << "missing target hash"; - return -1; - } - tgthash = params.tokens[params.cpos++]; - } + std::string srchash = params.tokens[params.cpos++]; + std::string tgthash; - if (LoadSrcTgtVersion2(params, tgt, src_blocks, &overlap) == -1) { - return -1; + if (onehash) { + tgthash = srchash; + } else { + if (params.cpos >= params.tokens.size()) { + LOG(ERROR) << "missing target hash"; + return -1; } + tgthash = params.tokens[params.cpos++]; + } - std::vector tgtbuffer(tgt.size * BLOCKSIZE); + // At least it needs to provide three parameters: , and + // "-"/. + if (params.cpos + 2 >= params.tokens.size()) { + LOG(ERROR) << "invalid parameters"; + return -1; + } - if (ReadBlocks(tgt, tgtbuffer, params.fd) == -1) { - return -1; - } + // + tgt = parse_range(params.tokens[params.cpos++]); - if (VerifyBlocks(tgthash, tgtbuffer, tgt.size, false) == 0) { - // Target blocks already have expected content, command should be skipped. - return 1; - } + std::vector tgtbuffer(tgt.size * BLOCKSIZE); + if (ReadBlocks(tgt, tgtbuffer, params.fd) == -1) { + return -1; + } - if (VerifyBlocks(srchash, params.buffer, src_blocks, true) == 0) { - // If source and target blocks overlap, stash the source blocks so we can - // resume from possible write errors. In verify mode, we can skip stashing - // because the source blocks won't be overwritten. - if (overlap && params.canwrite) { - LOG(INFO) << "stashing " << src_blocks << " overlapping blocks to " << srchash; + // Return now if target blocks already have expected content. + if (VerifyBlocks(tgthash, tgtbuffer, tgt.size, false) == 0) { + return 1; + } - bool stash_exists = false; - if (WriteStash(params.stashbase, srchash, src_blocks, params.buffer, true, - &stash_exists) != 0) { - LOG(ERROR) << "failed to stash overlapping source blocks"; - return -1; - } + // Load source blocks. + if (LoadSourceBlocks(params, tgt, src_blocks, overlap) == -1) { + return -1; + } - params.stashed += src_blocks; - // Can be deleted when the write has completed. - if (!stash_exists) { - params.freestash = srchash; - } - } + if (VerifyBlocks(srchash, params.buffer, *src_blocks, true) == 0) { + // If source and target blocks overlap, stash the source blocks so we can + // resume from possible write errors. In verify mode, we can skip stashing + // because the source blocks won't be overwritten. + if (*overlap && params.canwrite) { + LOG(INFO) << "stashing " << *src_blocks << " overlapping blocks to " << srchash; + + bool stash_exists = false; + if (WriteStash(params.stashbase, srchash, *src_blocks, params.buffer, true, + &stash_exists) != 0) { + LOG(ERROR) << "failed to stash overlapping source blocks"; + return -1; + } - // Source blocks have expected content, command can proceed. - return 0; + params.stashed += *src_blocks; + // Can be deleted when the write has completed. + if (!stash_exists) { + params.freestash = srchash; + } } - if (overlap && LoadStash(params, srchash, true, nullptr, params.buffer, true) == 0) { - // Overlapping source blocks were previously stashed, command can proceed. - // We are recovering from an interrupted command, so we don't know if the - // stash can safely be deleted after this command. - return 0; - } + // Source blocks have expected content, command can proceed. + return 0; + } - // Valid source data not available, update cannot be resumed. - LOG(ERROR) << "partition has unexpected contents"; - PrintHashForCorruptedSourceBlocks(params, params.buffer); + if (*overlap && LoadStash(params, srchash, true, nullptr, params.buffer, true) == 0) { + // Overlapping source blocks were previously stashed, command can proceed. We are recovering + // from an interrupted command, so we don't know if the stash can safely be deleted after this + // command. + return 0; + } - params.isunresumable = true; + // Valid source data not available, update cannot be resumed. + LOG(ERROR) << "partition has unexpected contents"; + PrintHashForCorruptedSourceBlocks(params, params.buffer); - return -1; + params.isunresumable = true; + + return -1; } static int PerformCommandMove(CommandParameters& params) { size_t blocks = 0; bool overlap = false; RangeSet tgt; - int status = LoadSrcTgtVersion3(params, tgt, blocks, true, overlap); + int status = LoadSrcTgtVersion3(params, tgt, &blocks, true, &overlap); if (status == -1) { LOG(ERROR) << "failed to read blocks for move"; @@ -1270,13 +1267,13 @@ static int PerformCommandDiff(CommandParameters& params) { } size_t offset; - if (!android::base::ParseUint(params.tokens[params.cpos++].c_str(), &offset)) { + if (!android::base::ParseUint(params.tokens[params.cpos++], &offset)) { LOG(ERROR) << "invalid patch offset"; return -1; } size_t len; - if (!android::base::ParseUint(params.tokens[params.cpos++].c_str(), &len)) { + if (!android::base::ParseUint(params.tokens[params.cpos++], &len)) { LOG(ERROR) << "invalid patch len"; return -1; } @@ -1284,7 +1281,7 @@ static int PerformCommandDiff(CommandParameters& params) { RangeSet tgt; size_t blocks = 0; bool overlap = false; - int status = LoadSrcTgtVersion3(params, tgt, blocks, false, overlap); + int status = LoadSrcTgtVersion3(params, tgt, &blocks, false, &overlap); if (status == -1) { LOG(ERROR) << "failed to read blocks for diff"; @@ -1871,7 +1868,7 @@ Value* BlockImageRecoverFn(const char* name, State* state, LOG(INFO) << filename->data << " image corrupted, attempting to recover..."; // When opened with O_RDWR, libfec rewrites corrupted blocks when they are read - fec::io fh(filename->data.c_str(), O_RDWR); + fec::io fh(filename->data, O_RDWR); if (!fh) { ErrorAbort(state, kLibfecFailure, "fec_open \"%s\" failed: %s", filename->data.c_str(), -- cgit v1.2.3 From f7eb760fe76cb66c5d5341b03d6a66cc1f06d795 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 27 Mar 2017 15:12:48 -0700 Subject: applypatch: Change the ssize_t length parameters to size_t. Mostly for applypatch family APIs like ApplyBSDiffPatch() and ApplyImagePatch(). Changing to size_t doesn't indicate they would necessarily work with very large size_t (e.g. > ssize_t), just similar to write(2). But otherwise accepting negative length doesn't make much sense. Also change the return type of SinkFn from ssize_t to size_t. Callers tell a successful sink by comparing the number of written bytes against the desired value. Negative return values like -1 are not needed. This also makes it consistent with bsdiff::bspatch interface. Test: recovery_component_test Test: Apply an incremental with the new updater. Change-Id: I7ff1615203a5c9854134f75d019e266f4ea6e714 --- updater/blockimg.cpp | 81 ++++++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 41 deletions(-) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 1cad6da92..5a27ff41a 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -240,57 +240,56 @@ struct RangeSinkState { size_t p_remain; }; -static ssize_t RangeSinkWrite(const uint8_t* data, ssize_t size, void* token) { - RangeSinkState* rss = reinterpret_cast(token); +static size_t RangeSinkWrite(const uint8_t* data, size_t size, void* token) { + RangeSinkState* rss = static_cast(token); - if (rss->p_remain == 0) { - LOG(ERROR) << "range sink write overrun"; - return 0; - } - - ssize_t written = 0; - while (size > 0) { - size_t write_now = size; - - if (rss->p_remain < write_now) { - write_now = rss->p_remain; - } + if (rss->p_remain == 0) { + LOG(ERROR) << "range sink write overrun"; + return 0; + } - if (write_all(rss->fd, data, write_now) == -1) { - break; - } + size_t written = 0; + while (size > 0) { + size_t write_now = size; - data += write_now; - size -= write_now; + if (rss->p_remain < write_now) { + write_now = rss->p_remain; + } - rss->p_remain -= write_now; - written += write_now; + if (write_all(rss->fd, data, write_now) == -1) { + break; + } - if (rss->p_remain == 0) { - // move to the next block - ++rss->p_block; - if (rss->p_block < rss->tgt.count) { - rss->p_remain = (rss->tgt.pos[rss->p_block * 2 + 1] - - rss->tgt.pos[rss->p_block * 2]) * BLOCKSIZE; + data += write_now; + size -= write_now; - off64_t offset = static_cast(rss->tgt.pos[rss->p_block*2]) * BLOCKSIZE; - if (!discard_blocks(rss->fd, offset, rss->p_remain)) { - break; - } + rss->p_remain -= write_now; + written += write_now; - if (!check_lseek(rss->fd, offset, SEEK_SET)) { - break; - } + if (rss->p_remain == 0) { + // Move to the next block. + ++rss->p_block; + if (rss->p_block < rss->tgt.count) { + rss->p_remain = + (rss->tgt.pos[rss->p_block * 2 + 1] - rss->tgt.pos[rss->p_block * 2]) * BLOCKSIZE; + + off64_t offset = static_cast(rss->tgt.pos[rss->p_block * 2]) * BLOCKSIZE; + if (!discard_blocks(rss->fd, offset, rss->p_remain)) { + break; + } - } else { - // we can't write any more; return how many bytes have - // been written so far. - break; - } + if (!check_lseek(rss->fd, offset, SEEK_SET)) { + break; } + + } else { + // We can't write any more; return how many bytes have been written so far. + break; + } } + } - return written; + return written; } // All of the data for all the 'new' transfers is contained in one @@ -338,7 +337,7 @@ static bool receive_new_data(const uint8_t* data, size_t size, void* cookie) { // At this point nti->rss is set, and we own it. The main // thread is waiting for it to disappear from nti. - ssize_t written = RangeSinkWrite(data, size, nti->rss); + size_t written = RangeSinkWrite(data, size, nti->rss); data += written; size -= written; -- cgit v1.2.3 From c0e1c46a707370952ea1ddeb71b176d04fe71bb9 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 1 Feb 2017 10:20:10 -0800 Subject: applypatch: Let Apply{BSDiff,Image}Patch accept std::function. Test: mmma bootable/recovery system/update_engine Test: recovery_component_test Change-Id: I93c2caa87bf94a53509bb37f98f2c02bcadb6f5c --- updater/blockimg.cpp | 151 ++++++++++++++++++++++++++------------------------- 1 file changed, 76 insertions(+), 75 deletions(-) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 5a27ff41a..8c0f885a1 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -240,9 +240,7 @@ struct RangeSinkState { size_t p_remain; }; -static size_t RangeSinkWrite(const uint8_t* data, size_t size, void* token) { - RangeSinkState* rss = static_cast(token); - +static size_t RangeSinkWrite(const uint8_t* data, size_t size, RangeSinkState* rss) { if (rss->p_remain == 0) { LOG(ERROR) << "range sink write overrun"; return 0; @@ -1258,92 +1256,95 @@ static int PerformCommandNew(CommandParameters& params) { } static int PerformCommandDiff(CommandParameters& params) { + // + if (params.cpos + 1 >= params.tokens.size()) { + LOG(ERROR) << "missing patch offset or length for " << params.cmdname; + return -1; + } - // - if (params.cpos + 1 >= params.tokens.size()) { - LOG(ERROR) << "missing patch offset or length for " << params.cmdname; - return -1; - } + size_t offset; + if (!android::base::ParseUint(params.tokens[params.cpos++], &offset)) { + LOG(ERROR) << "invalid patch offset"; + return -1; + } - size_t offset; - if (!android::base::ParseUint(params.tokens[params.cpos++], &offset)) { - LOG(ERROR) << "invalid patch offset"; - return -1; - } + size_t len; + if (!android::base::ParseUint(params.tokens[params.cpos++], &len)) { + LOG(ERROR) << "invalid patch len"; + return -1; + } - size_t len; - if (!android::base::ParseUint(params.tokens[params.cpos++], &len)) { - LOG(ERROR) << "invalid patch len"; - return -1; - } + RangeSet tgt; + size_t blocks = 0; + bool overlap = false; + int status = LoadSrcTgtVersion3(params, tgt, &blocks, false, &overlap); - RangeSet tgt; - size_t blocks = 0; - bool overlap = false; - int status = LoadSrcTgtVersion3(params, tgt, &blocks, false, &overlap); + if (status == -1) { + LOG(ERROR) << "failed to read blocks for diff"; + return -1; + } - if (status == -1) { - LOG(ERROR) << "failed to read blocks for diff"; - return -1; - } + if (status == 0) { + params.foundwrites = true; + } else if (params.foundwrites) { + LOG(WARNING) << "warning: commands executed out of order [" << params.cmdname << "]"; + } + if (params.canwrite) { if (status == 0) { - params.foundwrites = true; - } else if (params.foundwrites) { - LOG(WARNING) << "warning: commands executed out of order [" << params.cmdname << "]"; - } - - if (params.canwrite) { - if (status == 0) { - LOG(INFO) << "patching " << blocks << " blocks to " << tgt.size; - Value patch_value(VAL_BLOB, - std::string(reinterpret_cast(params.patch_start + offset), len)); - RangeSinkState rss(tgt); - rss.fd = params.fd; - rss.p_block = 0; - rss.p_remain = (tgt.pos[1] - tgt.pos[0]) * BLOCKSIZE; - - off64_t offset = static_cast(tgt.pos[0]) * BLOCKSIZE; - if (!discard_blocks(params.fd, offset, rss.p_remain)) { - return -1; - } - - if (!check_lseek(params.fd, offset, SEEK_SET)) { - return -1; - } + LOG(INFO) << "patching " << blocks << " blocks to " << tgt.size; + Value patch_value( + VAL_BLOB, std::string(reinterpret_cast(params.patch_start + offset), len)); + RangeSinkState rss(tgt); + rss.fd = params.fd; + rss.p_block = 0; + rss.p_remain = (tgt.pos[1] - tgt.pos[0]) * BLOCKSIZE; + + off64_t offset = static_cast(tgt.pos[0]) * BLOCKSIZE; + if (!discard_blocks(params.fd, offset, rss.p_remain)) { + return -1; + } - if (params.cmdname[0] == 'i') { // imgdiff - if (ApplyImagePatch(params.buffer.data(), blocks * BLOCKSIZE, &patch_value, - &RangeSinkWrite, &rss, nullptr, nullptr) != 0) { - LOG(ERROR) << "Failed to apply image patch."; - return -1; - } - } else { - if (ApplyBSDiffPatch(params.buffer.data(), blocks * BLOCKSIZE, &patch_value, - 0, &RangeSinkWrite, &rss, nullptr) != 0) { - LOG(ERROR) << "Failed to apply bsdiff patch."; - return -1; - } - } + if (!check_lseek(params.fd, offset, SEEK_SET)) { + return -1; + } - // We expect the output of the patcher to fill the tgt ranges exactly. - if (rss.p_block != tgt.count || rss.p_remain != 0) { - LOG(ERROR) << "range sink underrun?"; - } - } else { - LOG(INFO) << "skipping " << blocks << " blocks already patched to " << tgt.size - << " [" << params.cmdline << "]"; + if (params.cmdname[0] == 'i') { // imgdiff + if (ApplyImagePatch( + params.buffer.data(), blocks * BLOCKSIZE, &patch_value, + std::bind(&RangeSinkWrite, std::placeholders::_1, std::placeholders::_2, &rss), + nullptr, nullptr) != 0) { + LOG(ERROR) << "Failed to apply image patch."; + return -1; } - } + } else { + if (ApplyBSDiffPatch( + params.buffer.data(), blocks * BLOCKSIZE, &patch_value, 0, + std::bind(&RangeSinkWrite, std::placeholders::_1, std::placeholders::_2, &rss), + nullptr) != 0) { + LOG(ERROR) << "Failed to apply bsdiff patch."; + return -1; + } + } - if (!params.freestash.empty()) { - FreeStash(params.stashbase, params.freestash); - params.freestash.clear(); + // We expect the output of the patcher to fill the tgt ranges exactly. + if (rss.p_block != tgt.count || rss.p_remain != 0) { + LOG(ERROR) << "range sink underrun?"; + } + } else { + LOG(INFO) << "skipping " << blocks << " blocks already patched to " << tgt.size << " [" + << params.cmdline << "]"; } + } - params.written += tgt.size; + if (!params.freestash.empty()) { + FreeStash(params.stashbase, params.freestash); + params.freestash.clear(); + } - return 0; + params.written += tgt.size; + + return 0; } static int PerformCommandErase(CommandParameters& params) { -- cgit v1.2.3 From 60a70afc0a79048b0fa0813862ee9fb31fd11dd4 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Sun, 26 Mar 2017 14:03:52 -0700 Subject: updater: Move RangeSinkWrite into RangeSinkState. Then rename RangeSinkState to RangeSinkWriter. RangeSinkWriter reads data from the given FD, and writes them to the desination RangeSet. Test: Apply an incremental with the new updater. Change-Id: I5e3ab6fc082efa1726562c55b56e2d418fe4acaf --- updater/blockimg.cpp | 310 ++++++++++++++++++++++++--------------------------- 1 file changed, 146 insertions(+), 164 deletions(-) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 8c0f885a1..e39fac30d 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -231,125 +231,135 @@ static void allocate(size_t size, std::vector& buffer) { buffer.resize(size); } -struct RangeSinkState { - explicit RangeSinkState(RangeSet& rs) : tgt(rs) { }; - - int fd; - const RangeSet& tgt; - size_t p_block; - size_t p_remain; -}; +/** + * RangeSinkWriter reads data from the given FD, and writes them to the destination specified by the + * given RangeSet. + */ +class RangeSinkWriter { + public: + RangeSinkWriter(int fd, const RangeSet& tgt) + : fd_(fd), tgt_(tgt), next_range_(0), current_range_left_(0) { + CHECK_NE(tgt.count, static_cast(0)); + }; -static size_t RangeSinkWrite(const uint8_t* data, size_t size, RangeSinkState* rss) { - if (rss->p_remain == 0) { - LOG(ERROR) << "range sink write overrun"; - return 0; + bool Finished() const { + return next_range_ == tgt_.count && current_range_left_ == 0; } - size_t written = 0; - while (size > 0) { - size_t write_now = size; - - if (rss->p_remain < write_now) { - write_now = rss->p_remain; - } - - if (write_all(rss->fd, data, write_now) == -1) { - break; + size_t Write(const uint8_t* data, size_t size) { + if (Finished()) { + LOG(ERROR) << "range sink write overrun; can't write " << size << " bytes"; + return 0; } - data += write_now; - size -= write_now; - - rss->p_remain -= write_now; - written += write_now; - - if (rss->p_remain == 0) { - // Move to the next block. - ++rss->p_block; - if (rss->p_block < rss->tgt.count) { - rss->p_remain = - (rss->tgt.pos[rss->p_block * 2 + 1] - rss->tgt.pos[rss->p_block * 2]) * BLOCKSIZE; - - off64_t offset = static_cast(rss->tgt.pos[rss->p_block * 2]) * BLOCKSIZE; - if (!discard_blocks(rss->fd, offset, rss->p_remain)) { + size_t written = 0; + while (size > 0) { + // Move to the next range as needed. + if (current_range_left_ == 0) { + if (next_range_ < tgt_.count) { + off64_t offset = static_cast(tgt_.pos[next_range_ * 2]) * BLOCKSIZE; + current_range_left_ = + (tgt_.pos[next_range_ * 2 + 1] - tgt_.pos[next_range_ * 2]) * BLOCKSIZE; + next_range_++; + if (!discard_blocks(fd_, offset, current_range_left_)) { + break; + } + + if (!check_lseek(fd_, offset, SEEK_SET)) { + break; + } + } else { + // We can't write any more; return how many bytes have been written so far. break; } + } - if (!check_lseek(rss->fd, offset, SEEK_SET)) { - break; - } + size_t write_now = size; + if (current_range_left_ < write_now) { + write_now = current_range_left_; + } - } else { - // We can't write any more; return how many bytes have been written so far. + if (write_all(fd_, data, write_now) == -1) { break; } + + data += write_now; + size -= write_now; + + current_range_left_ -= write_now; + written += write_now; } - } - return written; -} + return written; + } -// All of the data for all the 'new' transfers is contained in one -// file in the update package, concatenated together in the order in -// which transfers.list will need it. We want to stream it out of the -// archive (it's compressed) without writing it to a temp file, but we -// can't write each section until it's that transfer's turn to go. -// -// To achieve this, we expand the new data from the archive in a -// background thread, and block that threads 'receive uncompressed -// data' function until the main thread has reached a point where we -// want some new data to be written. We signal the background thread -// with the destination for the data and block the main thread, -// waiting for the background thread to complete writing that section. -// Then it signals the main thread to wake up and goes back to -// blocking waiting for a transfer. -// -// NewThreadInfo is the struct used to pass information back and forth -// between the two threads. When the main thread wants some data -// written, it sets rss to the destination location and signals the -// condition. When the background thread is done writing, it clears -// rss and signals the condition again. + private: + // The input data. + int fd_; + // The destination for the data. + const RangeSet& tgt_; + // The next range that we should write to. + size_t next_range_; + // The number of bytes to write before moving to the next range. + size_t current_range_left_; +}; +/** + * All of the data for all the 'new' transfers is contained in one file in the update package, + * concatenated together in the order in which transfers.list will need it. We want to stream it out + * of the archive (it's compressed) without writing it to a temp file, but we can't write each + * section until it's that transfer's turn to go. + * + * To achieve this, we expand the new data from the archive in a background thread, and block that + * threads 'receive uncompressed data' function until the main thread has reached a point where we + * want some new data to be written. We signal the background thread with the destination for the + * data and block the main thread, waiting for the background thread to complete writing that + * section. Then it signals the main thread to wake up and goes back to blocking waiting for a + * transfer. + * + * NewThreadInfo is the struct used to pass information back and forth between the two threads. When + * the main thread wants some data written, it sets writer to the destination location and signals + * the condition. When the background thread is done writing, it clears writer and signals the + * condition again. + */ struct NewThreadInfo { - ZipArchiveHandle za; - ZipEntry entry; + ZipArchiveHandle za; + ZipEntry entry; - RangeSinkState* rss; + RangeSinkWriter* writer; - pthread_mutex_t mu; - pthread_cond_t cv; + pthread_mutex_t mu; + pthread_cond_t cv; }; static bool receive_new_data(const uint8_t* data, size_t size, void* cookie) { - NewThreadInfo* nti = reinterpret_cast(cookie); + NewThreadInfo* nti = static_cast(cookie); - while (size > 0) { - // Wait for nti->rss to be non-null, indicating some of this - // data is wanted. - pthread_mutex_lock(&nti->mu); - while (nti->rss == nullptr) { - pthread_cond_wait(&nti->cv, &nti->mu); - } - pthread_mutex_unlock(&nti->mu); + while (size > 0) { + // Wait for nti->writer to be non-null, indicating some of this data is wanted. + pthread_mutex_lock(&nti->mu); + while (nti->writer == nullptr) { + pthread_cond_wait(&nti->cv, &nti->mu); + } + pthread_mutex_unlock(&nti->mu); - // At this point nti->rss is set, and we own it. The main - // thread is waiting for it to disappear from nti. - size_t written = RangeSinkWrite(data, size, nti->rss); - data += written; - size -= written; + // At this point nti->writer is set, and we own it. The main thread is waiting for it to + // disappear from nti. + size_t written = nti->writer->Write(data, size); + data += written; + size -= written; - if (nti->rss->p_block == nti->rss->tgt.count) { - // we have written all the bytes desired by this rss. + if (nti->writer->Finished()) { + // We have written all the bytes desired by this writer. - pthread_mutex_lock(&nti->mu); - nti->rss = nullptr; - pthread_cond_broadcast(&nti->cv); - pthread_mutex_unlock(&nti->mu); - } + pthread_mutex_lock(&nti->mu); + nti->writer = nullptr; + pthread_cond_broadcast(&nti->cv); + pthread_mutex_unlock(&nti->mu); } + } - return true; + return true; } static void* unzip_new_data(void* cookie) { @@ -380,28 +390,26 @@ static int ReadBlocks(const RangeSet& src, std::vector& buffer, int fd) } static int WriteBlocks(const RangeSet& tgt, const std::vector& buffer, int fd) { - const uint8_t* data = buffer.data(); - - size_t p = 0; - for (size_t i = 0; i < tgt.count; ++i) { - off64_t offset = static_cast(tgt.pos[i * 2]) * BLOCKSIZE; - size_t size = (tgt.pos[i * 2 + 1] - tgt.pos[i * 2]) * BLOCKSIZE; - if (!discard_blocks(fd, offset, size)) { - return -1; - } - - if (!check_lseek(fd, offset, SEEK_SET)) { - return -1; - } + size_t written = 0; + for (size_t i = 0; i < tgt.count; ++i) { + off64_t offset = static_cast(tgt.pos[i * 2]) * BLOCKSIZE; + size_t size = (tgt.pos[i * 2 + 1] - tgt.pos[i * 2]) * BLOCKSIZE; + if (!discard_blocks(fd, offset, size)) { + return -1; + } - if (write_all(fd, data + p, size) == -1) { - return -1; - } + if (!check_lseek(fd, offset, SEEK_SET)) { + return -1; + } - p += size; + if (write_all(fd, buffer.data() + written, size) == -1) { + return -1; } - return 0; + written += size; + } + + return 0; } // Parameters for transfer list command functions @@ -1214,45 +1222,31 @@ static int PerformCommandZero(CommandParameters& params) { } static int PerformCommandNew(CommandParameters& params) { + if (params.cpos >= params.tokens.size()) { + LOG(ERROR) << "missing target blocks for new"; + return -1; + } - if (params.cpos >= params.tokens.size()) { - LOG(ERROR) << "missing target blocks for new"; - return -1; - } - - RangeSet tgt = parse_range(params.tokens[params.cpos++]); - - if (params.canwrite) { - LOG(INFO) << " writing " << tgt.size << " blocks of new data"; - - RangeSinkState rss(tgt); - rss.fd = params.fd; - rss.p_block = 0; - rss.p_remain = (tgt.pos[1] - tgt.pos[0]) * BLOCKSIZE; - - off64_t offset = static_cast(tgt.pos[0]) * BLOCKSIZE; - if (!discard_blocks(params.fd, offset, tgt.size * BLOCKSIZE)) { - return -1; - } - - if (!check_lseek(params.fd, offset, SEEK_SET)) { - return -1; - } + RangeSet tgt = parse_range(params.tokens[params.cpos++]); - pthread_mutex_lock(¶ms.nti.mu); - params.nti.rss = &rss; - pthread_cond_broadcast(¶ms.nti.cv); + if (params.canwrite) { + LOG(INFO) << " writing " << tgt.size << " blocks of new data"; - while (params.nti.rss) { - pthread_cond_wait(¶ms.nti.cv, ¶ms.nti.mu); - } + RangeSinkWriter writer(params.fd, tgt); + pthread_mutex_lock(¶ms.nti.mu); + params.nti.writer = &writer; + pthread_cond_broadcast(¶ms.nti.cv); - pthread_mutex_unlock(¶ms.nti.mu); + while (params.nti.writer != nullptr) { + pthread_cond_wait(¶ms.nti.cv, ¶ms.nti.mu); } - params.written += tgt.size; + pthread_mutex_unlock(¶ms.nti.mu); + } - return 0; + params.written += tgt.size; + + return 0; } static int PerformCommandDiff(CommandParameters& params) { @@ -1295,40 +1289,28 @@ static int PerformCommandDiff(CommandParameters& params) { LOG(INFO) << "patching " << blocks << " blocks to " << tgt.size; Value patch_value( VAL_BLOB, std::string(reinterpret_cast(params.patch_start + offset), len)); - RangeSinkState rss(tgt); - rss.fd = params.fd; - rss.p_block = 0; - rss.p_remain = (tgt.pos[1] - tgt.pos[0]) * BLOCKSIZE; - - off64_t offset = static_cast(tgt.pos[0]) * BLOCKSIZE; - if (!discard_blocks(params.fd, offset, rss.p_remain)) { - return -1; - } - - if (!check_lseek(params.fd, offset, SEEK_SET)) { - return -1; - } + RangeSinkWriter writer(params.fd, tgt); if (params.cmdname[0] == 'i') { // imgdiff - if (ApplyImagePatch( - params.buffer.data(), blocks * BLOCKSIZE, &patch_value, - std::bind(&RangeSinkWrite, std::placeholders::_1, std::placeholders::_2, &rss), - nullptr, nullptr) != 0) { + if (ApplyImagePatch(params.buffer.data(), blocks * BLOCKSIZE, &patch_value, + std::bind(&RangeSinkWriter::Write, &writer, std::placeholders::_1, + std::placeholders::_2), + nullptr, nullptr) != 0) { LOG(ERROR) << "Failed to apply image patch."; return -1; } } else { - if (ApplyBSDiffPatch( - params.buffer.data(), blocks * BLOCKSIZE, &patch_value, 0, - std::bind(&RangeSinkWrite, std::placeholders::_1, std::placeholders::_2, &rss), - nullptr) != 0) { + if (ApplyBSDiffPatch(params.buffer.data(), blocks * BLOCKSIZE, &patch_value, 0, + std::bind(&RangeSinkWriter::Write, &writer, std::placeholders::_1, + std::placeholders::_2), + nullptr) != 0) { LOG(ERROR) << "Failed to apply bsdiff patch."; return -1; } } // We expect the output of the patcher to fill the tgt ranges exactly. - if (rss.p_block != tgt.count || rss.p_remain != 0) { + if (!writer.Finished()) { LOG(ERROR) << "range sink underrun?"; } } else { -- cgit v1.2.3 From 0bbc764bbe8fc04eba1bf708fbf97526950cbeef Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 29 Mar 2017 23:57:47 -0700 Subject: updater: Don't append newline when calling uiPrint(). LOG(INFO) already appends a newline. Don't print redundant newline. Test: No extra blank lines when calling ui_print(). And on-screen UI shows the same. Change-Id: I74e9a8504a7146a6cb3dae02fe2406d0dd54069b --- updater/blockimg.cpp | 3 ++- updater/install.cpp | 16 ++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 8c0f885a1..4409cbefe 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -1831,7 +1832,7 @@ Value* CheckFirstBlockFn(const char* name, State* state, uint16_t mount_count = *reinterpret_cast(&block0_buffer[0x400+0x34]); if (mount_count > 0) { - uiPrintf(state, "Device was remounted R/W %d times\n", mount_count); + uiPrintf(state, "Device was remounted R/W %" PRIu16 " times", mount_count); uiPrintf(state, "Last remount happened on %s", ctime(&mount_time)); } diff --git a/updater/install.cpp b/updater/install.cpp index f91f3fc9f..857d7f1e0 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -181,8 +181,8 @@ Value* MountFn(const char* name, State* state, const std::vector>& argv) { std::vector args; if (!ReadArgs(state, argv, &args)) { - return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name); + return ErrorAbort(state, kArgsParsingFailure, "%s(): Failed to parse the argument(s)", name); } - std::string buffer = android::base::Join(args, "") + "\n"; + std::string buffer = android::base::Join(args, ""); uiPrint(state, buffer); return StringValue(buffer); } -- cgit v1.2.3 From 8f23757ad42352f57db0b2ab9fc8b6e92eeaedd9 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Sun, 26 Mar 2017 13:36:49 -0700 Subject: Move parse_range() and range_overlaps() into RangeSet. Also move RangeSet into a header file to make it testable, and add unit tests. In RangeSet::Parse() (the former parse_range()), use libbase logging to do assertions. This has the same effect as the previous exit(EXIT_FAILURE) to terminate the updater process and abort an update. The difference lies in the exit status code (i.e. WEXITSTATUS(status) in install.cpp), which changes from 1 (i.e. EXIT_FAILURE) to 0. Test: recovery_unit_test Test: Apply an incremental update with the new updater. Change-Id: Ie8393c78b0d8ae0fd5f0ca0646d871308d71fff0 --- updater/blockimg.cpp | 128 ++++++------------------------------- updater/include/updater/rangeset.h | 95 +++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 110 deletions(-) create mode 100644 updater/include/updater/rangeset.h (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index a1a5773d4..fc7a561eb 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -50,9 +50,10 @@ #include "edify/expr.h" #include "error_code.h" -#include "updater/install.h" #include "ota_io.h" #include "print_sha1.h" +#include "updater/install.h" +#include "updater/rangeset.h" #include "updater/updater.h" // Set this to 0 to interpret 'erase' transfers to mean do a @@ -65,100 +66,10 @@ static constexpr const char* STASH_DIRECTORY_BASE = "/cache/recovery"; static constexpr mode_t STASH_DIRECTORY_MODE = 0700; static constexpr mode_t STASH_FILE_MODE = 0600; -struct RangeSet { - size_t count; // Limit is INT_MAX. - size_t size; - std::vector pos; // Actual limit is INT_MAX. - - // Get the block number for the ith(starting from 0) block in the range set. - int get_block(size_t idx) const { - if (idx >= size) { - LOG(ERROR) << "index: " << idx << " is greater than range set size: " << size; - return -1; - } - for (size_t i = 0; i < pos.size(); i += 2) { - if (idx < pos[i + 1] - pos[i]) { - return pos[i] + idx; - } - idx -= (pos[i + 1] - pos[i]); - } - return -1; - } -}; - static CauseCode failure_type = kNoCause; static bool is_retry = false; static std::unordered_map stash_map; -static RangeSet parse_range(const std::string& range_text) { - RangeSet rs; - - std::vector pieces = android::base::Split(range_text, ","); - if (pieces.size() < 3) { - goto err; - } - - size_t num; - if (!android::base::ParseUint(pieces[0], &num, static_cast(INT_MAX))) { - goto err; - } - - if (num == 0 || num % 2) { - goto err; // must be even - } else if (num != pieces.size() - 1) { - goto err; - } - - rs.pos.resize(num); - rs.count = num / 2; - rs.size = 0; - - for (size_t i = 0; i < num; i += 2) { - if (!android::base::ParseUint(pieces[i + 1], &rs.pos[i], static_cast(INT_MAX))) { - goto err; - } - - if (!android::base::ParseUint(pieces[i + 2], &rs.pos[i + 1], static_cast(INT_MAX))) { - goto err; - } - - if (rs.pos[i] >= rs.pos[i + 1]) { - goto err; // empty or negative range - } - - size_t sz = rs.pos[i + 1] - rs.pos[i]; - if (rs.size > SIZE_MAX - sz) { - goto err; // overflow - } - - rs.size += sz; - } - - return rs; - -err: - LOG(ERROR) << "failed to parse range '" << range_text << "'"; - exit(EXIT_FAILURE); -} - -static bool range_overlaps(const RangeSet& r1, const RangeSet& r2) { - for (size_t i = 0; i < r1.count; ++i) { - size_t r1_0 = r1.pos[i * 2]; - size_t r1_1 = r1.pos[i * 2 + 1]; - - for (size_t j = 0; j < r2.count; ++j) { - size_t r2_0 = r2.pos[j * 2]; - size_t r2_1 = r2.pos[j * 2 + 1]; - - if (!(r2_0 >= r1_1 || r1_0 >= r2_1)) { - return true; - } - } - } - - return false; -} - static int read_all(int fd, uint8_t* data, size_t size) { size_t so_far = 0; while (so_far < size) { @@ -469,7 +380,7 @@ static void PrintHashForCorruptedSourceBlocks(const CommandParameters& params, return; } - RangeSet src = parse_range(params.tokens[pos++]); + RangeSet src = RangeSet::Parse(params.tokens[pos++]); RangeSet locs; // If there's no stashed blocks, content in the buffer is consecutive and has the same @@ -483,17 +394,15 @@ static void PrintHashForCorruptedSourceBlocks(const CommandParameters& params, // Example: for the tokens <4,63946,63947,63948,63979> <4,6,7,8,39> ; // We want to print SHA-1 for the data in buffer[6], buffer[8], buffer[9] ... buffer[38]; // this corresponds to the 32 src blocks #63946, #63948, #63949 ... #63978. - locs = parse_range(params.tokens[pos++]); + locs = RangeSet::Parse(params.tokens[pos++]); CHECK_EQ(src.size, locs.size); CHECK_EQ(locs.pos.size() % 2, static_cast(0)); } LOG(INFO) << "printing hash in hex for " << src.size << " source blocks"; for (size_t i = 0; i < src.size; i++) { - int block_num = src.get_block(i); - CHECK_NE(block_num, -1); - int buffer_index = locs.get_block(i); - CHECK_NE(buffer_index, -1); + size_t block_num = src.GetBlockNumber(i); + size_t buffer_index = locs.GetBlockNumber(i); CHECK_LE((buffer_index + 1) * BLOCKSIZE, buffer.size()); uint8_t digest[SHA_DIGEST_LENGTH]; @@ -512,8 +421,7 @@ static void PrintHashForCorruptedStashedBlocks(const std::string& id, CHECK_EQ(src.size * BLOCKSIZE, buffer.size()); for (size_t i = 0; i < src.size; i++) { - int block_num = src.get_block(i); - CHECK_NE(block_num, -1); + size_t block_num = src.GetBlockNumber(i); uint8_t digest[SHA_DIGEST_LENGTH]; SHA1(buffer.data() + i * BLOCKSIZE, BLOCKSIZE, digest); @@ -925,8 +833,8 @@ static int LoadSourceBlocks(CommandParameters& params, const RangeSet& tgt, size // no source ranges, only stashes params.cpos++; } else { - RangeSet src = parse_range(params.tokens[params.cpos++]); - *overlap = range_overlaps(src, tgt); + RangeSet src = RangeSet::Parse(params.tokens[params.cpos++]); + *overlap = src.Overlaps(tgt); if (ReadBlocks(src, params.buffer, params.fd) == -1) { return -1; @@ -937,7 +845,7 @@ static int LoadSourceBlocks(CommandParameters& params, const RangeSet& tgt, size return 0; } - RangeSet locs = parse_range(params.tokens[params.cpos++]); + RangeSet locs = RangeSet::Parse(params.tokens[params.cpos++]); MoveRange(params.buffer, locs, params.buffer); } @@ -959,7 +867,7 @@ static int LoadSourceBlocks(CommandParameters& params, const RangeSet& tgt, size continue; } - RangeSet locs = parse_range(tokens[1]); + RangeSet locs = RangeSet::Parse(tokens[1]); MoveRange(params.buffer, locs, stash); } @@ -1023,7 +931,7 @@ static int LoadSrcTgtVersion3(CommandParameters& params, RangeSet& tgt, size_t* } // - tgt = parse_range(params.tokens[params.cpos++]); + tgt = RangeSet::Parse(params.tokens[params.cpos++]); std::vector tgtbuffer(tgt.size * BLOCKSIZE); if (ReadBlocks(tgt, tgtbuffer, params.fd) == -1) { @@ -1135,7 +1043,7 @@ static int PerformCommandStash(CommandParameters& params) { return 0; } - RangeSet src = parse_range(params.tokens[params.cpos++]); + RangeSet src = RangeSet::Parse(params.tokens[params.cpos++]); allocate(src.size * BLOCKSIZE, params.buffer); if (ReadBlocks(src, params.buffer, params.fd) == -1) { @@ -1186,7 +1094,7 @@ static int PerformCommandZero(CommandParameters& params) { return -1; } - RangeSet tgt = parse_range(params.tokens[params.cpos++]); + RangeSet tgt = RangeSet::Parse(params.tokens[params.cpos++]); LOG(INFO) << " zeroing " << tgt.size << " blocks"; @@ -1228,7 +1136,7 @@ static int PerformCommandNew(CommandParameters& params) { return -1; } - RangeSet tgt = parse_range(params.tokens[params.cpos++]); + RangeSet tgt = RangeSet::Parse(params.tokens[params.cpos++]); if (params.canwrite) { LOG(INFO) << " writing " << tgt.size << " blocks of new data"; @@ -1351,7 +1259,7 @@ static int PerformCommandErase(CommandParameters& params) { return -1; } - RangeSet tgt = parse_range(params.tokens[params.cpos++]); + RangeSet tgt = RangeSet::Parse(params.tokens[params.cpos++]); if (params.canwrite) { LOG(INFO) << " erasing " << tgt.size << " blocks"; @@ -1733,7 +1641,7 @@ Value* RangeSha1Fn(const char* name, State* state, const std::vectordata); + RangeSet rs = RangeSet::Parse(ranges->data); SHA_CTX ctx; SHA1_Init(&ctx); @@ -1871,7 +1779,7 @@ Value* BlockImageRecoverFn(const char* name, State* state, return StringValue(""); } - RangeSet rs = parse_range(ranges->data); + RangeSet rs = RangeSet::Parse(ranges->data); uint8_t buffer[BLOCKSIZE]; diff --git a/updater/include/updater/rangeset.h b/updater/include/updater/rangeset.h new file mode 100644 index 000000000..afaa82dcd --- /dev/null +++ b/updater/include/updater/rangeset.h @@ -0,0 +1,95 @@ +/* + * 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. + */ + +#pragma once + +#include + +#include +#include + +#include +#include +#include + +struct RangeSet { + size_t count; // Limit is INT_MAX. + size_t size; // The number of blocks in the RangeSet. + std::vector pos; // Actual limit is INT_MAX. + + static RangeSet Parse(const std::string& range_text) { + std::vector pieces = android::base::Split(range_text, ","); + CHECK_GE(pieces.size(), static_cast(3)) << "Invalid range text: " << range_text; + + size_t num; + CHECK(android::base::ParseUint(pieces[0], &num, static_cast(INT_MAX))) + << "Failed to parse the number of tokens: " << range_text; + + CHECK_NE(num, static_cast(0)) << "Invalid number of tokens: " << range_text; + CHECK_EQ(num % 2, static_cast(0)) << "Number of tokens must be even: " << range_text; + CHECK_EQ(num, pieces.size() - 1) << "Mismatching number of tokens: " << range_text; + + std::vector pairs(num); + size_t size = 0; + for (size_t i = 0; i < num; i += 2) { + CHECK(android::base::ParseUint(pieces[i + 1], &pairs[i], static_cast(INT_MAX))); + CHECK(android::base::ParseUint(pieces[i + 2], &pairs[i + 1], static_cast(INT_MAX))); + CHECK_LT(pairs[i], pairs[i + 1]) + << "Empty or negative range: " << pairs[i] << ", " << pairs[i + 1]; + + size_t sz = pairs[i + 1] - pairs[i]; + CHECK_LE(size, SIZE_MAX - sz) << "RangeSet size overflow"; + size += sz; + } + + return RangeSet{ num / 2, size, std::move(pairs) }; + } + + // Get the block number for the i-th (starting from 0) block in the RangeSet. + size_t GetBlockNumber(size_t idx) const { + CHECK_LT(idx, size) << "Index " << idx << " is greater than RangeSet size " << size; + for (size_t i = 0; i < pos.size(); i += 2) { + if (idx < pos[i + 1] - pos[i]) { + return pos[i] + idx; + } + idx -= (pos[i + 1] - pos[i]); + } + CHECK(false); + return 0; // Unreachable, but to make compiler happy. + } + + // RangeSet has half-closed half-open bounds. For example, "3,5" contains blocks 3 and 4. So "3,5" + // and "5,7" are not overlapped. + bool Overlaps(const RangeSet& other) const { + for (size_t i = 0; i < count; ++i) { + size_t start = pos[i * 2]; + size_t end = pos[i * 2 + 1]; + for (size_t j = 0; j < other.count; ++j) { + size_t other_start = other.pos[j * 2]; + size_t other_end = other.pos[j * 2 + 1]; + // [start, end) vs [other_start, other_end) + if (!(other_start >= end || start >= other_end)) { + return true; + } + } + } + return false; + } + + bool operator==(const RangeSet& other) const { + return (count == other.count && size == other.size && pos == other.pos); + } +}; -- cgit v1.2.3 From c97edcb4f49d6ea573c4d3b5c3b8bd2610fa8f7c Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 31 Mar 2017 01:18:13 -0700 Subject: updater: Keep the parsed parameters in std::unique_ptr. We don't need to take raw pointers out of the parsed arguments. std::unique_ptr handles the dereferencing automatically. Test: mmma bootable/recovery Change-Id: I1beabf6e04dc350bdad7b36cee5fb345c82b28f2 --- updater/blockimg.cpp | 317 +++++++++++++++++++++++++-------------------------- 1 file changed, 157 insertions(+), 160 deletions(-) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index fc7a561eb..8199447a9 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -1317,10 +1317,10 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, return nullptr; } - const Value* blockdev_filename = args[0].get(); - const Value* transfer_list_value = args[1].get(); - const Value* new_data_fn = args[2].get(); - const Value* patch_data_fn = args[3].get(); + const std::unique_ptr& blockdev_filename = args[0]; + const std::unique_ptr& transfer_list_value = args[1]; + const std::unique_ptr& new_data_fn = args[2]; + const std::unique_ptr& patch_data_fn = args[3]; if (blockdev_filename->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "blockdev_filename argument to %s must be string", name); @@ -1610,64 +1610,62 @@ Value* BlockImageUpdateFn(const char* name, State* state, } Value* RangeSha1Fn(const char* name, State* state, const std::vector>& argv) { - if (argv.size() != 2) { - ErrorAbort(state, kArgsParsingFailure, "range_sha1 expects 2 arguments, got %zu", - argv.size()); - return StringValue(""); - } + if (argv.size() != 2) { + ErrorAbort(state, kArgsParsingFailure, "range_sha1 expects 2 arguments, got %zu", argv.size()); + return StringValue(""); + } - std::vector> args; - if (!ReadValueArgs(state, argv, &args)) { - return nullptr; - } + std::vector> args; + if (!ReadValueArgs(state, argv, &args)) { + return nullptr; + } - const Value* blockdev_filename = args[0].get(); - const Value* ranges = args[1].get(); + const std::unique_ptr& blockdev_filename = args[0]; + const std::unique_ptr& ranges = args[1]; - if (blockdev_filename->type != VAL_STRING) { - ErrorAbort(state, kArgsParsingFailure, "blockdev_filename argument to %s must be string", - name); - return StringValue(""); - } - if (ranges->type != VAL_STRING) { - ErrorAbort(state, kArgsParsingFailure, "ranges argument to %s must be string", name); - return StringValue(""); - } + if (blockdev_filename->type != VAL_STRING) { + ErrorAbort(state, kArgsParsingFailure, "blockdev_filename argument to %s must be string", name); + return StringValue(""); + } + if (ranges->type != VAL_STRING) { + ErrorAbort(state, kArgsParsingFailure, "ranges argument to %s must be string", name); + return StringValue(""); + } - android::base::unique_fd fd(ota_open(blockdev_filename->data.c_str(), O_RDWR)); - if (fd == -1) { - ErrorAbort(state, kFileOpenFailure, "open \"%s\" failed: %s", - blockdev_filename->data.c_str(), strerror(errno)); - return StringValue(""); - } + android::base::unique_fd fd(ota_open(blockdev_filename->data.c_str(), O_RDWR)); + if (fd == -1) { + ErrorAbort(state, kFileOpenFailure, "open \"%s\" failed: %s", blockdev_filename->data.c_str(), + strerror(errno)); + return StringValue(""); + } - RangeSet rs = RangeSet::Parse(ranges->data); + RangeSet rs = RangeSet::Parse(ranges->data); - SHA_CTX ctx; - SHA1_Init(&ctx); + SHA_CTX ctx; + SHA1_Init(&ctx); - std::vector buffer(BLOCKSIZE); - for (size_t i = 0; i < rs.count; ++i) { - if (!check_lseek(fd, (off64_t)rs.pos[i*2] * BLOCKSIZE, SEEK_SET)) { - ErrorAbort(state, kLseekFailure, "failed to seek %s: %s", - blockdev_filename->data.c_str(), strerror(errno)); - return StringValue(""); - } + std::vector buffer(BLOCKSIZE); + for (size_t i = 0; i < rs.count; ++i) { + if (!check_lseek(fd, (off64_t)rs.pos[i * 2] * BLOCKSIZE, SEEK_SET)) { + ErrorAbort(state, kLseekFailure, "failed to seek %s: %s", blockdev_filename->data.c_str(), + strerror(errno)); + return StringValue(""); + } - for (size_t j = rs.pos[i*2]; j < rs.pos[i*2+1]; ++j) { - if (read_all(fd, buffer, BLOCKSIZE) == -1) { - ErrorAbort(state, kFreadFailure, "failed to read %s: %s", - blockdev_filename->data.c_str(), strerror(errno)); - return StringValue(""); - } + for (size_t j = rs.pos[i * 2]; j < rs.pos[i * 2 + 1]; ++j) { + if (read_all(fd, buffer, BLOCKSIZE) == -1) { + ErrorAbort(state, kFreadFailure, "failed to read %s: %s", blockdev_filename->data.c_str(), + strerror(errno)); + return StringValue(""); + } - SHA1_Update(&ctx, buffer.data(), BLOCKSIZE); - } + SHA1_Update(&ctx, buffer.data(), BLOCKSIZE); } - uint8_t digest[SHA_DIGEST_LENGTH]; - SHA1_Final(digest, &ctx); + } + uint8_t digest[SHA_DIGEST_LENGTH]; + SHA1_Final(digest, &ctx); - return StringValue(print_sha1(digest)); + return StringValue(print_sha1(digest)); } // This function checks if a device has been remounted R/W prior to an incremental @@ -1677,145 +1675,144 @@ Value* RangeSha1Fn(const char* name, State* state, const std::vector>& argv) { - if (argv.size() != 1) { - ErrorAbort(state, kArgsParsingFailure, "check_first_block expects 1 argument, got %zu", - argv.size()); - return StringValue(""); - } + if (argv.size() != 1) { + ErrorAbort(state, kArgsParsingFailure, "check_first_block expects 1 argument, got %zu", + argv.size()); + return StringValue(""); + } - std::vector> args; - if (!ReadValueArgs(state, argv, &args)) { - return nullptr; - } + std::vector> args; + if (!ReadValueArgs(state, argv, &args)) { + return nullptr; + } - const Value* arg_filename = args[0].get(); + const std::unique_ptr& arg_filename = args[0]; - if (arg_filename->type != VAL_STRING) { - ErrorAbort(state, kArgsParsingFailure, "filename argument to %s must be string", name); - return StringValue(""); - } + if (arg_filename->type != VAL_STRING) { + ErrorAbort(state, kArgsParsingFailure, "filename argument to %s must be string", name); + return StringValue(""); + } - android::base::unique_fd fd(ota_open(arg_filename->data.c_str(), O_RDONLY)); - if (fd == -1) { - ErrorAbort(state, kFileOpenFailure, "open \"%s\" failed: %s", arg_filename->data.c_str(), - strerror(errno)); - return StringValue(""); - } + android::base::unique_fd fd(ota_open(arg_filename->data.c_str(), O_RDONLY)); + if (fd == -1) { + ErrorAbort(state, kFileOpenFailure, "open \"%s\" failed: %s", arg_filename->data.c_str(), + strerror(errno)); + return StringValue(""); + } - RangeSet blk0 {1 /*count*/, 1/*size*/, std::vector {0, 1}/*position*/}; - std::vector block0_buffer(BLOCKSIZE); + RangeSet blk0{ 1 /*count*/, 1 /*size*/, std::vector{ 0, 1 } /*position*/ }; + std::vector block0_buffer(BLOCKSIZE); - if (ReadBlocks(blk0, block0_buffer, fd) == -1) { - ErrorAbort(state, kFreadFailure, "failed to read %s: %s", arg_filename->data.c_str(), - strerror(errno)); - return StringValue(""); - } + if (ReadBlocks(blk0, block0_buffer, fd) == -1) { + ErrorAbort(state, kFreadFailure, "failed to read %s: %s", arg_filename->data.c_str(), + strerror(errno)); + return StringValue(""); + } - // https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout - // Super block starts from block 0, offset 0x400 - // 0x2C: len32 Mount time - // 0x30: len32 Write time - // 0x34: len16 Number of mounts since the last fsck - // 0x38: len16 Magic signature 0xEF53 + // https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout + // Super block starts from block 0, offset 0x400 + // 0x2C: len32 Mount time + // 0x30: len32 Write time + // 0x34: len16 Number of mounts since the last fsck + // 0x38: len16 Magic signature 0xEF53 - time_t mount_time = *reinterpret_cast(&block0_buffer[0x400+0x2C]); - uint16_t mount_count = *reinterpret_cast(&block0_buffer[0x400+0x34]); + time_t mount_time = *reinterpret_cast(&block0_buffer[0x400 + 0x2C]); + uint16_t mount_count = *reinterpret_cast(&block0_buffer[0x400 + 0x34]); - if (mount_count > 0) { - uiPrintf(state, "Device was remounted R/W %" PRIu16 " times", mount_count); - uiPrintf(state, "Last remount happened on %s", ctime(&mount_time)); - } + if (mount_count > 0) { + uiPrintf(state, "Device was remounted R/W %" PRIu16 " times", mount_count); + uiPrintf(state, "Last remount happened on %s", ctime(&mount_time)); + } - return StringValue("t"); + return StringValue("t"); } - Value* BlockImageRecoverFn(const char* name, State* state, const std::vector>& argv) { - if (argv.size() != 2) { - ErrorAbort(state, kArgsParsingFailure, "block_image_recover expects 2 arguments, got %zu", - argv.size()); - return StringValue(""); - } + if (argv.size() != 2) { + ErrorAbort(state, kArgsParsingFailure, "block_image_recover expects 2 arguments, got %zu", + argv.size()); + return StringValue(""); + } - std::vector> args; - if (!ReadValueArgs(state, argv, &args)) { - return nullptr; - } + std::vector> args; + if (!ReadValueArgs(state, argv, &args)) { + return nullptr; + } - const Value* filename = args[0].get(); - const Value* ranges = args[1].get(); + const std::unique_ptr& filename = args[0]; + const std::unique_ptr& ranges = args[1]; - if (filename->type != VAL_STRING) { - ErrorAbort(state, kArgsParsingFailure, "filename argument to %s must be string", name); - return StringValue(""); - } - if (ranges->type != VAL_STRING) { - ErrorAbort(state, kArgsParsingFailure, "ranges argument to %s must be string", name); - return StringValue(""); - } + if (filename->type != VAL_STRING) { + ErrorAbort(state, kArgsParsingFailure, "filename argument to %s must be string", name); + return StringValue(""); + } + if (ranges->type != VAL_STRING) { + ErrorAbort(state, kArgsParsingFailure, "ranges argument to %s must be string", name); + return StringValue(""); + } - // Output notice to log when recover is attempted - LOG(INFO) << filename->data << " image corrupted, attempting to recover..."; + // Output notice to log when recover is attempted + LOG(INFO) << filename->data << " image corrupted, attempting to recover..."; - // When opened with O_RDWR, libfec rewrites corrupted blocks when they are read - fec::io fh(filename->data, O_RDWR); + // When opened with O_RDWR, libfec rewrites corrupted blocks when they are read + fec::io fh(filename->data, O_RDWR); - if (!fh) { - ErrorAbort(state, kLibfecFailure, "fec_open \"%s\" failed: %s", filename->data.c_str(), - strerror(errno)); - return StringValue(""); - } + if (!fh) { + ErrorAbort(state, kLibfecFailure, "fec_open \"%s\" failed: %s", filename->data.c_str(), + strerror(errno)); + return StringValue(""); + } - if (!fh.has_ecc() || !fh.has_verity()) { - ErrorAbort(state, kLibfecFailure, "unable to use metadata to correct errors"); - return StringValue(""); - } + if (!fh.has_ecc() || !fh.has_verity()) { + ErrorAbort(state, kLibfecFailure, "unable to use metadata to correct errors"); + return StringValue(""); + } - fec_status status; + fec_status status; - if (!fh.get_status(status)) { - ErrorAbort(state, kLibfecFailure, "failed to read FEC status"); - return StringValue(""); - } + if (!fh.get_status(status)) { + ErrorAbort(state, kLibfecFailure, "failed to read FEC status"); + return StringValue(""); + } - RangeSet rs = RangeSet::Parse(ranges->data); + RangeSet rs = RangeSet::Parse(ranges->data); - uint8_t buffer[BLOCKSIZE]; + uint8_t buffer[BLOCKSIZE]; - for (size_t i = 0; i < rs.count; ++i) { - for (size_t j = rs.pos[i * 2]; j < rs.pos[i * 2 + 1]; ++j) { - // Stay within the data area, libfec validates and corrects metadata - if (status.data_size <= (uint64_t)j * BLOCKSIZE) { - continue; - } + for (size_t i = 0; i < rs.count; ++i) { + for (size_t j = rs.pos[i * 2]; j < rs.pos[i * 2 + 1]; ++j) { + // Stay within the data area, libfec validates and corrects metadata + if (status.data_size <= (uint64_t)j * BLOCKSIZE) { + continue; + } - if (fh.pread(buffer, BLOCKSIZE, (off64_t)j * BLOCKSIZE) != BLOCKSIZE) { - ErrorAbort(state, kLibfecFailure, "failed to recover %s (block %zu): %s", - filename->data.c_str(), j, strerror(errno)); - return StringValue(""); - } + if (fh.pread(buffer, BLOCKSIZE, (off64_t)j * BLOCKSIZE) != BLOCKSIZE) { + ErrorAbort(state, kLibfecFailure, "failed to recover %s (block %zu): %s", + filename->data.c_str(), j, strerror(errno)); + return StringValue(""); + } - // If we want to be able to recover from a situation where rewriting a corrected - // block doesn't guarantee the same data will be returned when re-read later, we - // can save a copy of corrected blocks to /cache. Note: - // - // 1. Maximum space required from /cache is the same as the maximum number of - // corrupted blocks we can correct. For RS(255, 253) and a 2 GiB partition, - // this would be ~16 MiB, for example. - // - // 2. To find out if this block was corrupted, call fec_get_status after each - // read and check if the errors field value has increased. - } + // If we want to be able to recover from a situation where rewriting a corrected + // block doesn't guarantee the same data will be returned when re-read later, we + // can save a copy of corrected blocks to /cache. Note: + // + // 1. Maximum space required from /cache is the same as the maximum number of + // corrupted blocks we can correct. For RS(255, 253) and a 2 GiB partition, + // this would be ~16 MiB, for example. + // + // 2. To find out if this block was corrupted, call fec_get_status after each + // read and check if the errors field value has increased. } - LOG(INFO) << "..." << filename->data << " image recovered successfully."; - return StringValue("t"); + } + LOG(INFO) << "..." << filename->data << " image recovered successfully."; + return StringValue("t"); } void RegisterBlockImageFunctions() { - RegisterFunction("block_image_verify", BlockImageVerifyFn); - RegisterFunction("block_image_update", BlockImageUpdateFn); - RegisterFunction("block_image_recover", BlockImageRecoverFn); - RegisterFunction("check_first_block", CheckFirstBlockFn); - RegisterFunction("range_sha1", RangeSha1Fn); + RegisterFunction("block_image_verify", BlockImageVerifyFn); + RegisterFunction("block_image_update", BlockImageUpdateFn); + RegisterFunction("block_image_recover", BlockImageRecoverFn); + RegisterFunction("check_first_block", CheckFirstBlockFn); + RegisterFunction("range_sha1", RangeSha1Fn); } -- cgit v1.2.3 From bf5b77dbf73eef715bb49b43e113833efbcb2994 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 30 Mar 2017 16:57:29 -0700 Subject: Change the internal representation in RangeSet. This CL makes the following changes to RangeSet: - Uses std::pair to represent a Range; - Uses std::vector to represent a RangeSet; - Provides const iterators (forward and reverse); - Provides const accessor; - 'blocks()' returns the number of blocks (formerly 'size'); - 'size()' returns the number of Range's (formerly 'count'). Test: recovery_unit_test Test: Apply an incremental update with the new updater. Change-Id: Ia1fbb343370a152e1f7aa050cf914c2da09b1396 --- updater/blockimg.cpp | 392 ++++++++++++++++++------------------- updater/include/updater/rangeset.h | 125 +++++++++--- 2 files changed, 284 insertions(+), 233 deletions(-) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 8199447a9..0f08d17eb 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -112,18 +112,17 @@ static int write_all(int fd, const std::vector& buffer, size_t size) { } static bool discard_blocks(int fd, off64_t offset, uint64_t size) { - // Don't discard blocks unless the update is a retry run. - if (!is_retry) { - return true; - } - - uint64_t args[2] = {static_cast(offset), size}; - int status = ioctl(fd, BLKDISCARD, &args); - if (status == -1) { - PLOG(ERROR) << "BLKDISCARD ioctl failed"; - return false; - } + // Don't discard blocks unless the update is a retry run. + if (!is_retry) { return true; + } + + uint64_t args[2] = { static_cast(offset), size }; + if (ioctl(fd, BLKDISCARD, &args) == -1) { + PLOG(ERROR) << "BLKDISCARD ioctl failed"; + return false; + } + return true; } static bool check_lseek(int fd, off64_t offset, int whence) { @@ -151,11 +150,11 @@ class RangeSinkWriter { public: RangeSinkWriter(int fd, const RangeSet& tgt) : fd_(fd), tgt_(tgt), next_range_(0), current_range_left_(0) { - CHECK_NE(tgt.count, static_cast(0)); + CHECK_NE(tgt.size(), static_cast(0)); }; bool Finished() const { - return next_range_ == tgt_.count && current_range_left_ == 0; + return next_range_ == tgt_.size() && current_range_left_ == 0; } size_t Write(const uint8_t* data, size_t size) { @@ -168,10 +167,10 @@ class RangeSinkWriter { while (size > 0) { // Move to the next range as needed. if (current_range_left_ == 0) { - if (next_range_ < tgt_.count) { - off64_t offset = static_cast(tgt_.pos[next_range_ * 2]) * BLOCKSIZE; - current_range_left_ = - (tgt_.pos[next_range_ * 2 + 1] - tgt_.pos[next_range_ * 2]) * BLOCKSIZE; + if (next_range_ < tgt_.size()) { + const Range& range = tgt_[next_range_]; + off64_t offset = static_cast(range.first) * BLOCKSIZE; + current_range_left_ = (range.second - range.first) * BLOCKSIZE; next_range_++; if (!discard_blocks(fd_, offset, current_range_left_)) { break; @@ -281,31 +280,28 @@ static void* unzip_new_data(void* cookie) { } static int ReadBlocks(const RangeSet& src, std::vector& buffer, int fd) { - size_t p = 0; - uint8_t* data = buffer.data(); - - for (size_t i = 0; i < src.count; ++i) { - if (!check_lseek(fd, (off64_t) src.pos[i * 2] * BLOCKSIZE, SEEK_SET)) { - return -1; - } - - size_t size = (src.pos[i * 2 + 1] - src.pos[i * 2]) * BLOCKSIZE; - - if (read_all(fd, data + p, size) == -1) { - return -1; - } + size_t p = 0; + for (const auto& range : src) { + if (!check_lseek(fd, static_cast(range.first) * BLOCKSIZE, SEEK_SET)) { + return -1; + } - p += size; + size_t size = (range.second - range.first) * BLOCKSIZE; + if (read_all(fd, buffer.data() + p, size) == -1) { + return -1; } - return 0; + p += size; + } + + return 0; } static int WriteBlocks(const RangeSet& tgt, const std::vector& buffer, int fd) { size_t written = 0; - for (size_t i = 0; i < tgt.count; ++i) { - off64_t offset = static_cast(tgt.pos[i * 2]) * BLOCKSIZE; - size_t size = (tgt.pos[i * 2 + 1] - tgt.pos[i * 2]) * BLOCKSIZE; + for (const auto& range : tgt) { + off64_t offset = static_cast(range.first) * BLOCKSIZE; + size_t size = (range.second - range.first) * BLOCKSIZE; if (!discard_blocks(fd, offset, size)) { return -1; } @@ -386,21 +382,18 @@ static void PrintHashForCorruptedSourceBlocks(const CommandParameters& params, // If there's no stashed blocks, content in the buffer is consecutive and has the same // order as the source blocks. if (pos == params.tokens.size()) { - locs.count = 1; - locs.size = src.size; - locs.pos = { 0, src.size }; + locs = RangeSet(std::vector{ Range{ 0, src.blocks() } }); } else { // Otherwise, the next token is the offset of the source blocks in the target range. // Example: for the tokens <4,63946,63947,63948,63979> <4,6,7,8,39> ; // We want to print SHA-1 for the data in buffer[6], buffer[8], buffer[9] ... buffer[38]; // this corresponds to the 32 src blocks #63946, #63948, #63949 ... #63978. locs = RangeSet::Parse(params.tokens[pos++]); - CHECK_EQ(src.size, locs.size); - CHECK_EQ(locs.pos.size() % 2, static_cast(0)); + CHECK_EQ(src.blocks(), locs.blocks()); } - LOG(INFO) << "printing hash in hex for " << src.size << " source blocks"; - for (size_t i = 0; i < src.size; i++) { + LOG(INFO) << "printing hash in hex for " << src.blocks() << " source blocks"; + for (size_t i = 0; i < src.blocks(); i++) { size_t block_num = src.GetBlockNumber(i); size_t buffer_index = locs.GetBlockNumber(i); CHECK_LE((buffer_index + 1) * BLOCKSIZE, buffer.size()); @@ -418,9 +411,9 @@ static void PrintHashForCorruptedStashedBlocks(const std::string& id, const std::vector& buffer, const RangeSet& src) { LOG(INFO) << "printing hash in hex for stash_id: " << id; - CHECK_EQ(src.size * BLOCKSIZE, buffer.size()); + CHECK_EQ(src.blocks() * BLOCKSIZE, buffer.size()); - for (size_t i = 0; i < src.size; i++) { + for (size_t i = 0; i < src.blocks(); i++) { size_t block_num = src.GetBlockNumber(i); uint8_t digest[SHA_DIGEST_LENGTH]; @@ -440,7 +433,7 @@ static void PrintHashForMissingStashedBlocks(const std::string& id, int fd) { LOG(INFO) << "print hash in hex for source blocks in missing stash: " << id; const RangeSet& src = stash_map[id]; - std::vector buffer(src.size * BLOCKSIZE); + std::vector buffer(src.blocks() * BLOCKSIZE); if (ReadBlocks(src, buffer, fd) == -1) { LOG(ERROR) << "failed to read source blocks for stash: " << id; return; @@ -532,81 +525,77 @@ static void DeleteStash(const std::string& base) { static int LoadStash(CommandParameters& params, const std::string& id, bool verify, size_t* blocks, std::vector& buffer, bool printnoent) { - // In verify mode, if source range_set was saved for the given hash, - // check contents in the source blocks first. If the check fails, - // search for the stashed files on /cache as usual. - if (!params.canwrite) { - if (stash_map.find(id) != stash_map.end()) { - const RangeSet& src = stash_map[id]; - allocate(src.size * BLOCKSIZE, buffer); - - if (ReadBlocks(src, buffer, params.fd) == -1) { - LOG(ERROR) << "failed to read source blocks in stash map."; - return -1; - } - if (VerifyBlocks(id, buffer, src.size, true) != 0) { - LOG(ERROR) << "failed to verify loaded source blocks in stash map."; - PrintHashForCorruptedStashedBlocks(id, buffer, src); - return -1; - } - return 0; - } - } - - size_t blockcount = 0; + // In verify mode, if source range_set was saved for the given hash, check contents in the source + // blocks first. If the check fails, search for the stashed files on /cache as usual. + if (!params.canwrite) { + if (stash_map.find(id) != stash_map.end()) { + const RangeSet& src = stash_map[id]; + allocate(src.blocks() * BLOCKSIZE, buffer); - if (!blocks) { - blocks = &blockcount; + if (ReadBlocks(src, buffer, params.fd) == -1) { + LOG(ERROR) << "failed to read source blocks in stash map."; + return -1; + } + if (VerifyBlocks(id, buffer, src.blocks(), true) != 0) { + LOG(ERROR) << "failed to verify loaded source blocks in stash map."; + PrintHashForCorruptedStashedBlocks(id, buffer, src); + return -1; + } + return 0; } + } - std::string fn = GetStashFileName(params.stashbase, id, ""); + size_t blockcount = 0; + if (!blocks) { + blocks = &blockcount; + } - struct stat sb; - int res = stat(fn.c_str(), &sb); + std::string fn = GetStashFileName(params.stashbase, id, ""); - if (res == -1) { - if (errno != ENOENT || printnoent) { - PLOG(ERROR) << "stat \"" << fn << "\" failed"; - PrintHashForMissingStashedBlocks(id, params.fd); - } - return -1; + struct stat sb; + if (stat(fn.c_str(), &sb) == -1) { + if (errno != ENOENT || printnoent) { + PLOG(ERROR) << "stat \"" << fn << "\" failed"; + PrintHashForMissingStashedBlocks(id, params.fd); } + return -1; + } - LOG(INFO) << " loading " << fn; + LOG(INFO) << " loading " << fn; - if ((sb.st_size % BLOCKSIZE) != 0) { - LOG(ERROR) << fn << " size " << sb.st_size << " not multiple of block size " << BLOCKSIZE; - return -1; - } + if ((sb.st_size % BLOCKSIZE) != 0) { + LOG(ERROR) << fn << " size " << sb.st_size << " not multiple of block size " << BLOCKSIZE; + return -1; + } - android::base::unique_fd fd(TEMP_FAILURE_RETRY(ota_open(fn.c_str(), O_RDONLY))); - if (fd == -1) { - PLOG(ERROR) << "open \"" << fn << "\" failed"; - return -1; - } + android::base::unique_fd fd(TEMP_FAILURE_RETRY(ota_open(fn.c_str(), O_RDONLY))); + if (fd == -1) { + PLOG(ERROR) << "open \"" << fn << "\" failed"; + return -1; + } - allocate(sb.st_size, buffer); + allocate(sb.st_size, buffer); - if (read_all(fd, buffer, sb.st_size) == -1) { - return -1; - } + if (read_all(fd, buffer, sb.st_size) == -1) { + return -1; + } - *blocks = sb.st_size / BLOCKSIZE; + *blocks = sb.st_size / BLOCKSIZE; - if (verify && VerifyBlocks(id, buffer, *blocks, true) != 0) { - LOG(ERROR) << "unexpected contents in " << fn; - if (stash_map.find(id) == stash_map.end()) { - LOG(ERROR) << "failed to find source blocks number for stash " << id - << " when executing command: " << params.cmdname; - } else { - const RangeSet& src = stash_map[id]; - PrintHashForCorruptedStashedBlocks(id, buffer, src); - } - DeleteFile(fn); - return -1; + if (verify && VerifyBlocks(id, buffer, *blocks, true) != 0) { + LOG(ERROR) << "unexpected contents in " << fn; + if (stash_map.find(id) == stash_map.end()) { + LOG(ERROR) << "failed to find source blocks number for stash " << id + << " when executing command: " << params.cmdname; + } else { + const RangeSet& src = stash_map[id]; + PrintHashForCorruptedStashedBlocks(id, buffer, src); } + DeleteFile(fn); + return -1; + } - return 0; + return 0; } static int WriteStash(const std::string& base, const std::string& id, int blocks, @@ -780,21 +769,19 @@ static int FreeStash(const std::string& base, const std::string& id) { return 0; } +// Source contains packed data, which we want to move to the locations given in locs in the dest +// buffer. source and dest may be the same buffer. static void MoveRange(std::vector& dest, const RangeSet& locs, - const std::vector& source) { - // source contains packed data, which we want to move to the - // locations given in locs in the dest buffer. source and dest - // may be the same buffer. - - const uint8_t* from = source.data(); - uint8_t* to = dest.data(); - size_t start = locs.size; - for (int i = locs.count-1; i >= 0; --i) { - size_t blocks = locs.pos[i*2+1] - locs.pos[i*2]; - start -= blocks; - memmove(to + (locs.pos[i*2] * BLOCKSIZE), from + (start * BLOCKSIZE), - blocks * BLOCKSIZE); - } + const std::vector& source) { + const uint8_t* from = source.data(); + uint8_t* to = dest.data(); + size_t start = locs.blocks(); + // Must do the movement backward. + for (auto it = locs.crbegin(); it != locs.crend(); it++) { + size_t blocks = it->second - it->first; + start -= blocks; + memmove(to + (it->first * BLOCKSIZE), from + (start * BLOCKSIZE), blocks * BLOCKSIZE); + } } /** @@ -933,13 +920,13 @@ static int LoadSrcTgtVersion3(CommandParameters& params, RangeSet& tgt, size_t* // tgt = RangeSet::Parse(params.tokens[params.cpos++]); - std::vector tgtbuffer(tgt.size * BLOCKSIZE); + std::vector tgtbuffer(tgt.blocks() * BLOCKSIZE); if (ReadBlocks(tgt, tgtbuffer, params.fd) == -1) { return -1; } // Return now if target blocks already have expected content. - if (VerifyBlocks(tgthash, tgtbuffer, tgt.size, false) == 0) { + if (VerifyBlocks(tgthash, tgtbuffer, tgt.blocks(), false) == 0) { return 1; } @@ -1023,7 +1010,7 @@ static int PerformCommandMove(CommandParameters& params) { params.freestash.clear(); } - params.written += tgt.size; + params.written += tgt.blocks(); return 0; } @@ -1045,11 +1032,11 @@ static int PerformCommandStash(CommandParameters& params) { RangeSet src = RangeSet::Parse(params.tokens[params.cpos++]); - allocate(src.size * BLOCKSIZE, params.buffer); + allocate(src.blocks() * BLOCKSIZE, params.buffer); if (ReadBlocks(src, params.buffer, params.fd) == -1) { return -1; } - blocks = src.size; + blocks = src.blocks(); stash_map[id] = src; if (VerifyBlocks(id, params.buffer, blocks, true) != 0) { @@ -1088,46 +1075,45 @@ static int PerformCommandFree(CommandParameters& params) { } static int PerformCommandZero(CommandParameters& params) { + if (params.cpos >= params.tokens.size()) { + LOG(ERROR) << "missing target blocks for zero"; + return -1; + } - if (params.cpos >= params.tokens.size()) { - LOG(ERROR) << "missing target blocks for zero"; - return -1; - } + RangeSet tgt = RangeSet::Parse(params.tokens[params.cpos++]); - RangeSet tgt = RangeSet::Parse(params.tokens[params.cpos++]); + LOG(INFO) << " zeroing " << tgt.blocks() << " blocks"; - LOG(INFO) << " zeroing " << tgt.size << " blocks"; + allocate(BLOCKSIZE, params.buffer); + memset(params.buffer.data(), 0, BLOCKSIZE); + + if (params.canwrite) { + for (const auto& range : tgt) { + off64_t offset = static_cast(range.first) * BLOCKSIZE; + size_t size = (range.second - range.first) * BLOCKSIZE; + if (!discard_blocks(params.fd, offset, size)) { + return -1; + } - allocate(BLOCKSIZE, params.buffer); - memset(params.buffer.data(), 0, BLOCKSIZE); + if (!check_lseek(params.fd, offset, SEEK_SET)) { + return -1; + } - if (params.canwrite) { - for (size_t i = 0; i < tgt.count; ++i) { - off64_t offset = static_cast(tgt.pos[i * 2]) * BLOCKSIZE; - size_t size = (tgt.pos[i * 2 + 1] - tgt.pos[i * 2]) * BLOCKSIZE; - if (!discard_blocks(params.fd, offset, size)) { - return -1; - } - - if (!check_lseek(params.fd, offset, SEEK_SET)) { - return -1; - } - - for (size_t j = tgt.pos[i * 2]; j < tgt.pos[i * 2 + 1]; ++j) { - if (write_all(params.fd, params.buffer, BLOCKSIZE) == -1) { - return -1; - } - } + for (size_t j = range.first; j < range.second; ++j) { + if (write_all(params.fd, params.buffer, BLOCKSIZE) == -1) { + return -1; } + } } + } - if (params.cmdname[0] == 'z') { - // Update only for the zero command, as the erase command will call - // this if DEBUG_ERASE is defined. - params.written += tgt.size; - } + if (params.cmdname[0] == 'z') { + // Update only for the zero command, as the erase command will call + // this if DEBUG_ERASE is defined. + params.written += tgt.blocks(); + } - return 0; + return 0; } static int PerformCommandNew(CommandParameters& params) { @@ -1139,7 +1125,7 @@ static int PerformCommandNew(CommandParameters& params) { RangeSet tgt = RangeSet::Parse(params.tokens[params.cpos++]); if (params.canwrite) { - LOG(INFO) << " writing " << tgt.size << " blocks of new data"; + LOG(INFO) << " writing " << tgt.blocks() << " blocks of new data"; RangeSinkWriter writer(params.fd, tgt); pthread_mutex_lock(¶ms.nti.mu); @@ -1153,7 +1139,7 @@ static int PerformCommandNew(CommandParameters& params) { pthread_mutex_unlock(¶ms.nti.mu); } - params.written += tgt.size; + params.written += tgt.blocks(); return 0; } @@ -1195,7 +1181,7 @@ static int PerformCommandDiff(CommandParameters& params) { if (params.canwrite) { if (status == 0) { - LOG(INFO) << "patching " << blocks << " blocks to " << tgt.size; + LOG(INFO) << "patching " << blocks << " blocks to " << tgt.blocks(); Value patch_value( VAL_BLOB, std::string(reinterpret_cast(params.patch_start + offset), len)); @@ -1223,7 +1209,7 @@ static int PerformCommandDiff(CommandParameters& params) { LOG(ERROR) << "range sink underrun?"; } } else { - LOG(INFO) << "skipping " << blocks << " blocks already patched to " << tgt.size << " [" + LOG(INFO) << "skipping " << blocks << " blocks already patched to " << tgt.blocks() << " [" << params.cmdline << "]"; } } @@ -1233,52 +1219,52 @@ static int PerformCommandDiff(CommandParameters& params) { params.freestash.clear(); } - params.written += tgt.size; + params.written += tgt.blocks(); return 0; } static int PerformCommandErase(CommandParameters& params) { - if (DEBUG_ERASE) { - return PerformCommandZero(params); - } + if (DEBUG_ERASE) { + return PerformCommandZero(params); + } - struct stat sb; - if (fstat(params.fd, &sb) == -1) { - PLOG(ERROR) << "failed to fstat device to erase"; - return -1; - } + struct stat sb; + if (fstat(params.fd, &sb) == -1) { + PLOG(ERROR) << "failed to fstat device to erase"; + return -1; + } - if (!S_ISBLK(sb.st_mode)) { - LOG(ERROR) << "not a block device; skipping erase"; - return -1; - } + if (!S_ISBLK(sb.st_mode)) { + LOG(ERROR) << "not a block device; skipping erase"; + return -1; + } - if (params.cpos >= params.tokens.size()) { - LOG(ERROR) << "missing target blocks for erase"; - return -1; - } + if (params.cpos >= params.tokens.size()) { + LOG(ERROR) << "missing target blocks for erase"; + return -1; + } + + RangeSet tgt = RangeSet::Parse(params.tokens[params.cpos++]); - RangeSet tgt = RangeSet::Parse(params.tokens[params.cpos++]); + if (params.canwrite) { + LOG(INFO) << " erasing " << tgt.blocks() << " blocks"; - if (params.canwrite) { - LOG(INFO) << " erasing " << tgt.size << " blocks"; - - for (size_t i = 0; i < tgt.count; ++i) { - uint64_t blocks[2]; - // offset in bytes - blocks[0] = tgt.pos[i * 2] * (uint64_t) BLOCKSIZE; - // length in bytes - blocks[1] = (tgt.pos[i * 2 + 1] - tgt.pos[i * 2]) * (uint64_t) BLOCKSIZE; - - if (ioctl(params.fd, BLKDISCARD, &blocks) == -1) { - PLOG(ERROR) << "BLKDISCARD ioctl failed"; - return -1; - } - } + for (const auto& range : tgt) { + uint64_t blocks[2]; + // offset in bytes + blocks[0] = range.first * static_cast(BLOCKSIZE); + // length in bytes + blocks[1] = (range.second - range.first) * static_cast(BLOCKSIZE); + + if (ioctl(params.fd, BLKDISCARD, &blocks) == -1) { + PLOG(ERROR) << "BLKDISCARD ioctl failed"; + return -1; + } } + } - return 0; + return 0; } // Definitions for transfer list command functions @@ -1645,14 +1631,14 @@ Value* RangeSha1Fn(const char* name, State* state, const std::vector buffer(BLOCKSIZE); - for (size_t i = 0; i < rs.count; ++i) { - if (!check_lseek(fd, (off64_t)rs.pos[i * 2] * BLOCKSIZE, SEEK_SET)) { + for (const auto& range : rs) { + if (!check_lseek(fd, static_cast(range.first) * BLOCKSIZE, SEEK_SET)) { ErrorAbort(state, kLseekFailure, "failed to seek %s: %s", blockdev_filename->data.c_str(), strerror(errno)); return StringValue(""); } - for (size_t j = rs.pos[i * 2]; j < rs.pos[i * 2 + 1]; ++j) { + for (size_t j = range.first; j < range.second; ++j) { if (read_all(fd, buffer, BLOCKSIZE) == -1) { ErrorAbort(state, kFreadFailure, "failed to read %s: %s", blockdev_filename->data.c_str(), strerror(errno)); @@ -1700,7 +1686,7 @@ Value* CheckFirstBlockFn(const char* name, State* state, return StringValue(""); } - RangeSet blk0{ 1 /*count*/, 1 /*size*/, std::vector{ 0, 1 } /*position*/ }; + RangeSet blk0(std::vector{ Range{ 0, 1 } }); std::vector block0_buffer(BLOCKSIZE); if (ReadBlocks(blk0, block0_buffer, fd) == -1) { @@ -1770,24 +1756,20 @@ Value* BlockImageRecoverFn(const char* name, State* state, } fec_status status; - if (!fh.get_status(status)) { ErrorAbort(state, kLibfecFailure, "failed to read FEC status"); return StringValue(""); } - RangeSet rs = RangeSet::Parse(ranges->data); - uint8_t buffer[BLOCKSIZE]; - - for (size_t i = 0; i < rs.count; ++i) { - for (size_t j = rs.pos[i * 2]; j < rs.pos[i * 2 + 1]; ++j) { + for (const auto& range : RangeSet::Parse(ranges->data)) { + for (size_t j = range.first; j < range.second; ++j) { // Stay within the data area, libfec validates and corrects metadata - if (status.data_size <= (uint64_t)j * BLOCKSIZE) { + if (status.data_size <= static_cast(j) * BLOCKSIZE) { continue; } - if (fh.pread(buffer, BLOCKSIZE, (off64_t)j * BLOCKSIZE) != BLOCKSIZE) { + if (fh.pread(buffer, BLOCKSIZE, static_cast(j) * BLOCKSIZE) != BLOCKSIZE) { ErrorAbort(state, kLibfecFailure, "failed to recover %s (block %zu): %s", filename->data.c_str(), j, strerror(errno)); return StringValue(""); diff --git a/updater/include/updater/rangeset.h b/updater/include/updater/rangeset.h index afaa82dcd..fad038043 100644 --- a/updater/include/updater/rangeset.h +++ b/updater/include/updater/rangeset.h @@ -19,16 +19,35 @@ #include #include +#include #include #include #include #include -struct RangeSet { - size_t count; // Limit is INT_MAX. - size_t size; // The number of blocks in the RangeSet. - std::vector pos; // Actual limit is INT_MAX. +using Range = std::pair; + +class RangeSet { + public: + RangeSet() : blocks_(0) {} + + explicit RangeSet(std::vector&& pairs) { + CHECK_NE(pairs.size(), static_cast(0)) << "Invalid number of tokens"; + + // Sanity check the input. + size_t result = 0; + for (const auto& range : pairs) { + CHECK_LT(range.first, range.second) + << "Empty or negative range: " << range.first << ", " << range.second; + size_t sz = range.second - range.first; + CHECK_LE(result, SIZE_MAX - sz) << "RangeSet size overflow"; + result += sz; + } + + ranges_ = pairs; + blocks_ = result; + } static RangeSet Parse(const std::string& range_text) { std::vector pieces = android::base::Split(range_text, ","); @@ -42,44 +61,43 @@ struct RangeSet { CHECK_EQ(num % 2, static_cast(0)) << "Number of tokens must be even: " << range_text; CHECK_EQ(num, pieces.size() - 1) << "Mismatching number of tokens: " << range_text; - std::vector pairs(num); - size_t size = 0; + std::vector pairs; for (size_t i = 0; i < num; i += 2) { - CHECK(android::base::ParseUint(pieces[i + 1], &pairs[i], static_cast(INT_MAX))); - CHECK(android::base::ParseUint(pieces[i + 2], &pairs[i + 1], static_cast(INT_MAX))); - CHECK_LT(pairs[i], pairs[i + 1]) - << "Empty or negative range: " << pairs[i] << ", " << pairs[i + 1]; - - size_t sz = pairs[i + 1] - pairs[i]; - CHECK_LE(size, SIZE_MAX - sz) << "RangeSet size overflow"; - size += sz; + size_t first; + CHECK(android::base::ParseUint(pieces[i + 1], &first, static_cast(INT_MAX))); + size_t second; + CHECK(android::base::ParseUint(pieces[i + 2], &second, static_cast(INT_MAX))); + + pairs.emplace_back(first, second); } - return RangeSet{ num / 2, size, std::move(pairs) }; + return RangeSet(std::move(pairs)); } // Get the block number for the i-th (starting from 0) block in the RangeSet. size_t GetBlockNumber(size_t idx) const { - CHECK_LT(idx, size) << "Index " << idx << " is greater than RangeSet size " << size; - for (size_t i = 0; i < pos.size(); i += 2) { - if (idx < pos[i + 1] - pos[i]) { - return pos[i] + idx; + CHECK_LT(idx, blocks_) << "Out of bound index " << idx << " (total blocks: " << blocks_ << ")"; + + for (const auto& range : ranges_) { + if (idx < range.second - range.first) { + return range.first + idx; } - idx -= (pos[i + 1] - pos[i]); + idx -= (range.second - range.first); } - CHECK(false); + + CHECK(false) << "Failed to find block number for index " << idx; return 0; // Unreachable, but to make compiler happy. } // RangeSet has half-closed half-open bounds. For example, "3,5" contains blocks 3 and 4. So "3,5" // and "5,7" are not overlapped. bool Overlaps(const RangeSet& other) const { - for (size_t i = 0; i < count; ++i) { - size_t start = pos[i * 2]; - size_t end = pos[i * 2 + 1]; - for (size_t j = 0; j < other.count; ++j) { - size_t other_start = other.pos[j * 2]; - size_t other_end = other.pos[j * 2 + 1]; + for (const auto& range : ranges_) { + size_t start = range.first; + size_t end = range.second; + for (const auto& other_range : other.ranges_) { + size_t other_start = other_range.first; + size_t other_end = other_range.second; // [start, end) vs [other_start, other_end) if (!(other_start >= end || start >= other_end)) { return true; @@ -89,7 +107,58 @@ struct RangeSet { return false; } + // size() gives the number of Range's in this RangeSet. + size_t size() const { + return ranges_.size(); + } + + // blocks() gives the number of all blocks in this RangeSet. + size_t blocks() const { + return blocks_; + } + + // We provide const iterators only. + std::vector::const_iterator cbegin() const { + return ranges_.cbegin(); + } + + std::vector::const_iterator cend() const { + return ranges_.cend(); + } + + // Need to provide begin()/end() since range-based loop expects begin()/end(). + std::vector::const_iterator begin() const { + return ranges_.cbegin(); + } + + std::vector::const_iterator end() const { + return ranges_.cend(); + } + + // Reverse const iterators for MoveRange(). + std::vector::const_reverse_iterator crbegin() const { + return ranges_.crbegin(); + } + + std::vector::const_reverse_iterator crend() const { + return ranges_.crend(); + } + + const Range& operator[](size_t i) const { + return ranges_[i]; + } + bool operator==(const RangeSet& other) const { - return (count == other.count && size == other.size && pos == other.pos); + // The orders of Range's matter. "4,1,5,8,10" != "4,8,10,1,5". + return (ranges_ == other.ranges_); } + + bool operator!=(const RangeSet& other) const { + return ranges_ != other.ranges_; + } + + private: + // Actual limit for each value and the total number are both INT_MAX. + std::vector ranges_; + size_t blocks_; }; -- cgit v1.2.3 From 3a8d98dd902da8b69cff731b3db03c75b176b751 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Mon, 3 Apr 2017 20:01:17 -0700 Subject: Abort the update if there's not enough new data Right now the update stuck in a deadlock if there's less new data than expection. Add some checkers and abort the update if such case happens. Also add a corresponding test. Bug: 36787146 Test: update aborts correctly on bullhead && recovery_component_test passes Change-Id: I914e4a2a4cf157b99ef2fc65bd21c6981e38ca47 --- updater/blockimg.cpp | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 0f08d17eb..d5c170473 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -149,7 +149,7 @@ static void allocate(size_t size, std::vector& buffer) { class RangeSinkWriter { public: RangeSinkWriter(int fd, const RangeSet& tgt) - : fd_(fd), tgt_(tgt), next_range_(0), current_range_left_(0) { + : fd_(fd), tgt_(tgt), next_range_(0), current_range_left_(0), bytes_written_(0) { CHECK_NE(tgt.size(), static_cast(0)); }; @@ -201,9 +201,14 @@ class RangeSinkWriter { written += write_now; } + bytes_written_ += written; return written; } + size_t BytesWritten() const { + return bytes_written_; + } + private: // The input data. int fd_; @@ -213,6 +218,8 @@ class RangeSinkWriter { size_t next_range_; // The number of bytes to write before moving to the next range. size_t current_range_left_; + // Total bytes written by the writer. + size_t bytes_written_; }; /** @@ -238,6 +245,7 @@ struct NewThreadInfo { ZipEntry entry; RangeSinkWriter* writer; + bool receiver_available; pthread_mutex_t mu; pthread_cond_t cv; @@ -274,9 +282,16 @@ static bool receive_new_data(const uint8_t* data, size_t size, void* cookie) { } static void* unzip_new_data(void* cookie) { - NewThreadInfo* nti = static_cast(cookie); - ProcessZipEntryContents(nti->za, &nti->entry, receive_new_data, nti); - return nullptr; + NewThreadInfo* nti = static_cast(cookie); + ProcessZipEntryContents(nti->za, &nti->entry, receive_new_data, nti); + + pthread_mutex_lock(&nti->mu); + nti->receiver_available = false; + if (nti->writer != nullptr) { + pthread_cond_broadcast(&nti->cv); + } + pthread_mutex_unlock(&nti->mu); + return nullptr; } static int ReadBlocks(const RangeSet& src, std::vector& buffer, int fd) { @@ -1133,6 +1148,12 @@ static int PerformCommandNew(CommandParameters& params) { pthread_cond_broadcast(¶ms.nti.cv); while (params.nti.writer != nullptr) { + if (!params.nti.receiver_available) { + LOG(ERROR) << "missing " << (tgt.blocks() * BLOCKSIZE - params.nti.writer->BytesWritten()) + << " bytes of new data"; + pthread_mutex_unlock(¶ms.nti.mu); + return -1; + } pthread_cond_wait(¶ms.nti.cv, ¶ms.nti.mu); } @@ -1361,6 +1382,7 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, if (params.canwrite) { params.nti.za = za; params.nti.entry = new_entry; + params.nti.receiver_available = true; pthread_mutex_init(¶ms.nti.mu, nullptr); pthread_cond_init(¶ms.nti.cv, nullptr); -- cgit v1.2.3 From 8706a98aa635236a95795f0a0c122bb3e591a50d Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Tue, 18 Apr 2017 08:28:26 -0700 Subject: Adding support for quiescent reboot to recovery Bug: 37401320 Test: build and push OTA and hit adb reboot recovery,quiescent. The screen should remain off throughout the upgrade process. Change-Id: Ibed3795c09e26c4fa73684d40b94e40c78394d3f --- updater/install.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'updater') diff --git a/updater/install.cpp b/updater/install.cpp index 857d7f1e0..888239c83 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -890,7 +890,10 @@ Value* RebootNowFn(const char* name, State* state, const std::vector Date: Tue, 18 Apr 2017 23:54:29 -0700 Subject: Move sysMapFile and sysReleaseMap into MemMapping class. Test: recovery_component_test Test: recovery_unit_test Test: Apply an OTA on angler. Change-Id: I7170f03e4ce1fe06184ca1d7bcce0a695f33ac4d --- updater/updater.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'updater') diff --git a/updater/updater.cpp b/updater/updater.cpp index c09e267a5..749c86a1f 100644 --- a/updater/updater.cpp +++ b/updater/updater.cpp @@ -89,7 +89,7 @@ int main(int argc, char** argv) { const char* package_filename = argv[3]; MemMapping map; - if (sysMapFile(package_filename, &map) != 0) { + if (!map.MapFile(package_filename)) { LOG(ERROR) << "failed to map package " << argv[3]; return 3; } @@ -213,7 +213,6 @@ int main(int argc, char** argv) { if (updater_info.package_zip) { CloseArchive(updater_info.package_zip); } - sysReleaseMap(&map); return 0; } -- cgit v1.2.3 From e0c88793d184648a8d08b5d925b21cc6c9623ffc Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Tue, 2 May 2017 16:07:18 -0700 Subject: Add a default error code when updater script aborts We didn't report error/cause codes unless there's an explict "Abort()" call inside the updater script. As a result, some cause codes set by ErrorAbort() didn't show up in last_install. To fix the issue, add a default error code when the script terminates abnormally (i.e. with non zero status). Bug: 37912405 Test: error/cause code shows up in last_install when argument parsing fails Change-Id: Ic6d3bd1855b853aeaa0760071e593a00cf6f0209 --- updater/updater.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'updater') diff --git a/updater/updater.cpp b/updater/updater.cpp index c09e267a5..37e003e66 100644 --- a/updater/updater.cpp +++ b/updater/updater.cpp @@ -193,13 +193,15 @@ int main(int argc, char** argv) { } } - if (state.error_code != kNoError) { - fprintf(cmd_pipe, "log error: %d\n", state.error_code); - // Cause code should provide additional information about the abort; - // report only when an error exists. - if (state.cause_code != kNoCause) { - fprintf(cmd_pipe, "log cause: %d\n", state.cause_code); - } + // Installation has been aborted. Set the error code to kScriptExecutionFailure unless + // a more specific code has been set in errmsg. + if (state.error_code == kNoError) { + state.error_code = kScriptExecutionFailure; + } + fprintf(cmd_pipe, "log error: %d\n", state.error_code); + // Cause code should provide additional information about the abort. + if (state.cause_code != kNoCause) { + fprintf(cmd_pipe, "log cause: %d\n", state.cause_code); } if (updater_info.package_zip) { -- cgit v1.2.3 From 397a8137a0074ade60eee5efb32cc5a46c8af73c Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 12 May 2017 11:02:27 -0700 Subject: updater: Update the mkfs.f2fs argument to match f2fs-tools 1.8.0. Commit adeb41a8c0da3122a2907acb4aafd7ff9bce26af has switched the argument for recovery. This CL handles the case for updater. Note that there's a chance the updater may run against the old recovery (and f2fs 1.4.1 binary). Not sending a 0-sector argument to f2fs 1.4.1 also works. Bug: 37758867 Test: Make an OTA package that calls format f2fs, with mkfs.f2fs 1.8.0 and 1.4.1 binaries respectively. Change-Id: I4d4bbe8c57544d1c514b7aa37fbf22a0aab14e2c --- updater/install.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'updater') diff --git a/updater/install.cpp b/updater/install.cpp index 888239c83..c5f9a89bb 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -317,9 +317,11 @@ Value* FormatFn(const char* name, State* state, const std::vector(f2fs_argv)); + const char* f2fs_argv[] = { + "mkfs.f2fs", "-t", "-d1", location.c_str(), (size < 512) ? nullptr : num_sectors.c_str(), + nullptr + }; + int status = exec_cmd(f2fs_path, const_cast(f2fs_argv)); if (status != 0) { LOG(ERROR) << name << ": mkfs.f2fs failed (" << status << ") on " << location; return StringValue(""); -- cgit v1.2.3 From 53c38b15381ace565227e49104a6fd64c4c28dcc Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Wed, 10 May 2017 14:47:45 -0700 Subject: kill package_extract_dir It's only used by file-based OTA which has been deprecated for O. Test: mma Change-Id: I439c93155ca94554d827142c99aa6c0845cc7561 --- updater/install.cpp | 32 -------------------------------- 1 file changed, 32 deletions(-) (limited to 'updater') diff --git a/updater/install.cpp b/updater/install.cpp index c5f9a89bb..ff79edce0 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -61,7 +61,6 @@ #include "mounts.h" #include "ota_io.h" #include "otautil/DirUtil.h" -#include "otautil/ZipUtil.h" #include "print_sha1.h" #include "tune2fs.h" #include "updater/updater.h" @@ -389,36 +388,6 @@ Value* SetProgressFn(const char* name, State* state, const std::vector>&argv) { - if (argv.size() != 2) { - return ErrorAbort(state, kArgsParsingFailure, "%s() expects 2 args, got %zu", name, - argv.size()); - } - - std::vector args; - if (!ReadArgs(state, argv, &args)) { - return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name); - } - const std::string& zip_path = args[0]; - const std::string& dest_path = args[1]; - - ZipArchiveHandle za = static_cast(state->cookie)->package_zip; - - // To create a consistent system image, never use the clock for timestamps. - constexpr struct utimbuf timestamp = { 1217592000, 1217592000 }; // 8/1/2008 default - - bool success = ExtractPackageRecursive(za, zip_path, dest_path, ×tamp, sehandle); - - return StringValue(success ? "t" : ""); -} - // package_extract_file(package_file[, dest_file]) // Extracts a single package_file from the update package and writes it to dest_file, // overwriting existing files if necessary. Without the dest_file argument, returns the @@ -1037,7 +1006,6 @@ void RegisterInstallFunctions() { RegisterFunction("format", FormatFn); RegisterFunction("show_progress", ShowProgressFn); RegisterFunction("set_progress", SetProgressFn); - RegisterFunction("package_extract_dir", PackageExtractDirFn); RegisterFunction("package_extract_file", PackageExtractFileFn); RegisterFunction("getprop", GetPropFn); -- cgit v1.2.3 From 6957555e295e8ae34ba5e8cfbfb1c7bd76b0024d Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Tue, 16 May 2017 15:51:46 -0700 Subject: Retry the update if ApplyBSDiffPatch | ApplyImagePatch fails We have seen one case when bspatch failed likely due to patch corruption. Since the package has passed verification before, we want to reboot and retry the patch command again since there's no alternative for users. We won't delete the stash before reboot, and the src has passed SHA1 check. If there's an error on the patch, it will fail the package verification during retry. Bug: 37855643 Test: angler reboots and retries the update when bspatch fails. Change-Id: I2ebac9621bd1f0649bb301b9a28a0dd079ed4e1d --- updater/blockimg.cpp | 2 ++ updater/updater.cpp | 4 ++++ 2 files changed, 6 insertions(+) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index d5c170473..df366b0b8 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -1213,6 +1213,7 @@ static int PerformCommandDiff(CommandParameters& params) { std::placeholders::_2), nullptr, nullptr) != 0) { LOG(ERROR) << "Failed to apply image patch."; + failure_type = kPatchApplicationFailure; return -1; } } else { @@ -1221,6 +1222,7 @@ static int PerformCommandDiff(CommandParameters& params) { std::placeholders::_2), nullptr) != 0) { LOG(ERROR) << "Failed to apply bsdiff patch."; + failure_type = kPatchApplicationFailure; return -1; } } diff --git a/updater/updater.cpp b/updater/updater.cpp index 1be8b6040..f5ff6df91 100644 --- a/updater/updater.cpp +++ b/updater/updater.cpp @@ -202,6 +202,10 @@ int main(int argc, char** argv) { // Cause code should provide additional information about the abort. if (state.cause_code != kNoCause) { fprintf(cmd_pipe, "log cause: %d\n", state.cause_code); + if (state.cause_code == kPatchApplicationFailure) { + LOG(INFO) << "Patch application failed, retry update."; + fprintf(cmd_pipe, "retry_update\n"); + } } if (updater_info.package_zip) { -- cgit v1.2.3 From 2330dd8733ce0b207058e3003a3b1efebc022394 Mon Sep 17 00:00:00 2001 From: Jeff Vander Stoep Date: Wed, 14 Jun 2017 15:30:39 -0700 Subject: Fix "No file_contexts" warning Fixed by Loading the file_contexts specified in libselinux, whereas previously recovery loaded /file_contexts which no longer exists. Bug: 62587423 Test: build and flash recovery on Angler. Warning is gone. Test: Wipe data and cache. Test: sideload OTA Change-Id: I11581c878b860ac5f412e6e8e7acde811f37870f --- updater/updater.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'updater') diff --git a/updater/updater.cpp b/updater/updater.cpp index f5ff6df91..1d8fa8e92 100644 --- a/updater/updater.cpp +++ b/updater/updater.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -139,9 +140,8 @@ int main(int argc, char** argv) { return 6; } - struct selinux_opt seopts[] = { { SELABEL_OPT_PATH, "/file_contexts" } }; - - sehandle = selabel_open(SELABEL_CTX_FILE, seopts, 1); + sehandle = selinux_android_file_context_handle(); + selinux_android_set_sehandle(sehandle); if (!sehandle) { fprintf(cmd_pipe, "ui_print Warning: No file_contexts\n"); -- cgit v1.2.3 From ac31808cd37cfb98755e5821dbb2efb5fe5cb12a Mon Sep 17 00:00:00 2001 From: Jin Qian Date: Fri, 21 Apr 2017 14:36:12 -0700 Subject: recovery: replace make_ext4 with e2fsprogs Execute mke2fs to create empty ext4 filesystem. Execute e2fsdroid to add files to filesystem. Test: enter recovery mode and wipe data Bug: 35219933 Change-Id: I10a9f4c1f4754ad864b2df45b1f879180ab33876 --- updater/install.cpp | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) (limited to 'updater') diff --git a/updater/install.cpp b/updater/install.cpp index ff79edce0..c9a3a0799 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -302,9 +302,32 @@ Value* FormatFn(const char* name, State* state, const std::vector(mke2fs_argv)); + if (status != 0) { + LOG(WARNING) << name << ": mke2fs failed (" << status << ") on " << location + << ", falling back to make_ext4fs"; + status = make_ext4fs(location.c_str(), size, mount_point.c_str(), sehandle); + if (status != 0) { + LOG(ERROR) << name << ": make_ext4fs failed (" << status << ") on " << location; + return StringValue(""); + } + return StringValue(location); + } + + const char* e2fsdroid_argv[] = { "/sbin/e2fsdroid_static", "-e", "-S", + "/file_contexts", "-a", mount_point.c_str(), + location.c_str(), nullptr }; + status = exec_cmd(e2fsdroid_argv[0], const_cast(e2fsdroid_argv)); if (status != 0) { - LOG(ERROR) << name << ": make_ext4fs failed (" << status << ") on " << location; + LOG(ERROR) << name << ": e2fsdroid failed (" << status << ") on " << location; return StringValue(""); } return StringValue(location); -- cgit v1.2.3 From 619b162d67e0dd4e5e99c78e67dd8729aa37320b Mon Sep 17 00:00:00 2001 From: Jin Qian Date: Fri, 21 Apr 2017 14:36:12 -0700 Subject: recovery: replace make_ext4 with e2fsprogs Execute mke2fs to create empty ext4 filesystem. Execute e2fsdroid to add files to filesystem. Test: enter recovery mode and wipe data Bug: 35219933 Change-Id: I10a9f4c1f4754ad864b2df45b1f879180ab33876 Merged-In: I10a9f4c1f4754ad864b2df45b1f879180ab33876 --- updater/install.cpp | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) (limited to 'updater') diff --git a/updater/install.cpp b/updater/install.cpp index ff79edce0..c9a3a0799 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -302,9 +302,32 @@ Value* FormatFn(const char* name, State* state, const std::vector(mke2fs_argv)); + if (status != 0) { + LOG(WARNING) << name << ": mke2fs failed (" << status << ") on " << location + << ", falling back to make_ext4fs"; + status = make_ext4fs(location.c_str(), size, mount_point.c_str(), sehandle); + if (status != 0) { + LOG(ERROR) << name << ": make_ext4fs failed (" << status << ") on " << location; + return StringValue(""); + } + return StringValue(location); + } + + const char* e2fsdroid_argv[] = { "/sbin/e2fsdroid_static", "-e", "-S", + "/file_contexts", "-a", mount_point.c_str(), + location.c_str(), nullptr }; + status = exec_cmd(e2fsdroid_argv[0], const_cast(e2fsdroid_argv)); if (status != 0) { - LOG(ERROR) << name << ": make_ext4fs failed (" << status << ") on " << location; + LOG(ERROR) << name << ": e2fsdroid failed (" << status << ") on " << location; return StringValue(""); } return StringValue(location); -- cgit v1.2.3 From 107a34f9fc2cb2cfbb1ff83631f778c2c147db22 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Thu, 29 Jun 2017 17:04:21 -0700 Subject: Add support to decompress brotli compressed new data Add a new writer that can decode the brotli-compressed system/vendor new data stored in the OTA zip. Brotli generally gives better compression rate at the cost of slightly increased time consumption. The patch.dat is already compressed by BZ; so there's no point to further compress it. For the given 1.9G bullhead system image: Size: 875M -> 787M; ~10% reduction of package size. Time: 147s -> 153s; ~4% increase of the block_image_update execution time. (I guess I/O takes much longer time than decompression.) Also it takes 4 minutes to compress the system image on my local machine, 3 more minutes than zip. Test: recovery tests pass && apply a full OTA with brotli compressed system/vendor.new.dat on bullhead Change-Id: I232335ebf662a9c55579ca073ad45265700a621e --- updater/Android.mk | 1 + updater/blockimg.cpp | 175 +++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 144 insertions(+), 32 deletions(-) (limited to 'updater') diff --git a/updater/Android.mk b/updater/Android.mk index a113fe86c..86dc48e30 100644 --- a/updater/Android.mk +++ b/updater/Android.mk @@ -47,6 +47,7 @@ updater_common_static_libraries := \ libcrypto_utils \ libcutils \ libtune2fs \ + libbrotli \ $(tune2fs_static_libraries) # libupdater (static library) diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index df366b0b8..2bec487fe 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -149,40 +150,32 @@ static void allocate(size_t size, std::vector& buffer) { class RangeSinkWriter { public: RangeSinkWriter(int fd, const RangeSet& tgt) - : fd_(fd), tgt_(tgt), next_range_(0), current_range_left_(0), bytes_written_(0) { + : fd_(fd), + tgt_(tgt), + next_range_(0), + current_range_left_(0), + bytes_written_(0) { CHECK_NE(tgt.size(), static_cast(0)); }; + virtual ~RangeSinkWriter() {}; + bool Finished() const { return next_range_ == tgt_.size() && current_range_left_ == 0; } - size_t Write(const uint8_t* data, size_t size) { + // Return number of bytes consumed; and 0 indicates a writing failure. + virtual size_t Write(const uint8_t* data, size_t size) { if (Finished()) { LOG(ERROR) << "range sink write overrun; can't write " << size << " bytes"; return 0; } - size_t written = 0; + size_t consumed = 0; while (size > 0) { // Move to the next range as needed. - if (current_range_left_ == 0) { - if (next_range_ < tgt_.size()) { - const Range& range = tgt_[next_range_]; - off64_t offset = static_cast(range.first) * BLOCKSIZE; - current_range_left_ = (range.second - range.first) * BLOCKSIZE; - next_range_++; - if (!discard_blocks(fd_, offset, current_range_left_)) { - break; - } - - if (!check_lseek(fd_, offset, SEEK_SET)) { - break; - } - } else { - // We can't write any more; return how many bytes have been written so far. - break; - } + if (!SeekToOutputRange()) { + break; } size_t write_now = size; @@ -198,21 +191,47 @@ class RangeSinkWriter { size -= write_now; current_range_left_ -= write_now; - written += write_now; + consumed += write_now; } - bytes_written_ += written; - return written; + bytes_written_ += consumed; + return consumed; } size_t BytesWritten() const { return bytes_written_; } - private: - // The input data. + protected: + // Set up the output cursor, move to next range if needed. + bool SeekToOutputRange() { + // We haven't finished the current range yet. + if (current_range_left_ != 0) { + return true; + } + // We can't write any more; let the write function return how many bytes have been written + // so far. + if (next_range_ >= tgt_.size()) { + return false; + } + + const Range& range = tgt_[next_range_]; + off64_t offset = static_cast(range.first) * BLOCKSIZE; + current_range_left_ = (range.second - range.first) * BLOCKSIZE; + next_range_++; + + if (!discard_blocks(fd_, offset, current_range_left_)) { + return false; + } + if (!check_lseek(fd_, offset, SEEK_SET)) { + return false; + } + return true; + } + + // The output file descriptor. int fd_; - // The destination for the data. + // The destination ranges for the data. const RangeSet& tgt_; // The next range that we should write to. size_t next_range_; @@ -222,6 +241,75 @@ class RangeSinkWriter { size_t bytes_written_; }; +class BrotliNewDataWriter : public RangeSinkWriter { + public: + BrotliNewDataWriter(int fd, const RangeSet& tgt, BrotliDecoderState* state) + : RangeSinkWriter(fd, tgt), state_(state) {} + + size_t Write(const uint8_t* data, size_t size) override { + if (Finished()) { + LOG(ERROR) << "Brotli new data write overrun; can't write " << size << " bytes"; + return 0; + } + CHECK(state_ != nullptr); + + size_t consumed = 0; + while (true) { + // Move to the next range as needed. + if (!SeekToOutputRange()) { + break; + } + + size_t available_in = size; + size_t write_now = std::min(32768, current_range_left_); + uint8_t buffer[write_now]; + + size_t available_out = write_now; + uint8_t* next_out = buffer; + + // The brotli decoder will update |data|, |available_in|, |next_out| and |available_out|. + BrotliDecoderResult result = BrotliDecoderDecompressStream( + state_, &available_in, &data, &available_out, &next_out, nullptr); + + // We don't have a way to recover from the decode error; report the failure. + if (result == BROTLI_DECODER_RESULT_ERROR) { + LOG(ERROR) << "Decompression failed with " + << BrotliDecoderErrorString(BrotliDecoderGetErrorCode(state_)); + return 0; + } + + if (write_all(fd_, buffer, write_now - available_out) == -1) { + return 0; + } + + LOG(DEBUG) << "bytes written: " << write_now - available_out << ", bytes consumed " + << size - available_in << ", decoder status " << result; + + // Update the total bytes written to output by the current writer; this is different from the + // consumed input bytes. + bytes_written_ += write_now - available_out; + current_range_left_ -= (write_now - available_out); + consumed += (size - available_in); + + // Update the remaining size. The input data ptr is already updated by brotli decoder + // function. + size = available_in; + + // Continue if we have more output to write, or more input to consume. + if (result == BROTLI_DECODER_RESULT_SUCCESS || + (result == BROTLI_DECODER_RESULT_NEEDS_MORE_INPUT && size == 0)) { + break; + } + } + + return consumed; + } + + private: + // Pointer to the decoder state. (initialized by PerformBlockImageUpdate) + BrotliDecoderState* state_; +}; + /** * All of the data for all the 'new' transfers is contained in one file in the update package, * concatenated together in the order in which transfers.list will need it. We want to stream it out @@ -243,8 +331,10 @@ class RangeSinkWriter { struct NewThreadInfo { ZipArchiveHandle za; ZipEntry entry; + bool brotli_compressed; - RangeSinkWriter* writer; + std::unique_ptr writer; + BrotliDecoderState* brotli_decoder_state; bool receiver_available; pthread_mutex_t mu; @@ -264,9 +354,16 @@ static bool receive_new_data(const uint8_t* data, size_t size, void* cookie) { // At this point nti->writer is set, and we own it. The main thread is waiting for it to // disappear from nti. - size_t written = nti->writer->Write(data, size); - data += written; - size -= written; + size_t consumed = nti->writer->Write(data, size); + + // We encounter a fatal error if we fail to consume any input bytes. If this happens, abort the + // extraction. + if (consumed == 0) { + LOG(ERROR) << "Failed to process " << size << " input bytes."; + return false; + } + data += consumed; + size -= consumed; if (nti->writer->Finished()) { // We have written all the bytes desired by this writer. @@ -1142,9 +1239,13 @@ static int PerformCommandNew(CommandParameters& params) { if (params.canwrite) { LOG(INFO) << " writing " << tgt.blocks() << " blocks of new data"; - RangeSinkWriter writer(params.fd, tgt); pthread_mutex_lock(¶ms.nti.mu); - params.nti.writer = &writer; + if (params.nti.brotli_compressed) { + params.nti.writer = + std::make_unique(params.fd, tgt, params.nti.brotli_decoder_state); + } else { + params.nti.writer = std::make_unique(params.fd, tgt); + } pthread_cond_broadcast(¶ms.nti.cv); while (params.nti.writer != nullptr) { @@ -1384,6 +1485,12 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, if (params.canwrite) { params.nti.za = za; params.nti.entry = new_entry; + // The entry is compressed by brotli if has a 'br' extension. + params.nti.brotli_compressed = android::base::EndsWith(new_data_fn->data, ".br"); + if (params.nti.brotli_compressed) { + // Initialize brotli decoder state. + params.nti.brotli_decoder_state = BrotliDecoderCreateInstance(nullptr, nullptr, nullptr); + } params.nti.receiver_available = true; pthread_mutex_init(¶ms.nti.mu, nullptr); @@ -1526,6 +1633,10 @@ pbiudone: } // params.fd will be automatically closed because it's a unique_fd. + if (params.nti.brotli_decoder_state != nullptr) { + BrotliDecoderDestroyInstance(params.nti.brotli_decoder_state); + } + // Only delete the stash if the update cannot be resumed, or it's a verification run and we // created the stash. if (params.isunresumable || (!params.canwrite && params.createdstash)) { -- cgit v1.2.3 From 7af933b6a6fd687bd17710ef6fda0ad5483e4d6d Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 11 Jul 2017 16:45:04 -0700 Subject: Remove the obsolete reference to /file_contexts. This file no longer exists: - /file_contexts has been split into plat_file_contexts and nonplat_file_contexts since commit b236eb6ca204cefcb926e19bd5682f9dcad4021d (system/sepolicy). - It was named /file_contexts.bin prior to the split. '-S file_contexts' is also no longer required by e2fsdroid, since commit 2fff6fb036cbbb6dedd7da3d208b312a9038a5ce (external/e2fsprogs). It will load the file contexts via libselinux. Test: Trigger the path by performing a data wipe for converting to FBE. Change-Id: I179939da409e5c0415ae0ea0bf5ddb23f9e6331e --- updater/install.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'updater') diff --git a/updater/install.cpp b/updater/install.cpp index c9a3a0799..bfe91e7f9 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -322,8 +322,7 @@ Value* FormatFn(const char* name, State* state, const std::vector(e2fsdroid_argv)); if (status != 0) { -- cgit v1.2.3 From d9759e8a7ba89d7005d86cf8c8e3e561d04e56a4 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Fri, 21 Jul 2017 21:06:52 +0000 Subject: Fix a case when brotli writer fails to write last few blocks of data receive_new_data may exit too early if the zip processor has sent all the raw data. As a result, the last few 'new' commands will fail even though the brotli decoder has more output in its buffer. Restruct the code so that 'NewThreadInfo' owns the decoder state solely; and receive_brotli_new_data is responsible for the decompression. Also reduce the test data size to 100 blocks to avoid the test timeout. Bug: 63802629 Test: recovery_component_test. on bullhead, apply full updates with and w/o brotli compressed entries, apply an incremental update. Change-Id: Id429b2c2f31951897961525609fa12c3657216b7 (cherry picked from commit 6ed175d5412deeaec9691f85757e45452407b8e3) --- updater/blockimg.cpp | 175 ++++++++++++++++++++++++--------------------------- 1 file changed, 81 insertions(+), 94 deletions(-) (limited to 'updater') diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 2bec487fe..a0b9ad233 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -158,20 +158,22 @@ class RangeSinkWriter { CHECK_NE(tgt.size(), static_cast(0)); }; - virtual ~RangeSinkWriter() {}; - bool Finished() const { return next_range_ == tgt_.size() && current_range_left_ == 0; } - // Return number of bytes consumed; and 0 indicates a writing failure. - virtual size_t Write(const uint8_t* data, size_t size) { + size_t AvailableSpace() const { + return tgt_.blocks() * BLOCKSIZE - bytes_written_; + } + + // Return number of bytes written; and 0 indicates a writing failure. + size_t Write(const uint8_t* data, size_t size) { if (Finished()) { LOG(ERROR) << "range sink write overrun; can't write " << size << " bytes"; return 0; } - size_t consumed = 0; + size_t written = 0; while (size > 0) { // Move to the next range as needed. if (!SeekToOutputRange()) { @@ -191,18 +193,18 @@ class RangeSinkWriter { size -= write_now; current_range_left_ -= write_now; - consumed += write_now; + written += write_now; } - bytes_written_ += consumed; - return consumed; + bytes_written_ += written; + return written; } size_t BytesWritten() const { return bytes_written_; } - protected: + private: // Set up the output cursor, move to next range if needed. bool SeekToOutputRange() { // We haven't finished the current range yet. @@ -241,75 +243,6 @@ class RangeSinkWriter { size_t bytes_written_; }; -class BrotliNewDataWriter : public RangeSinkWriter { - public: - BrotliNewDataWriter(int fd, const RangeSet& tgt, BrotliDecoderState* state) - : RangeSinkWriter(fd, tgt), state_(state) {} - - size_t Write(const uint8_t* data, size_t size) override { - if (Finished()) { - LOG(ERROR) << "Brotli new data write overrun; can't write " << size << " bytes"; - return 0; - } - CHECK(state_ != nullptr); - - size_t consumed = 0; - while (true) { - // Move to the next range as needed. - if (!SeekToOutputRange()) { - break; - } - - size_t available_in = size; - size_t write_now = std::min(32768, current_range_left_); - uint8_t buffer[write_now]; - - size_t available_out = write_now; - uint8_t* next_out = buffer; - - // The brotli decoder will update |data|, |available_in|, |next_out| and |available_out|. - BrotliDecoderResult result = BrotliDecoderDecompressStream( - state_, &available_in, &data, &available_out, &next_out, nullptr); - - // We don't have a way to recover from the decode error; report the failure. - if (result == BROTLI_DECODER_RESULT_ERROR) { - LOG(ERROR) << "Decompression failed with " - << BrotliDecoderErrorString(BrotliDecoderGetErrorCode(state_)); - return 0; - } - - if (write_all(fd_, buffer, write_now - available_out) == -1) { - return 0; - } - - LOG(DEBUG) << "bytes written: " << write_now - available_out << ", bytes consumed " - << size - available_in << ", decoder status " << result; - - // Update the total bytes written to output by the current writer; this is different from the - // consumed input bytes. - bytes_written_ += write_now - available_out; - current_range_left_ -= (write_now - available_out); - consumed += (size - available_in); - - // Update the remaining size. The input data ptr is already updated by brotli decoder - // function. - size = available_in; - - // Continue if we have more output to write, or more input to consume. - if (result == BROTLI_DECODER_RESULT_SUCCESS || - (result == BROTLI_DECODER_RESULT_NEEDS_MORE_INPUT && size == 0)) { - break; - } - } - - return consumed; - } - - private: - // Pointer to the decoder state. (initialized by PerformBlockImageUpdate) - BrotliDecoderState* state_; -}; - /** * All of the data for all the 'new' transfers is contained in one file in the update package, * concatenated together in the order in which transfers.list will need it. We want to stream it out @@ -354,16 +287,73 @@ static bool receive_new_data(const uint8_t* data, size_t size, void* cookie) { // At this point nti->writer is set, and we own it. The main thread is waiting for it to // disappear from nti. - size_t consumed = nti->writer->Write(data, size); + size_t write_now = std::min(size, nti->writer->AvailableSpace()); + if (nti->writer->Write(data, write_now) != write_now) { + LOG(ERROR) << "Failed to write " << write_now << " bytes."; + return false; + } + + data += write_now; + size -= write_now; + + if (nti->writer->Finished()) { + // We have written all the bytes desired by this writer. + + pthread_mutex_lock(&nti->mu); + nti->writer = nullptr; + pthread_cond_broadcast(&nti->cv); + pthread_mutex_unlock(&nti->mu); + } + } + + return true; +} + +static bool receive_brotli_new_data(const uint8_t* data, size_t size, void* cookie) { + NewThreadInfo* nti = static_cast(cookie); + + while (size > 0 || BrotliDecoderHasMoreOutput(nti->brotli_decoder_state)) { + // Wait for nti->writer to be non-null, indicating some of this data is wanted. + pthread_mutex_lock(&nti->mu); + while (nti->writer == nullptr) { + pthread_cond_wait(&nti->cv, &nti->mu); + } + pthread_mutex_unlock(&nti->mu); + + // At this point nti->writer is set, and we own it. The main thread is waiting for it to + // disappear from nti. + + size_t buffer_size = std::min(32768, nti->writer->AvailableSpace()); + if (buffer_size == 0) { + LOG(ERROR) << "No space left in output range"; + return false; + } + uint8_t buffer[buffer_size]; + size_t available_in = size; + size_t available_out = buffer_size; + uint8_t* next_out = buffer; + + // The brotli decoder will update |data|, |available_in|, |next_out| and |available_out|. + BrotliDecoderResult result = BrotliDecoderDecompressStream( + nti->brotli_decoder_state, &available_in, &data, &available_out, &next_out, nullptr); - // We encounter a fatal error if we fail to consume any input bytes. If this happens, abort the - // extraction. - if (consumed == 0) { - LOG(ERROR) << "Failed to process " << size << " input bytes."; + if (result == BROTLI_DECODER_RESULT_ERROR) { + LOG(ERROR) << "Decompression failed with " + << BrotliDecoderErrorString(BrotliDecoderGetErrorCode(nti->brotli_decoder_state)); return false; } - data += consumed; - size -= consumed; + + LOG(DEBUG) << "bytes to write: " << buffer_size - available_out << ", bytes consumed " + << size - available_in << ", decoder status " << result; + + size_t write_now = buffer_size - available_out; + if (nti->writer->Write(buffer, write_now) != write_now) { + LOG(ERROR) << "Failed to write " << write_now << " bytes."; + return false; + } + + // Update the remaining size. The input data ptr is already updated by brotli decoder function. + size = available_in; if (nti->writer->Finished()) { // We have written all the bytes desired by this writer. @@ -380,8 +370,11 @@ static bool receive_new_data(const uint8_t* data, size_t size, void* cookie) { static void* unzip_new_data(void* cookie) { NewThreadInfo* nti = static_cast(cookie); - ProcessZipEntryContents(nti->za, &nti->entry, receive_new_data, nti); - + if (nti->brotli_compressed) { + ProcessZipEntryContents(nti->za, &nti->entry, receive_brotli_new_data, nti); + } else { + ProcessZipEntryContents(nti->za, &nti->entry, receive_new_data, nti); + } pthread_mutex_lock(&nti->mu); nti->receiver_available = false; if (nti->writer != nullptr) { @@ -1240,12 +1233,7 @@ static int PerformCommandNew(CommandParameters& params) { LOG(INFO) << " writing " << tgt.blocks() << " blocks of new data"; pthread_mutex_lock(¶ms.nti.mu); - if (params.nti.brotli_compressed) { - params.nti.writer = - std::make_unique(params.fd, tgt, params.nti.brotli_decoder_state); - } else { - params.nti.writer = std::make_unique(params.fd, tgt); - } + params.nti.writer = std::make_unique(params.fd, tgt); pthread_cond_broadcast(¶ms.nti.cv); while (params.nti.writer != nullptr) { @@ -1485,7 +1473,6 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, if (params.canwrite) { params.nti.za = za; params.nti.entry = new_entry; - // The entry is compressed by brotli if has a 'br' extension. params.nti.brotli_compressed = android::base::EndsWith(new_data_fn->data, ".br"); if (params.nti.brotli_compressed) { // Initialize brotli decoder state. -- cgit v1.2.3