From 168724c31ad5241e157ebb35135a734fa075d53b Mon Sep 17 00:00:00 2001 From: Doug Zongker Date: Thu, 19 Dec 2013 15:16:57 -0800 Subject: fix unnecessarily slow writing of EMMC partitions These were attempts to write partitions "conservatively" in hopes of fixing the problems with writing the radio partition on Nexus 4. They didn't work (a kernel patch was needed), but got left in. They make writing of partitions unnecessarily slow (ie, we really shouldn't need to sync() after every 4kb). Roll back most of them, but leave the verification read-back in. Change-Id: I94badc0979e88816c5aa0485f6316c02be69173c --- applypatch/applypatch.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) (limited to 'applypatch') diff --git a/applypatch/applypatch.c b/applypatch/applypatch.c index 6b8da2a8a..cb9bc2349 100644 --- a/applypatch/applypatch.c +++ b/applypatch/applypatch.c @@ -424,20 +424,18 @@ int WriteToPartition(unsigned char* data, size_t len, { size_t start = 0; int success = 0; - int fd = open(partition, O_RDWR | O_SYNC); + int fd = open(partition, O_RDWR); if (fd < 0) { printf("failed to open %s: %s\n", partition, strerror(errno)); return -1; } int attempt; - for (attempt = 0; attempt < 10; ++attempt) { - size_t next_sync = start + (1<<20); - printf("raw O_SYNC write %s attempt %d start at %d\n", partition, attempt+1, start); + for (attempt = 0; attempt < 2; ++attempt) { lseek(fd, start, SEEK_SET); while (start < len) { size_t to_write = len - start; - if (to_write > 4096) to_write = 4096; + if (to_write > 1<<20) to_write = 1<<20; ssize_t written = write(fd, data+start, to_write); if (written < 0) { @@ -450,10 +448,6 @@ int WriteToPartition(unsigned char* data, size_t len, } } start += written; - if (start >= next_sync) { - fsync(fd); - next_sync = start + (1<<20); - } } fsync(fd); @@ -506,8 +500,6 @@ int WriteToPartition(unsigned char* data, size_t len, success = true; break; } - - sleep(2); } if (!success) { @@ -519,11 +511,7 @@ int WriteToPartition(unsigned char* data, size_t len, printf("error closing %s (%s)\n", partition, strerror(errno)); return -1; } - // hack: sync and sleep after closing in hopes of getting - // the data actually onto flash. - printf("sleeping after close\n"); sync(); - sleep(5); break; } } -- cgit v1.2.3 From a1bc148c7c81f886426c253f2ea7beb0f301f6b0 Mon Sep 17 00:00:00 2001 From: Doug Zongker Date: Thu, 13 Feb 2014 15:18:19 -0800 Subject: remove 'retouch' ASLR support Older versions of android supported an ASLR system where binaries were randomly twiddled at OTA install time. Remove support for this; we now use the ASLR support in the linux kernel. Change-Id: I8348eb0d6424692668dc1a00e2416fbef6c158a2 --- applypatch/applypatch.c | 35 ++++++++--------------------------- applypatch/applypatch.h | 4 +--- applypatch/main.c | 4 ++-- 3 files changed, 11 insertions(+), 32 deletions(-) (limited to 'applypatch') diff --git a/applypatch/applypatch.c b/applypatch/applypatch.c index cb9bc2349..c9c40c98f 100644 --- a/applypatch/applypatch.c +++ b/applypatch/applypatch.c @@ -24,6 +24,7 @@ #include #include #include +#include #include "mincrypt/sha.h" #include "applypatch.h" @@ -44,14 +45,11 @@ static int GenerateTarget(FileContents* source_file, static int mtd_partitions_scanned = 0; -// Read a file into memory; optionally (retouch_flag == RETOUCH_DO_MASK) mask -// the retouched entries back to their original value (such that SHA-1 checks -// don't fail due to randomization); store the file contents and associated +// Read a file into memory; store the file contents and associated // metadata in *file. // // Return 0 on success. -int LoadFileContents(const char* filename, FileContents* file, - int retouch_flag) { +int LoadFileContents(const char* filename, FileContents* file) { file->data = NULL; // A special 'filename' beginning with "MTD:" or "EMMC:" means to @@ -87,20 +85,6 @@ int LoadFileContents(const char* filename, FileContents* file, } fclose(f); - // apply_patch[_check] functions are blind to randomization. Randomization - // is taken care of in [Undo]RetouchBinariesFn. If there is a mismatch - // within a file, this means the file is assumed "corrupt" for simplicity. - if (retouch_flag) { - int32_t desired_offset = 0; - if (retouch_mask_data(file->data, file->size, - &desired_offset, NULL) != RETOUCH_DATA_MATCHED) { - printf("error trying to mask retouch entries\n"); - free(file->data); - file->data = NULL; - return -1; - } - } - SHA_hash(file->data, file->size, file->sha1); return 0; } @@ -579,7 +563,7 @@ int applypatch_check(const char* filename, // LoadFileContents is successful. (Useful for reading // partitions, where the filename encodes the sha1s; no need to // check them twice.) - if (LoadFileContents(filename, &file, RETOUCH_DO_MASK) != 0 || + if (LoadFileContents(filename, &file) != 0 || (num_patches > 0 && FindMatchingPatch(file.sha1, patch_sha1_str, num_patches) < 0)) { printf("file \"%s\" doesn't have any of expected " @@ -594,7 +578,7 @@ int applypatch_check(const char* filename, // exists and matches the sha1 we're looking for, the check still // passes. - if (LoadFileContents(CACHE_TEMP_SOURCE, &file, RETOUCH_DO_MASK) != 0) { + if (LoadFileContents(CACHE_TEMP_SOURCE, &file) != 0) { printf("failed to load cache file\n"); return 1; } @@ -730,8 +714,7 @@ int applypatch(const char* source_filename, const Value* copy_patch_value = NULL; // We try to load the target file into the source_file object. - if (LoadFileContents(target_filename, &source_file, - RETOUCH_DO_MASK) == 0) { + if (LoadFileContents(target_filename, &source_file) == 0) { if (memcmp(source_file.sha1, target_sha1, SHA_DIGEST_SIZE) == 0) { // The early-exit case: the patch was already applied, this file // has the desired hash, nothing for us to do. @@ -750,8 +733,7 @@ int applypatch(const char* source_filename, // target file, or we did but it's different from the source file. free(source_file.data); source_file.data = NULL; - LoadFileContents(source_filename, &source_file, - RETOUCH_DO_MASK); + LoadFileContents(source_filename, &source_file); } if (source_file.data != NULL) { @@ -767,8 +749,7 @@ int applypatch(const char* source_filename, source_file.data = NULL; printf("source file is bad; trying copy\n"); - if (LoadFileContents(CACHE_TEMP_SOURCE, ©_file, - RETOUCH_DO_MASK) < 0) { + if (LoadFileContents(CACHE_TEMP_SOURCE, ©_file) < 0) { // fail. printf("failed to read copy file\n"); return 1; diff --git a/applypatch/applypatch.h b/applypatch/applypatch.h index f1f13a100..ee54c24ea 100644 --- a/applypatch/applypatch.h +++ b/applypatch/applypatch.h @@ -19,7 +19,6 @@ #include #include "mincrypt/sha.h" -#include "minelf/Retouch.h" #include "edify/expr.h" typedef struct _Patch { @@ -61,8 +60,7 @@ int applypatch_check(const char* filename, int num_patches, char** const patch_sha1_str); -int LoadFileContents(const char* filename, FileContents* file, - int retouch_flag); +int LoadFileContents(const char* filename, FileContents* file); int SaveFileContents(const char* filename, const FileContents* file); void FreeFileContents(FileContents* file); int FindMatchingPatch(uint8_t* sha1, char* const * const patch_sha1_str, diff --git a/applypatch/main.c b/applypatch/main.c index f61db5d9e..8e9fe80ef 100644 --- a/applypatch/main.c +++ b/applypatch/main.c @@ -74,7 +74,7 @@ static int ParsePatchArgs(int argc, char** argv, (*patches)[i] = NULL; } else { FileContents fc; - if (LoadFileContents(colon, &fc, RETOUCH_DONT_MASK) != 0) { + if (LoadFileContents(colon, &fc) != 0) { goto abort; } (*patches)[i] = malloc(sizeof(Value)); @@ -103,7 +103,7 @@ int PatchMode(int argc, char** argv) { Value* bonus = NULL; if (argc >= 3 && strcmp(argv[1], "-b") == 0) { FileContents fc; - if (LoadFileContents(argv[2], &fc, RETOUCH_DONT_MASK) != 0) { + if (LoadFileContents(argv[2], &fc) != 0) { printf("failed to load bonus file %s\n", argv[2]); return 1; } -- cgit v1.2.3 From 3eb681d1de4eb0a4807e851c323568ed3f360381 Mon Sep 17 00:00:00 2001 From: Doug Zongker Date: Thu, 13 Feb 2014 15:49:35 -0800 Subject: remove remaining libminelf references Change-Id: Id38b08607829bccc031693cc03e60e849903b6f8 --- applypatch/Android.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'applypatch') diff --git a/applypatch/Android.mk b/applypatch/Android.mk index ef57f243c..4984093dd 100644 --- a/applypatch/Android.mk +++ b/applypatch/Android.mk @@ -28,7 +28,7 @@ include $(CLEAR_VARS) LOCAL_SRC_FILES := main.c LOCAL_MODULE := applypatch LOCAL_C_INCLUDES += bootable/recovery -LOCAL_STATIC_LIBRARIES += libapplypatch libmtdutils libmincrypt libbz libminelf +LOCAL_STATIC_LIBRARIES += libapplypatch libmtdutils libmincrypt libbz LOCAL_SHARED_LIBRARIES += libz libcutils libstdc++ libc include $(BUILD_EXECUTABLE) @@ -40,7 +40,7 @@ LOCAL_MODULE := applypatch_static LOCAL_FORCE_STATIC_EXECUTABLE := true LOCAL_MODULE_TAGS := eng LOCAL_C_INCLUDES += bootable/recovery -LOCAL_STATIC_LIBRARIES += libapplypatch libmtdutils libmincrypt libbz libminelf +LOCAL_STATIC_LIBRARIES += libapplypatch libmtdutils libmincrypt libbz LOCAL_STATIC_LIBRARIES += libz libcutils libstdc++ libc include $(BUILD_EXECUTABLE) -- cgit v1.2.3 From f3bb31c32fa879ccce358c15c93b7bd8582d1756 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Fri, 14 Mar 2014 09:39:48 -0700 Subject: Recovery 64-bit compile issues Change-Id: I92d5abd1a628feab3b0246924fab7f97ba3b9d34 --- applypatch/applypatch.c | 12 ++++++------ applypatch/imgpatch.c | 7 ++++--- 2 files changed, 10 insertions(+), 9 deletions(-) (limited to 'applypatch') diff --git a/applypatch/applypatch.c b/applypatch/applypatch.c index 6b8da2a8a..9d32ee904 100644 --- a/applypatch/applypatch.c +++ b/applypatch/applypatch.c @@ -247,7 +247,7 @@ static int LoadPartitionContents(const char* filename, FileContents* file) { break; } if (next != read) { - printf("short read (%d bytes of %d) for partition \"%s\"\n", + printf("short read (%zu bytes of %zu) for partition \"%s\"\n", read, next, partition); free(file->data); file->data = NULL; @@ -274,7 +274,7 @@ static int LoadPartitionContents(const char* filename, FileContents* file) { if (memcmp(sha_so_far, parsed_sha, SHA_DIGEST_SIZE) == 0) { // we have a match. stop reading the partition; we'll return // the data we've read so far. - printf("partition read matched size %d sha %s\n", + printf("partition read matched size %zu sha %s\n", size[index[i]], sha1sum[index[i]]); break; } @@ -402,7 +402,7 @@ int WriteToPartition(unsigned char* data, size_t len, size_t written = mtd_write_data(ctx, (char*)data, len); if (written != len) { - printf("only wrote %d of %d bytes to MTD %s\n", + printf("only wrote %zu of %zu bytes to MTD %s\n", written, len, partition); mtd_write_close(ctx); return -1; @@ -482,20 +482,20 @@ int WriteToPartition(unsigned char* data, size_t len, if (errno == EINTR) { read_count = 0; } else { - printf("verify read error %s at %d: %s\n", + printf("verify read error %s at %zu: %s\n", partition, p, strerror(errno)); return -1; } } if ((size_t)read_count < to_read) { - printf("short verify read %s at %d: %d %d %s\n", + printf("short verify read %s at %zu: %zd %zu %s\n", partition, p, read_count, to_read, strerror(errno)); } so_far += read_count; } if (memcmp(buffer, data+p, to_read)) { - printf("verification failed starting at %d\n", p); + printf("verification failed starting at %zu\n", p); start = p; break; } diff --git a/applypatch/imgpatch.c b/applypatch/imgpatch.c index 3a1df3872..af4d07281 100644 --- a/applypatch/imgpatch.c +++ b/applypatch/imgpatch.c @@ -18,6 +18,7 @@ // format. #include +#include #include #include #include @@ -35,7 +36,7 @@ * file, and update the SHA context with the output data as well. * Return 0 on success. */ -int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, +int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size __unused, const Value* patch, SinkFn sink, void* token, SHA_CTX* ctx, const Value* bonus_data) { @@ -132,7 +133,7 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, unsigned char* expanded_source = malloc(expanded_len); if (expanded_source == NULL) { - printf("failed to allocate %d bytes for expanded_source\n", + printf("failed to allocate %zu bytes for expanded_source\n", expanded_len); return -1; } @@ -163,7 +164,7 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, // We should have filled the output buffer exactly, except // for the bonus_size. if (strm.avail_out != bonus_size) { - printf("source inflation short by %d bytes\n", strm.avail_out-bonus_size); + printf("source inflation short by %zu bytes\n", strm.avail_out-bonus_size); return -1; } inflateEnd(&strm); -- cgit v1.2.3 From 4aa12dd0decafb139239779ab38e6ffda23109ab Mon Sep 17 00:00:00 2001 From: Doug Zongker Date: Tue, 13 May 2014 08:40:49 -0700 Subject: fix vulnerability in bspatch Patches with control data tuples with negative numbers in the first and/or second can cause bspatch to write to arbitrary locations in the heap. Change-Id: I8c5d81948be773e6483241131d3d166b6da27cb8 --- applypatch/bspatch.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'applypatch') diff --git a/applypatch/bspatch.c b/applypatch/bspatch.c index 2e80f81d0..1dc7ab10b 100644 --- a/applypatch/bspatch.c +++ b/applypatch/bspatch.c @@ -205,6 +205,11 @@ int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, ctrl[1] = offtin(buf+8); ctrl[2] = offtin(buf+16); + if (ctrl[0] < 0 || ctrl[1] < 0) { + printf("corrupt patch (negative byte counts)\n"); + return 1; + } + // Sanity check if (newpos + ctrl[0] > *new_size) { printf("corrupt patch (new file overrun)\n"); -- cgit v1.2.3 From bc7ffeda98a861e346c30c771d3258030f7fcf21 Mon Sep 17 00:00:00 2001 From: Doug Zongker Date: Fri, 15 Aug 2014 14:31:52 -0700 Subject: installer for new block OTA system (Cherry-pick back from master.) Bug: 16984795 Change-Id: Ifa3d8345c5e2a0be86fb28faa080ca82592a96b4 --- applypatch/applypatch.c | 6 +++--- applypatch/applypatch.h | 2 +- applypatch/bspatch.c | 4 +--- applypatch/imgpatch.c | 4 ++-- 4 files changed, 7 insertions(+), 9 deletions(-) (limited to 'applypatch') diff --git a/applypatch/applypatch.c b/applypatch/applypatch.c index 60e9e4a5c..bfb9440e4 100644 --- a/applypatch/applypatch.c +++ b/applypatch/applypatch.c @@ -32,7 +32,7 @@ #include "edify/expr.h" static int LoadPartitionContents(const char* filename, FileContents* file); -static ssize_t FileSink(unsigned char* data, ssize_t len, void* token); +static ssize_t FileSink(const unsigned char* data, ssize_t len, void* token); static int GenerateTarget(FileContents* source_file, const Value* source_patch_value, FileContents* copy_file, @@ -599,7 +599,7 @@ int ShowLicenses() { return 0; } -ssize_t FileSink(unsigned char* data, ssize_t len, void* token) { +ssize_t FileSink(const unsigned char* data, ssize_t len, void* token) { int fd = *(int *)token; ssize_t done = 0; ssize_t wrote; @@ -620,7 +620,7 @@ typedef struct { ssize_t pos; } MemorySinkInfo; -ssize_t MemorySink(unsigned char* data, ssize_t len, void* token) { +ssize_t MemorySink(const unsigned char* data, ssize_t len, void* token) { MemorySinkInfo* msi = (MemorySinkInfo*)token; if (msi->size - msi->pos < len) { return -1; diff --git a/applypatch/applypatch.h b/applypatch/applypatch.h index ee54c24ea..edec84812 100644 --- a/applypatch/applypatch.h +++ b/applypatch/applypatch.h @@ -40,7 +40,7 @@ typedef struct _FileContents { // and use it as the source instead. #define CACHE_TEMP_SOURCE "/cache/saved.file" -typedef ssize_t (*SinkFn)(unsigned char*, ssize_t, void*); +typedef ssize_t (*SinkFn)(const unsigned char*, ssize_t, void*); // applypatch.c int ShowLicenses(); diff --git a/applypatch/bspatch.c b/applypatch/bspatch.c index 1dc7ab10b..b34ec2a88 100644 --- a/applypatch/bspatch.c +++ b/applypatch/bspatch.c @@ -112,9 +112,7 @@ int ApplyBSDiffPatch(const unsigned char* old_data, ssize_t old_size, printf("short write of output: %d (%s)\n", errno, strerror(errno)); return 1; } - if (ctx) { - SHA_update(ctx, new_data, new_size); - } + if (ctx) SHA_update(ctx, new_data, new_size); free(new_data); return 0; diff --git a/applypatch/imgpatch.c b/applypatch/imgpatch.c index af4d07281..33c448762 100644 --- a/applypatch/imgpatch.c +++ b/applypatch/imgpatch.c @@ -95,7 +95,7 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size __unused, printf("failed to read chunk %d raw data\n", i); return -1; } - SHA_update(ctx, patch->data + pos, data_len); + if (ctx) SHA_update(ctx, patch->data + pos, data_len); if (sink((unsigned char*)patch->data + pos, data_len, token) != data_len) { printf("failed to write chunk %d raw data\n", i); @@ -217,7 +217,7 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size __unused, (long)have); return -1; } - SHA_update(ctx, temp_data, have); + if (ctx) SHA_update(ctx, temp_data, have); } while (ret != Z_STREAM_END); deflateEnd(&strm); -- cgit v1.2.3 From cddb68b5eafbeba696d5276bda1f1a9f70bbde42 Mon Sep 17 00:00:00 2001 From: Michael Runge Date: Wed, 29 Oct 2014 12:42:15 -0700 Subject: Use more aggressive sync writing to applypatch. We have seen cases where the boot partition is patched, but upon recovery the partition appears to be corrupted. Open up all patched files/partitions with O_SYNC, and do not ignore the errors from fsync/close operations. Bug: 18170529 Change-Id: I392ad0a321d937c4ad02eaeea9170be384a4744b --- applypatch/applypatch.c | 44 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) (limited to 'applypatch') diff --git a/applypatch/applypatch.c b/applypatch/applypatch.c index bfb9440e4..2c86e0984 100644 --- a/applypatch/applypatch.c +++ b/applypatch/applypatch.c @@ -309,7 +309,7 @@ static int LoadPartitionContents(const char* filename, FileContents* file) { // Save the contents of the given FileContents object under the given // filename. Return 0 on success. int SaveFileContents(const char* filename, const FileContents* file) { - int fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR); + int fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_SYNC, S_IRUSR | S_IWUSR); if (fd < 0) { printf("failed to open \"%s\" for write: %s\n", filename, strerror(errno)); @@ -324,8 +324,14 @@ int SaveFileContents(const char* filename, const FileContents* file) { close(fd); return -1; } - fsync(fd); - close(fd); + if (fsync(fd) != 0) { + printf("fsync of \"%s\" failed: %s\n", filename, strerror(errno)); + return -1; + } + if (close(fd) != 0) { + printf("close of \"%s\" failed: %s\n", filename, strerror(errno)); + return -1; + } if (chmod(filename, file->st.st_mode) != 0) { printf("chmod of \"%s\" failed: %s\n", filename, strerror(errno)); @@ -408,7 +414,7 @@ int WriteToPartition(unsigned char* data, size_t len, { size_t start = 0; int success = 0; - int fd = open(partition, O_RDWR); + int fd = open(partition, O_RDWR | O_SYNC); if (fd < 0) { printf("failed to open %s: %s\n", partition, strerror(errno)); return -1; @@ -433,7 +439,22 @@ int WriteToPartition(unsigned char* data, size_t len, } start += written; } - fsync(fd); + if (fsync(fd) != 0) { + printf("failed to sync to %s (%s)\n", + partition, strerror(errno)); + return -1; + } + if (close(fd) != 0) { + printf("failed to close %s (%s)\n", + partition, strerror(errno)); + return -1; + } + fd = open(partition, O_RDONLY); + if (fd < 0) { + printf("failed to reopen %s for verify (%s)\n", + partition, strerror(errno)); + return -1; + } // drop caches so our subsequent verification read // won't just be reading the cache. @@ -919,7 +940,8 @@ static int GenerateTarget(FileContents* source_file, strcpy(outname, target_filename); strcat(outname, ".patch"); - output = open(outname, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR); + output = open(outname, O_WRONLY | O_CREAT | O_TRUNC | O_SYNC, + S_IRUSR | S_IWUSR); if (output < 0) { printf("failed to open output file %s: %s\n", outname, strerror(errno)); @@ -950,8 +972,14 @@ static int GenerateTarget(FileContents* source_file, } if (output >= 0) { - fsync(output); - close(output); + if (fsync(output) != 0) { + printf("failed to fsync file \"%s\" (%s)\n", outname, strerror(errno)); + result = 1; + } + if (close(output) != 0) { + printf("failed to close file \"%s\" (%s)\n", outname, strerror(errno)); + result = 1; + } } if (result != 0) { -- cgit v1.2.3