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)
diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp
index 9f5e2f2..9d8a217 100644
--- a/applypatch/applypatch.cpp
+++ b/applypatch/applypatch.cpp
@@ -56,8 +56,6 @@
//
// 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 @@
return -1;
}
- file->size = file->st.st_size;
- file->data = nullptr;
-
- std::unique_ptr<unsigned char, decltype(&free)> data(
- static_cast<unsigned char*>(malloc(file->size)), free);
- if (data == nullptr) {
- printf("failed to allocate memory: %s\n", strerror(errno));
- return -1;
- }
-
+ std::vector<unsigned char> 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<size_t>(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 @@
uint8_t parsed_sha[SHA_DIGEST_LENGTH];
// Allocate enough memory to hold the largest size.
- file->data = static_cast<unsigned char*>(malloc(size[index[pairs-1]]));
- char* p = (char*)file->data;
- file->size = 0; // # bytes read so far
+ std::vector<unsigned char> data(size[index[pairs-1]]);
+ char* p = reinterpret_cast<char*>(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 @@
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 @@
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 @@
found = true;
break;
}
-
- p += read;
}
switch (type) {
@@ -264,13 +248,13 @@
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 @@
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<ssize_t>(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 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 @@
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 @@
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 @@
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 @@
// 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 @@
}
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 @@
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 @@
}
FileContents source_file;
- source_file.data = NULL;
std::string target_str(target_filename);
std::vector<std::string> pieces = android::base::Split(target_str, ":");
@@ -777,7 +743,6 @@
// 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 @@
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 @@
// 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 @@
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 @@
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 14fb490..f392c55 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<unsigned char> 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 7606d5d..9013760 100644
--- a/applypatch/main.cpp
+++ b/applypatch/main.cpp
@@ -46,40 +46,32 @@
return CacheSizeCheck(bytes);
}
-// Parse arguments (which should be of the form "<sha1>" or
-// "<sha1>:<filename>" 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 "<sha1>:<filename>"
+// into the new parallel arrays *sha1s and *files.Returns true on
// success.
static bool ParsePatchArgs(int argc, char** argv, std::vector<char*>* sha1s,
- std::vector<std::unique_ptr<Value, decltype(&FreeValue)>>* patches) {
+ std::vector<FileContents>* 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, decltype(&FreeValue)> value(new Value, FreeValue);
- value->type = VAL_BLOB;
- value->size = fc.size;
- value->data = reinterpret_cast<char*>(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 PatchMode(int argc, char** argv) {
- std::unique_ptr<Value, decltype(&FreeValue)> 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<char*>(fc.data);
+ bonus->size = bonusFc.data.size();
+ bonus->data = reinterpret_cast<char*>(bonusFc.data.data());
argc -= 2;
argv += 2;
}
@@ -118,28 +112,29 @@
// If no <src-sha1>:<patch> 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<char*> sha1s;
- std::vector<std::unique_ptr<Value, decltype(&FreeValue)>> patches;
- if (!ParsePatchArgs(argc-5, argv+5, &sha1s, &patches)) {
+ std::vector<FileContents> files;
+ if (!ParsePatchArgs(argc-5, argv+5, &sha1s, &files)) {
printf("failed to parse patch args\n");
return 1;
}
-
- std::vector<Value*> patch_ptrs;
- for (const auto& p : patches) {
- patch_ptrs.push_back(p.get());
+ std::vector<Value> patches(files.size());
+ std::vector<Value*> 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<char*>(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 a2efc0b..b7d9e85 100644
--- a/updater/install.cpp
+++ b/updater/install.cpp
@@ -1398,21 +1398,22 @@
char* filename;
if (ReadArgs(state, argv, 1, &filename) < 0) return NULL;
- Value* v = reinterpret_cast<Value*>(malloc(sizeof(Value)));
+ Value* v = static_cast<Value*>(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<char*>(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;
}