updater: fix memory leak based on static analysis.
Bug: 26907377
Change-Id: I384c0131322b2d12f0ef489735e70e86819846a4
diff --git a/updater/install.cpp b/updater/install.cpp
index 5326b12..45bbf2b 100644
--- a/updater/install.cpp
+++ b/updater/install.cpp
@@ -34,6 +34,9 @@
#include <linux/xattr.h>
#include <inttypes.h>
+#include <memory>
+#include <vector>
+
#include <android-base/parseint.h>
#include <android-base/strings.h>
#include <android-base/stringprintf.h>
@@ -439,8 +442,7 @@
for (int i = 0; i < argc; ++i) {
paths[i] = Evaluate(state, argv[i]);
if (paths[i] == NULL) {
- int j;
- for (j = 0; j < i; ++i) {
+ for (int j = 0; j < i; ++j) {
free(paths[j]);
}
free(paths);
@@ -581,13 +583,13 @@
// as the result.
char* zip_path;
+ if (ReadArgs(state, argv, 1, &zip_path) < 0) return NULL;
+
Value* v = reinterpret_cast<Value*>(malloc(sizeof(Value)));
v->type = VAL_BLOB;
v->size = -1;
v->data = NULL;
- if (ReadArgs(state, argv, 1, &zip_path) < 0) return NULL;
-
ZipArchive* za = ((UpdaterInfo*)(state->cookie))->package_zip;
const ZipEntry* entry = mzFindZipEntry(za, zip_path);
if (entry == NULL) {
@@ -1193,44 +1195,40 @@
}
int patchcount = (argc-4) / 2;
- Value** patches = ReadValueVarArgs(state, argc-4, argv+4);
+ std::unique_ptr<Value*, decltype(&free)> arg_values(ReadValueVarArgs(state, argc-4, argv+4),
+ free);
+ if (!arg_values) {
+ return nullptr;
+ }
+ std::vector<std::unique_ptr<Value, decltype(&FreeValue)>> patch_shas;
+ std::vector<std::unique_ptr<Value, decltype(&FreeValue)>> patches;
+ // Protect values by unique_ptrs first to get rid of memory leak.
+ for (int i = 0; i < patchcount * 2; i += 2) {
+ patch_shas.emplace_back(arg_values.get()[i], FreeValue);
+ patches.emplace_back(arg_values.get()[i+1], FreeValue);
+ }
- int i;
- for (i = 0; i < patchcount; ++i) {
- if (patches[i*2]->type != VAL_STRING) {
+ for (int i = 0; i < patchcount; ++i) {
+ if (patch_shas[i]->type != VAL_STRING) {
ErrorAbort(state, "%s(): sha-1 #%d is not string", name, i);
- break;
+ return nullptr;
}
- if (patches[i*2+1]->type != VAL_BLOB) {
+ if (patches[i]->type != VAL_BLOB) {
ErrorAbort(state, "%s(): patch #%d is not blob", name, i);
- break;
+ return nullptr;
}
}
- if (i != patchcount) {
- for (i = 0; i < patchcount*2; ++i) {
- FreeValue(patches[i]);
- }
- free(patches);
- return NULL;
- }
- char** patch_sha_str = reinterpret_cast<char**>(malloc(patchcount * sizeof(char*)));
- for (i = 0; i < patchcount; ++i) {
- patch_sha_str[i] = patches[i*2]->data;
- patches[i*2]->data = NULL;
- FreeValue(patches[i*2]);
- patches[i] = patches[i*2+1];
+ std::vector<char*> patch_sha_str;
+ std::vector<Value*> patch_ptrs;
+ for (int i = 0; i < patchcount; ++i) {
+ patch_sha_str.push_back(patch_shas[i]->data);
+ patch_ptrs.push_back(patches[i].get());
}
int result = applypatch(source_filename, target_filename,
target_sha1, target_size,
- patchcount, patch_sha_str, patches, NULL);
-
- for (i = 0; i < patchcount; ++i) {
- FreeValue(patches[i]);
- }
- free(patch_sha_str);
- free(patches);
+ patchcount, patch_sha_str.data(), patch_ptrs.data(), NULL);
return StringValue(strdup(result == 0 ? "t" : ""));
}
@@ -1349,9 +1347,13 @@
return ErrorAbort(state, "%s() expects at least 1 arg", name);
}
- Value** args = ReadValueVarArgs(state, argc, argv);
- if (args == NULL) {
- return NULL;
+ std::unique_ptr<Value*, decltype(&free)> arg_values(ReadValueVarArgs(state, argc, argv), free);
+ if (arg_values == nullptr) {
+ return nullptr;
+ }
+ std::vector<std::unique_ptr<Value, decltype(&FreeValue)>> args;
+ for (int i = 0; i < argc; ++i) {
+ args.emplace_back(arg_values.get()[i], FreeValue);
}
if (args[0]->size < 0) {
@@ -1359,14 +1361,13 @@
}
uint8_t digest[SHA_DIGEST_LENGTH];
SHA1(reinterpret_cast<uint8_t*>(args[0]->data), args[0]->size, digest);
- FreeValue(args[0]);
if (argc == 1) {
return StringValue(PrintSha1(digest));
}
int i;
- uint8_t* arg_digest = reinterpret_cast<uint8_t*>(malloc(SHA_DIGEST_LENGTH));
+ uint8_t arg_digest[SHA_DIGEST_LENGTH];
for (i = 1; i < argc; ++i) {
if (args[i]->type != VAL_STRING) {
printf("%s(): arg %d is not a string; skipping",
@@ -1378,19 +1379,13 @@
} else if (memcmp(digest, arg_digest, SHA_DIGEST_LENGTH) == 0) {
break;
}
- FreeValue(args[i]);
}
if (i >= argc) {
// Didn't match any of the hex strings; return false.
return StringValue(strdup(""));
}
- // Found a match; free all the remaining arguments and return the
- // matched one.
- int j;
- for (j = i+1; j < argc; ++j) {
- FreeValue(args[j]);
- }
- return args[i];
+ // Found a match.
+ return args[i].release();
}
// Read a local file and return its contents (the Value* returned