From 7bad7c4646ee8fd8d6e6ed0ffd3ddbb0c1b41a2f Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 28 Apr 2015 17:24:24 -0700 Subject: Check all lseek calls succeed. Also add missing TEMP_FAILURE_RETRYs on read, write, and lseek. Bug: http://b/20625546 Change-Id: I03b198e11c1921b35518ee2dd005a7cfcf4fd94b --- minzip/SysUtil.c | 10 ++++++---- minzip/Zip.c | 6 ++---- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'minzip') diff --git a/minzip/SysUtil.c b/minzip/SysUtil.c index ac6f5c33f..b160c9e3d 100644 --- a/minzip/SysUtil.c +++ b/minzip/SysUtil.c @@ -27,11 +27,13 @@ static int getFileStartAndLength(int fd, off_t *start_, size_t *length_) assert(start_ != NULL); assert(length_ != NULL); - start = lseek(fd, 0L, SEEK_CUR); - end = lseek(fd, 0L, SEEK_END); - (void) lseek(fd, start, SEEK_SET); + // TODO: isn't start always 0 for the single call site? just use fstat instead? - if (start == (off_t) -1 || end == (off_t) -1) { + start = TEMP_FAILURE_RETRY(lseek(fd, 0L, SEEK_CUR)); + end = TEMP_FAILURE_RETRY(lseek(fd, 0L, SEEK_END)); + + if (TEMP_FAILURE_RETRY(lseek(fd, start, SEEK_SET)) == -1 || + start == (off_t) -1 || end == (off_t) -1) { LOGE("could not determine length of file\n"); return -1; } diff --git a/minzip/Zip.c b/minzip/Zip.c index d3ff79be6..40712e03a 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -675,13 +675,11 @@ static bool writeProcessFunction(const unsigned char *data, int dataLen, } ssize_t soFar = 0; while (true) { - ssize_t n = write(fd, data+soFar, dataLen-soFar); + ssize_t n = TEMP_FAILURE_RETRY(write(fd, data+soFar, dataLen-soFar)); if (n <= 0) { LOGE("Error writing %zd bytes from zip file from %p: %s\n", dataLen-soFar, data+soFar, strerror(errno)); - if (errno != EINTR) { - return false; - } + return false; } else if (n > 0) { soFar += n; if (soFar == dataLen) return true; -- cgit v1.2.3 From 80e46e08de5f65702fa7f7cd3ef83f905d919bbc Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 3 Jun 2015 10:49:29 -0700 Subject: recovery: Switch to clang And a few trival fixes to suppress warnings. Change-Id: I38734b5f4434643e85feab25f4807b46a45d8d65 --- minzip/Android.mk | 2 ++ minzip/Zip.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'minzip') diff --git a/minzip/Android.mk b/minzip/Android.mk index 045f35570..48d26bcb7 100644 --- a/minzip/Android.mk +++ b/minzip/Android.mk @@ -16,6 +16,8 @@ LOCAL_STATIC_LIBRARIES := libselinux LOCAL_MODULE := libminzip +LOCAL_CLANG := true + LOCAL_CFLAGS += -Wall include $(BUILD_STATIC_LIBRARY) diff --git a/minzip/Zip.c b/minzip/Zip.c index 40712e03a..a64c833f9 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -506,7 +506,6 @@ static bool processDeflatedEntry(const ZipArchive *pArchive, void *cookie) { long result = -1; - unsigned char readBuf[32 * 1024]; unsigned char procBuf[32 * 1024]; z_stream zstream; int zerr; @@ -603,7 +602,6 @@ bool mzProcessZipEntryContents(const ZipArchive *pArchive, void *cookie) { bool ret = false; - off_t oldOff; switch (pEntry->compression) { case STORED: -- cgit v1.2.3 From f267dee1cadba106eee373f7b1732bd4be9ebe13 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 23 Jun 2015 12:31:02 -0700 Subject: Just use fstat in sysMapFile. Also turn on -Werror and remove a dead function. Change-Id: I436f0a91c40e36db985190b3b98b0a4527cf0eeb --- minzip/Android.mk | 2 +- minzip/SysUtil.c | 86 +++++++++++++++---------------------------------------- minzip/Zip.c | 7 ----- 3 files changed, 24 insertions(+), 71 deletions(-) (limited to 'minzip') diff --git a/minzip/Android.mk b/minzip/Android.mk index 48d26bcb7..22eabfbb1 100644 --- a/minzip/Android.mk +++ b/minzip/Android.mk @@ -18,6 +18,6 @@ LOCAL_MODULE := libminzip LOCAL_CLANG := true -LOCAL_CFLAGS += -Wall +LOCAL_CFLAGS += -Werror -Wall include $(BUILD_STATIC_LIBRARY) diff --git a/minzip/SysUtil.c b/minzip/SysUtil.c index b160c9e3d..3dd3572fe 100644 --- a/minzip/SysUtil.c +++ b/minzip/SysUtil.c @@ -3,86 +3,46 @@ * * System utilities. */ -#include +#include +#include +#include +#include +#include #include -#include +#include #include #include -#include #include -#include -#include -#include -#include +#include +#include #define LOG_TAG "sysutil" #include "Log.h" #include "SysUtil.h" -static int getFileStartAndLength(int fd, off_t *start_, size_t *length_) -{ - off_t start, end; - size_t length; - - assert(start_ != NULL); - assert(length_ != NULL); - - // TODO: isn't start always 0 for the single call site? just use fstat instead? - - start = TEMP_FAILURE_RETRY(lseek(fd, 0L, SEEK_CUR)); - end = TEMP_FAILURE_RETRY(lseek(fd, 0L, SEEK_END)); - - if (TEMP_FAILURE_RETRY(lseek(fd, start, SEEK_SET)) == -1 || - start == (off_t) -1 || end == (off_t) -1) { - LOGE("could not determine length of file\n"); - return -1; - } - - length = end - start; - if (length == 0) { - LOGE("file is empty\n"); - return -1; - } - - *start_ = start; - *length_ = length; - - return 0; -} - -/* - * Map a file (from fd's current offset) into a private, read-only memory - * segment. The file offset must be a multiple of the page size. - * - * On success, returns 0 and fills out "pMap". On failure, returns a nonzero - * value and does not disturb "pMap". - */ -static int sysMapFD(int fd, MemMapping* pMap) -{ - off_t start; - size_t length; - void* memPtr; - +static bool sysMapFD(int fd, MemMapping* pMap) { assert(pMap != NULL); - if (getFileStartAndLength(fd, &start, &length) < 0) - return -1; + struct stat sb; + if (fstat(fd, &sb) == -1) { + LOGW("fstat(%d) failed: %s\n", fd, strerror(errno)); + return false; + } - memPtr = mmap(NULL, length, PROT_READ, MAP_PRIVATE, fd, start); + void* memPtr = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0); if (memPtr == MAP_FAILED) { - LOGW("mmap(%d, R, PRIVATE, %d, %d) failed: %s\n", (int) length, - fd, (int) start, strerror(errno)); - return -1; + LOGW("mmap(%d, R, PRIVATE, %d, 0) failed: %s\n", (int) sb.st_size, fd, strerror(errno)); + return false; } pMap->addr = memPtr; - pMap->length = length; + pMap->length = sb.st_size; pMap->range_count = 1; pMap->ranges = malloc(sizeof(MappedRange)); pMap->ranges[0].addr = memPtr; - pMap->ranges[0].length = length; + pMap->ranges[0].length = sb.st_size; - return 0; + return true; } static int sysMapBlockFile(FILE* mapf, MemMapping* pMap) @@ -180,13 +140,13 @@ int sysMapFile(const char* fn, MemMapping* pMap) fclose(mapf); } else { // This is a regular file. - int fd = open(fn, O_RDONLY, 0); - if (fd < 0) { + int fd = open(fn, O_RDONLY); + if (fd == -1) { LOGE("Unable to open '%s': %s\n", fn, strerror(errno)); return -1; } - if (sysMapFD(fd, pMap) != 0) { + if (!sysMapFD(fd, pMap)) { LOGE("Map of '%s' failed\n", fn); close(fd); return -1; diff --git a/minzip/Zip.c b/minzip/Zip.c index a64c833f9..c1dec742d 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -619,13 +619,6 @@ bool mzProcessZipEntryContents(const ZipArchive *pArchive, return ret; } -static bool crcProcessFunction(const unsigned char *data, int dataLen, - void *crc) -{ - *(unsigned long *)crc = crc32(*(unsigned long *)crc, data, dataLen); - return true; -} - typedef struct { char *buf; int bufLen; -- cgit v1.2.3 From 485b63702c312bf47a1fd4821fde7dcade41e09d Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 23 Jun 2015 23:23:33 -0700 Subject: recovery: Switch applypatch/ and updater/ to cpp. Mostly trivial changes to make cpp compiler happy. Change-Id: I1b0481465c67c3bbca35a839d0764190d84ff34e (cherry picked from commit ba9a42aa7e10686de186636fe9fecbf8c4cc7c19) --- minzip/Hash.h | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'minzip') diff --git a/minzip/Hash.h b/minzip/Hash.h index 8194537f3..e83eac414 100644 --- a/minzip/Hash.h +++ b/minzip/Hash.h @@ -15,6 +15,10 @@ #include #include +#ifdef __cplusplus +extern "C" { +#endif + /* compute the hash of an item with a specific type */ typedef unsigned int (*HashCompute)(const void* item); @@ -183,4 +187,8 @@ typedef unsigned int (*HashCalcFunc)(const void* item); void mzHashTableProbeCount(HashTable* pHashTable, HashCalcFunc calcFunc, HashCompareFunc cmpFunc); +#ifdef __cplusplus +} +#endif + #endif /*_MINZIP_HASH*/ -- cgit v1.2.3 From d7d0f7503456c3d275a49f90be35e03f02c51bbd Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 15 Jul 2015 14:13:06 -0700 Subject: Clean up LOG functions. For fatal errors, use LOGE to show messages. Bug: 22236461 Change-Id: Ie2ce7ec769f4502d732fbb53fb7b303c0cf9ed68 --- minzip/Hash.c | 4 ++-- minzip/SysUtil.c | 22 +++++++++++----------- minzip/Zip.c | 14 +++++++------- 3 files changed, 20 insertions(+), 20 deletions(-) (limited to 'minzip') diff --git a/minzip/Hash.c b/minzip/Hash.c index 8f8ed68e5..49bcb3161 100644 --- a/minzip/Hash.c +++ b/minzip/Hash.c @@ -361,7 +361,7 @@ void mzHashTableProbeCount(HashTable* pHashTable, HashCalcFunc calcFunc, { const void* data = (const void*)mzHashIterData(&iter); int count; - + count = countProbes(pHashTable, (*calcFunc)(data), data, cmpFunc); numEntries++; @@ -373,7 +373,7 @@ void mzHashTableProbeCount(HashTable* pHashTable, HashCalcFunc calcFunc, totalProbe += count; } - LOGI("Probe: min=%d max=%d, total=%d in %d (%d), avg=%.3f\n", + LOGV("Probe: min=%d max=%d, total=%d in %d (%d), avg=%.3f\n", minProbe, maxProbe, totalProbe, numEntries, pHashTable->tableSize, (float) totalProbe / (float) numEntries); } diff --git a/minzip/SysUtil.c b/minzip/SysUtil.c index 3dd3572fe..09ec8768f 100644 --- a/minzip/SysUtil.c +++ b/minzip/SysUtil.c @@ -25,13 +25,13 @@ static bool sysMapFD(int fd, MemMapping* pMap) { struct stat sb; if (fstat(fd, &sb) == -1) { - LOGW("fstat(%d) failed: %s\n", fd, strerror(errno)); + LOGE("fstat(%d) failed: %s\n", fd, strerror(errno)); return false; } void* memPtr = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0); if (memPtr == MAP_FAILED) { - LOGW("mmap(%d, R, PRIVATE, %d, 0) failed: %s\n", (int) sb.st_size, fd, strerror(errno)); + LOGE("mmap(%d, R, PRIVATE, %d, 0) failed: %s\n", (int) sb.st_size, fd, strerror(errno)); return false; } @@ -55,7 +55,7 @@ static int sysMapBlockFile(FILE* mapf, MemMapping* pMap) unsigned int i; if (fgets(block_dev, sizeof(block_dev), mapf) == NULL) { - LOGW("failed to read block device from header\n"); + LOGE("failed to read block device from header\n"); return -1; } for (i = 0; i < sizeof(block_dev); ++i) { @@ -66,7 +66,7 @@ static int sysMapBlockFile(FILE* mapf, MemMapping* pMap) } if (fscanf(mapf, "%zu %u\n%u\n", &size, &blksize, &range_count) != 3) { - LOGW("failed to parse block map header\n"); + LOGE("failed to parse block map header\n"); return -1; } @@ -80,7 +80,7 @@ static int sysMapBlockFile(FILE* mapf, MemMapping* pMap) unsigned char* reserve; reserve = mmap64(NULL, blocks * blksize, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0); if (reserve == MAP_FAILED) { - LOGW("failed to reserve address space: %s\n", strerror(errno)); + LOGE("failed to reserve address space: %s\n", strerror(errno)); return -1; } @@ -89,7 +89,7 @@ static int sysMapBlockFile(FILE* mapf, MemMapping* pMap) int fd = open(block_dev, O_RDONLY); if (fd < 0) { - LOGW("failed to open block device %s: %s\n", block_dev, strerror(errno)); + LOGE("failed to open block device %s: %s\n", block_dev, strerror(errno)); return -1; } @@ -97,13 +97,13 @@ static int sysMapBlockFile(FILE* mapf, MemMapping* pMap) for (i = 0; i < range_count; ++i) { int start, end; if (fscanf(mapf, "%d %d\n", &start, &end) != 2) { - LOGW("failed to parse range %d in block map\n", i); + LOGE("failed to parse range %d in block map\n", i); return -1; } void* addr = mmap64(next, (end-start)*blksize, PROT_READ, MAP_PRIVATE | MAP_FIXED, fd, ((off64_t)start)*blksize); if (addr == MAP_FAILED) { - LOGW("failed to map block %d: %s\n", i, strerror(errno)); + LOGE("failed to map block %d: %s\n", i, strerror(errno)); return -1; } pMap->ranges[i].addr = addr; @@ -128,12 +128,12 @@ int sysMapFile(const char* fn, MemMapping* pMap) // A map of blocks FILE* mapf = fopen(fn+1, "r"); if (mapf == NULL) { - LOGV("Unable to open '%s': %s\n", fn+1, strerror(errno)); + LOGE("Unable to open '%s': %s\n", fn+1, strerror(errno)); return -1; } if (sysMapBlockFile(mapf, pMap) != 0) { - LOGW("Map of '%s' failed\n", fn); + LOGE("Map of '%s' failed\n", fn); return -1; } @@ -165,7 +165,7 @@ void sysReleaseMap(MemMapping* pMap) int i; for (i = 0; i < pMap->range_count; ++i) { if (munmap(pMap->ranges[i].addr, pMap->ranges[i].length) < 0) { - LOGW("munmap(%p, %d) failed: %s\n", + LOGE("munmap(%p, %d) failed: %s\n", pMap->ranges[i].addr, (int)pMap->ranges[i].length, strerror(errno)); } } diff --git a/minzip/Zip.c b/minzip/Zip.c index c1dec742d..bdb565c64 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -198,10 +198,10 @@ static bool parseZipArchive(ZipArchive* pArchive) */ val = get4LE(pArchive->addr); if (val == ENDSIG) { - LOGI("Found Zip archive, but it looks empty\n"); + LOGW("Found Zip archive, but it looks empty\n"); goto bail; } else if (val != LOCSIG) { - LOGV("Not a Zip archive (found 0x%08x)\n", val); + LOGW("Not a Zip archive (found 0x%08x)\n", val); goto bail; } @@ -217,7 +217,7 @@ static bool parseZipArchive(ZipArchive* pArchive) ptr--; } if (ptr < (const unsigned char*) pArchive->addr) { - LOGI("Could not find end-of-central-directory in Zip\n"); + LOGW("Could not find end-of-central-directory in Zip\n"); goto bail; } @@ -429,7 +429,7 @@ int mzOpenZipArchive(unsigned char* addr, size_t length, ZipArchive* pArchive) if (length < ENDHDR) { err = -1; - LOGV("File '%s' too small to be zip (%zd)\n", fileName, map.length); + LOGW("Archive %p is too small to be zip (%zd)\n", pArchive, length); goto bail; } @@ -438,7 +438,7 @@ int mzOpenZipArchive(unsigned char* addr, size_t length, ZipArchive* pArchive) if (!parseZipArchive(pArchive)) { err = -1; - LOGV("Parsing '%s' failed\n", fileName); + LOGW("Parsing archive %p failed\n", pArchive); goto bail; } @@ -548,7 +548,7 @@ static bool processDeflatedEntry(const ZipArchive *pArchive, /* uncompress the data */ zerr = inflate(&zstream, Z_NO_FLUSH); if (zerr != Z_OK && zerr != Z_STREAM_END) { - LOGD("zlib inflate call failed (zerr=%d)\n", zerr); + LOGW("zlib inflate call failed (zerr=%d)\n", zerr); goto z_bail; } @@ -1007,7 +1007,7 @@ bool mzExtractRecursive(const ZipArchive *pArchive, if (callback != NULL) callback(targetFile, cookie); } - LOGD("Extracted %d file(s)\n", extractCount); + LOGV("Extracted %d file(s)\n", extractCount); free(helper.buf); free(zpath); -- cgit v1.2.3 From 4f2df162c6ab4a71ca86e4b38735b681729c353b Mon Sep 17 00:00:00 2001 From: Yabin Cui Date: Thu, 18 Feb 2016 11:32:10 -0800 Subject: Fix integer overflows in recovery procedure. Bug: 26960931 Change-Id: Ieae45caccfb4728fcf514f0d920976585d8e6caf (cherry picked from commit a029c9a45888141a2fa382e0b1868e55db1f36d2) --- minzip/SysUtil.c | 69 ++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 15 deletions(-) (limited to 'minzip') diff --git a/minzip/SysUtil.c b/minzip/SysUtil.c index b160c9e3d..b1fb4556d 100644 --- a/minzip/SysUtil.c +++ b/minzip/SysUtil.c @@ -3,6 +3,8 @@ * * System utilities. */ +#include +#include #include #include #include @@ -79,6 +81,11 @@ static int sysMapFD(int fd, MemMapping* pMap) pMap->length = length; pMap->range_count = 1; pMap->ranges = malloc(sizeof(MappedRange)); + if (pMap->ranges == NULL) { + LOGE("malloc failed: %s\n", strerror(errno)); + munmap(memPtr, length); + return -1; + } pMap->ranges[0].addr = memPtr; pMap->ranges[0].length = length; @@ -90,7 +97,7 @@ static int sysMapBlockFile(FILE* mapf, MemMapping* pMap) char block_dev[PATH_MAX+1]; size_t size; unsigned int blksize; - unsigned int blocks; + size_t blocks; unsigned int range_count; unsigned int i; @@ -109,49 +116,80 @@ static int sysMapBlockFile(FILE* mapf, MemMapping* pMap) LOGW("failed to parse block map header\n"); return -1; } - - blocks = ((size-1) / blksize) + 1; + if (blksize != 0) { + blocks = ((size-1) / blksize) + 1; + } + if (size == 0 || blksize == 0 || blocks > SIZE_MAX / blksize || range_count == 0) { + LOGE("invalid data in block map file: size %zu, blksize %u, range_count %u\n", + size, blksize, range_count); + return -1; + } pMap->range_count = range_count; - pMap->ranges = malloc(range_count * sizeof(MappedRange)); - memset(pMap->ranges, 0, range_count * sizeof(MappedRange)); + pMap->ranges = calloc(range_count, sizeof(MappedRange)); + if (pMap->ranges == NULL) { + LOGE("calloc(%u, %zu) failed: %s\n", range_count, sizeof(MappedRange), strerror(errno)); + return -1; + } // Reserve enough contiguous address space for the whole file. unsigned char* reserve; reserve = mmap64(NULL, blocks * blksize, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0); if (reserve == MAP_FAILED) { LOGW("failed to reserve address space: %s\n", strerror(errno)); + free(pMap->ranges); return -1; } - pMap->ranges[range_count-1].addr = reserve; - pMap->ranges[range_count-1].length = blocks * blksize; - int fd = open(block_dev, O_RDONLY); if (fd < 0) { LOGW("failed to open block device %s: %s\n", block_dev, strerror(errno)); + munmap(reserve, blocks * blksize); + free(pMap->ranges); return -1; } unsigned char* next = reserve; + size_t remaining_size = blocks * blksize; + bool success = true; for (i = 0; i < range_count; ++i) { - int start, end; - if (fscanf(mapf, "%d %d\n", &start, &end) != 2) { + size_t start, end; + if (fscanf(mapf, "%zu %zu\n", &start, &end) != 2) { LOGW("failed to parse range %d in block map\n", i); - return -1; + success = false; + break; + } + size_t length = (end - start) * blksize; + if (end <= start || (end - start) > SIZE_MAX / blksize || length > remaining_size) { + LOGE("unexpected range in block map: %zu %zu\n", start, end); + success = false; + break; } - void* addr = mmap64(next, (end-start)*blksize, PROT_READ, MAP_PRIVATE | MAP_FIXED, fd, ((off64_t)start)*blksize); + void* addr = mmap64(next, length, PROT_READ, MAP_PRIVATE | MAP_FIXED, fd, ((off64_t)start)*blksize); if (addr == MAP_FAILED) { LOGW("failed to map block %d: %s\n", i, strerror(errno)); - return -1; + success = false; + break; } pMap->ranges[i].addr = addr; - pMap->ranges[i].length = (end-start)*blksize; + pMap->ranges[i].length = length; - next += pMap->ranges[i].length; + next += length; + remaining_size -= length; + } + if (success && remaining_size != 0) { + LOGE("ranges in block map are invalid: remaining_size = %zu\n", remaining_size); + success = false; + } + if (!success) { + close(fd); + munmap(reserve, blocks * blksize); + free(pMap->ranges); + return -1; } + close(fd); pMap->addr = reserve; pMap->length = size; @@ -174,6 +212,7 @@ int sysMapFile(const char* fn, MemMapping* pMap) if (sysMapBlockFile(mapf, pMap) != 0) { LOGW("Map of '%s' failed\n", fn); + fclose(mapf); return -1; } -- cgit v1.2.3