From 1c522df25f9524eaa0273538b3de0b9ad1b8fcea Mon Sep 17 00:00:00 2001 From: Yabin Cui Date: Wed, 10 Feb 2016 16:41:10 -0800 Subject: applypatch: use vector to store data in FileContents. Cherry pick this patch because it fixes the problem that a newed Value is released by free(). Bug: 26906416 Change-Id: Ib53b445cd415a1ed5e95733fbc4073f9ef4dbc43 (cherry picked from commit d6c93afcc28cc65217ba65eeb646009c4f15a2ad) --- applypatch/applypatch.cpp | 105 +++++++++++++++------------------------------- applypatch/applypatch.h | 12 ++---- applypatch/main.cpp | 67 ++++++++++++++--------------- updater/install.cpp | 21 +++++----- 4 files changed, 78 insertions(+), 127 deletions(-) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index 9f5e2f200..9d8a21797 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -56,8 +56,6 @@ static bool mtd_partitions_scanned = false; // // Return 0 on success. int LoadFileContents(const char* filename, FileContents* file) { - file->data = NULL; - // A special 'filename' beginning with "MTD:" or "EMMC:" means to // load the contents of a partition. if (strncmp(filename, "MTD:", 4) == 0 || @@ -70,31 +68,22 @@ int LoadFileContents(const char* filename, FileContents* file) { return -1; } - file->size = file->st.st_size; - file->data = nullptr; - - std::unique_ptr data( - static_cast(malloc(file->size)), free); - if (data == nullptr) { - printf("failed to allocate memory: %s\n", strerror(errno)); - return -1; - } - + std::vector data(file->st.st_size); FILE* f = ota_fopen(filename, "rb"); if (f == NULL) { printf("failed to open \"%s\": %s\n", filename, strerror(errno)); return -1; } - size_t bytes_read = ota_fread(data.get(), 1, file->size, f); - if (bytes_read != static_cast(file->size)) { - printf("short read of \"%s\" (%zu bytes of %zd)\n", filename, bytes_read, file->size); - fclose(f); + size_t bytes_read = ota_fread(data.data(), 1, data.size(), f); + if (bytes_read != data.size()) { + printf("short read of \"%s\" (%zu bytes of %zd)\n", filename, bytes_read, data.size()); + ota_fclose(f); return -1; } ota_fclose(f); - file->data = data.release(); - SHA1(file->data, file->size, file->sha1); + file->data = std::move(data); + SHA1(file->data.data(), file->data.size(), file->sha1); return 0; } @@ -193,17 +182,17 @@ static int LoadPartitionContents(const char* filename, FileContents* file) { uint8_t parsed_sha[SHA_DIGEST_LENGTH]; // Allocate enough memory to hold the largest size. - file->data = static_cast(malloc(size[index[pairs-1]])); - char* p = (char*)file->data; - file->size = 0; // # bytes read so far + std::vector data(size[index[pairs-1]]); + char* p = reinterpret_cast(data.data()); + size_t data_size = 0; // # bytes read so far bool found = false; for (size_t i = 0; i < pairs; ++i) { // Read enough additional bytes to get us up to the next size. (Again, // we're trying the possibilities in order of increasing size). - size_t next = size[index[i]] - file->size; - size_t read = 0; + size_t next = size[index[i]] - data_size; if (next > 0) { + size_t read = 0; switch (type) { case MTD: read = mtd_read_data(ctx, p, next); @@ -216,12 +205,11 @@ static int LoadPartitionContents(const char* filename, FileContents* file) { if (next != read) { printf("short read (%zu bytes of %zu) for partition \"%s\"\n", read, next, partition); - free(file->data); - file->data = NULL; return -1; } SHA1_Update(&sha_ctx, p, read); - file->size += read; + data_size += read; + p += read; } // Duplicate the SHA context and finalize the duplicate so we can @@ -233,8 +221,6 @@ static int LoadPartitionContents(const char* filename, FileContents* file) { if (ParseSha1(sha1sum[index[i]].c_str(), parsed_sha) != 0) { printf("failed to parse sha1 %s in %s\n", sha1sum[index[i]].c_str(), filename); - free(file->data); - file->data = NULL; return -1; } @@ -246,8 +232,6 @@ static int LoadPartitionContents(const char* filename, FileContents* file) { found = true; break; } - - p += read; } switch (type) { @@ -264,13 +248,13 @@ static int LoadPartitionContents(const char* filename, FileContents* file) { if (!found) { // Ran off the end of the list of (size,sha1) pairs without finding a match. printf("contents of partition \"%s\" didn't match %s\n", partition, filename); - free(file->data); - file->data = NULL; return -1; } SHA1_Final(file->sha1, &sha_ctx); + data.resize(data_size); + file->data = std::move(data); // Fake some stat() info. file->st.st_mode = 0644; file->st.st_uid = 0; @@ -289,10 +273,10 @@ int SaveFileContents(const char* filename, const FileContents* file) { return -1; } - ssize_t bytes_written = FileSink(file->data, file->size, &fd); - if (bytes_written != file->size) { - printf("short write of \"%s\" (%zd bytes of %zd) (%s)\n", - filename, bytes_written, file->size, strerror(errno)); + ssize_t bytes_written = FileSink(file->data.data(), file->data.size(), &fd); + if (bytes_written != static_cast(file->data.size())) { + printf("short write of \"%s\" (%zd bytes of %zu) (%s)\n", + filename, bytes_written, file->data.size(), strerror(errno)); ota_close(fd); return -1; } @@ -543,7 +527,6 @@ int FindMatchingPatch(uint8_t* sha1, char* const * const patch_sha1_str, int applypatch_check(const char* filename, int num_patches, char** const patch_sha1_str) { FileContents file; - file.data = NULL; // It's okay to specify no sha1s; the check will pass if the // LoadFileContents is successful. (Useful for reading @@ -555,9 +538,6 @@ int applypatch_check(const char* filename, int num_patches, printf("file \"%s\" doesn't have any of expected " "sha1 sums; checking cache\n", filename); - free(file.data); - file.data = NULL; - // If the source file is missing or corrupted, it might be because // we were killed in the middle of patching it. A copy of it // should have been made in CACHE_TEMP_SOURCE. If that file @@ -571,12 +551,9 @@ int applypatch_check(const char* filename, int num_patches, if (FindMatchingPatch(file.sha1, patch_sha1_str, num_patches) < 0) { printf("cache bits don't match any sha1 for \"%s\"\n", filename); - free(file.data); return 1; } } - - free(file.data); return 0; } @@ -674,8 +651,6 @@ int applypatch(const char* source_filename, FileContents copy_file; FileContents source_file; - copy_file.data = NULL; - source_file.data = NULL; const Value* source_patch_value = NULL; const Value* copy_patch_value = NULL; @@ -685,22 +660,20 @@ int applypatch(const char* source_filename, // The early-exit case: the patch was already applied, this file // has the desired hash, nothing for us to do. printf("already %s\n", short_sha1(target_sha1).c_str()); - free(source_file.data); return 0; } } - if (source_file.data == NULL || + if (source_file.data.empty() || (target_filename != source_filename && strcmp(target_filename, source_filename) != 0)) { // Need to load the source file: either we failed to load the // target file, or we did but it's different from the source file. - free(source_file.data); - source_file.data = NULL; + source_file.data.clear(); LoadFileContents(source_filename, &source_file); } - if (source_file.data != NULL) { + if (!source_file.data.empty()) { int to_use = FindMatchingPatch(source_file.sha1, patch_sha1_str, num_patches); if (to_use >= 0) { source_patch_value = patch_data[to_use]; @@ -708,8 +681,7 @@ int applypatch(const char* source_filename, } if (source_patch_value == NULL) { - free(source_file.data); - source_file.data = NULL; + source_file.data.clear(); printf("source file is bad; trying copy\n"); if (LoadFileContents(CACHE_TEMP_SOURCE, ©_file) < 0) { @@ -726,19 +698,14 @@ int applypatch(const char* source_filename, if (copy_patch_value == NULL) { // fail. printf("copy file doesn't match source SHA-1s either\n"); - free(copy_file.data); return 1; } } - int result = GenerateTarget(&source_file, source_patch_value, - ©_file, copy_patch_value, - source_filename, target_filename, - target_sha1, target_size, bonus_data); - free(source_file.data); - free(copy_file.data); - - return result; + return GenerateTarget(&source_file, source_patch_value, + ©_file, copy_patch_value, + source_filename, target_filename, + target_sha1, target_size, bonus_data); } /* @@ -759,7 +726,6 @@ int applypatch_flash(const char* source_filename, const char* target_filename, } FileContents source_file; - source_file.data = NULL; std::string target_str(target_filename); std::vector pieces = android::base::Split(target_str, ":"); @@ -777,7 +743,6 @@ int applypatch_flash(const char* source_filename, const char* target_filename, // The early-exit case: the image was already applied, this partition // has the desired hash, nothing for us to do. printf("already %s\n", short_sha1(target_sha1).c_str()); - free(source_file.data); return 0; } @@ -787,18 +752,14 @@ int applypatch_flash(const char* source_filename, const char* target_filename, printf("source \"%s\" doesn't have expected sha1 sum\n", source_filename); printf("expected: %s, found: %s\n", short_sha1(target_sha1).c_str(), short_sha1(source_file.sha1).c_str()); - free(source_file.data); return 1; } } - if (WriteToPartition(source_file.data, target_size, target_filename) != 0) { + if (WriteToPartition(source_file.data.data(), target_size, target_filename) != 0) { printf("write of copied data to %s failed\n", target_filename); - free(source_file.data); return 1; } - - free(source_file.data); return 0; } @@ -867,7 +828,7 @@ static int GenerateTarget(FileContents* source_file, // We still write the original source to cache, in case // the partition write is interrupted. - if (MakeFreeSpaceOnCache(source_file->size) < 0) { + if (MakeFreeSpaceOnCache(source_file->data.size()) < 0) { printf("not enough free space on /cache\n"); return 1; } @@ -908,7 +869,7 @@ static int GenerateTarget(FileContents* source_file, return 1; } - if (MakeFreeSpaceOnCache(source_file->size) < 0) { + if (MakeFreeSpaceOnCache(source_file->data.size()) < 0) { printf("not enough free space on /cache\n"); return 1; } @@ -951,10 +912,10 @@ static int GenerateTarget(FileContents* source_file, int result; if (use_bsdiff) { - result = ApplyBSDiffPatch(source_to_use->data, source_to_use->size, + result = ApplyBSDiffPatch(source_to_use->data.data(), source_to_use->data.size(), patch, 0, sink, token, &ctx); } else { - result = ApplyImagePatch(source_to_use->data, source_to_use->size, + result = ApplyImagePatch(source_to_use->data.data(), source_to_use->data.size(), patch, sink, token, &ctx, bonus_data); } diff --git a/applypatch/applypatch.h b/applypatch/applypatch.h index 14fb490ba..f392c5534 100644 --- a/applypatch/applypatch.h +++ b/applypatch/applypatch.h @@ -24,17 +24,11 @@ #include "openssl/sha.h" #include "edify/expr.h" -typedef struct _Patch { +struct FileContents { uint8_t sha1[SHA_DIGEST_LENGTH]; - const char* patch_filename; -} Patch; - -typedef struct _FileContents { - uint8_t sha1[SHA_DIGEST_LENGTH]; - unsigned char* data; - ssize_t size; + std::vector data; struct stat st; -} FileContents; +}; // When there isn't enough room on the target filesystem to hold the // patched version of the file, we copy the original here and delete diff --git a/applypatch/main.cpp b/applypatch/main.cpp index 7606d5d3c..9013760c4 100644 --- a/applypatch/main.cpp +++ b/applypatch/main.cpp @@ -46,40 +46,32 @@ static int SpaceMode(int argc, char** argv) { return CacheSizeCheck(bytes); } -// Parse arguments (which should be of the form "" or -// ":" into the new parallel arrays *sha1s and -// *patches (loading file contents into the patches). Returns true on +// Parse arguments (which should be of the form ":" +// into the new parallel arrays *sha1s and *files.Returns true on // success. static bool ParsePatchArgs(int argc, char** argv, std::vector* sha1s, - std::vector>* patches) { + std::vector* files) { uint8_t digest[SHA_DIGEST_LENGTH]; for (int i = 0; i < argc; ++i) { char* colon = strchr(argv[i], ':'); - if (colon != NULL) { - *colon = '\0'; - ++colon; + if (colon == nullptr) { + printf("no ':' in patch argument \"%s\"\n", argv[i]); + return false; } - + *colon = '\0'; + ++colon; if (ParseSha1(argv[i], digest) != 0) { printf("failed to parse sha1 \"%s\"\n", argv[i]); return false; } sha1s->push_back(argv[i]); - if (colon == NULL) { - patches->emplace_back(nullptr, FreeValue); - } else { - FileContents fc; - if (LoadFileContents(colon, &fc) != 0) { - return false; - } - std::unique_ptr value(new Value, FreeValue); - value->type = VAL_BLOB; - value->size = fc.size; - value->data = reinterpret_cast(fc.data); - patches->push_back(std::move(value)); + FileContents fc; + if (LoadFileContents(colon, &fc) != 0) { + return false; } + files->push_back(std::move(fc)); } return true; } @@ -90,17 +82,19 @@ static int FlashMode(const char* src_filename, const char* tgt_filename, } static int PatchMode(int argc, char** argv) { - std::unique_ptr bonus(nullptr, FreeValue); + FileContents bonusFc; + Value bonusValue; + Value* bonus = nullptr; + if (argc >= 3 && strcmp(argv[1], "-b") == 0) { - FileContents fc; - if (LoadFileContents(argv[2], &fc) != 0) { + if (LoadFileContents(argv[2], &bonusFc) != 0) { printf("failed to load bonus file %s\n", argv[2]); return 1; } - bonus.reset(new Value); + bonus = &bonusValue; bonus->type = VAL_BLOB; - bonus->size = fc.size; - bonus->data = reinterpret_cast(fc.data); + bonus->size = bonusFc.data.size(); + bonus->data = reinterpret_cast(bonusFc.data.data()); argc -= 2; argv += 2; } @@ -118,28 +112,29 @@ static int PatchMode(int argc, char** argv) { // If no : is provided, it is in flash mode. if (argc == 5) { - if (bonus != NULL) { + if (bonus != nullptr) { printf("bonus file not supported in flash mode\n"); return 1; } return FlashMode(argv[1], argv[2], argv[3], target_size); } - - std::vector sha1s; - std::vector> patches; - if (!ParsePatchArgs(argc-5, argv+5, &sha1s, &patches)) { + std::vector files; + if (!ParsePatchArgs(argc-5, argv+5, &sha1s, &files)) { printf("failed to parse patch args\n"); return 1; } - - std::vector patch_ptrs; - for (const auto& p : patches) { - patch_ptrs.push_back(p.get()); + std::vector patches(files.size()); + std::vector patch_ptrs(files.size()); + for (size_t i = 0; i < files.size(); ++i) { + patches[i].type = VAL_BLOB; + patches[i].size = files[i].data.size(); + patches[i].data = reinterpret_cast(files[i].data.data()); + patch_ptrs[i] = &patches[i]; } return applypatch(argv[1], argv[2], argv[3], target_size, patch_ptrs.size(), sha1s.data(), - patch_ptrs.data(), bonus.get()); + patch_ptrs.data(), bonus); } // This program applies binary patches to files in a way that is safe diff --git a/updater/install.cpp b/updater/install.cpp index a2efc0b97..b7d9e85a2 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -1398,21 +1398,22 @@ Value* ReadFileFn(const char* name, State* state, int argc, Expr* argv[]) { char* filename; if (ReadArgs(state, argv, 1, &filename) < 0) return NULL; - Value* v = reinterpret_cast(malloc(sizeof(Value))); + Value* v = static_cast(malloc(sizeof(Value))); + if (v == nullptr) { + return nullptr; + } v->type = VAL_BLOB; + v->size = -1; + v->data = nullptr; FileContents fc; if (LoadFileContents(filename, &fc) != 0) { - free(filename); - v->size = -1; - v->data = NULL; - free(fc.data); - return v; + v->data = static_cast(malloc(fc.data.size())); + if (v->data != nullptr) { + memcpy(v->data, fc.data.data(), fc.data.size()); + v->size = fc.data.size(); + } } - - v->size = fc.size; - v->data = (char*)fc.data; - free(filename); return v; } -- cgit v1.2.3