Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ZipFileRO: Use precise widths for zip file types.

Browse Source 
getEntryInfo crashes on 64-bit devices because "long" types
were being passed int pointers (that pointed to a stack frame)
that were reinterpret_cast'ed to long* (sigh.). To fix this issue
once and for all, use types with explicitly defined widths.

This change also removes some dead invariant checking from
Asset.cpp instead of cleaning it up.

Note that we've introduced a wart in NativeLibraryHelper, where
we need to deal with zlib's uLong type, which is "at least 32 bits
wide".

bug: 21622286

Change-Id: Iae675a9601db7aae03a8b80b40321d2cc1d97f50
This commit is contained in:
Narayan Kamath
2015-06-16 12:02:57 +01:00
parent 5e063b1da5
commit 4600dd053d
10 changed files with 49 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cmds/bootanimation/BootAnimation.cpp
View File

@@ -530,7 +530,7 @@ bool BootAnimation::movie()
if (leaf.size() > 0) { if (leaf.size() > 0) {
for (size_t j=0 ; j<pcount ; j++) { for (size_t j=0 ; j<pcount ; j++) {
if (path == animation.parts[j].path) { if (path == animation.parts[j].path) {
int method; uint16_t method;
// supports only stored png files // supports only stored png files
if (mZip->getEntryInfo(entry, &method, NULL, NULL, NULL, NULL, NULL)) { if (mZip->getEntryInfo(entry, &method, NULL, NULL, NULL, NULL, NULL)) {
if (method == ZipFileRO::kCompressStored) { if (method == ZipFileRO::kCompressStored) {

2
cmds/idmap/create.cpp
View File

@@ -23,7 +23,7 @@ namespace {
if (entry == NULL) { if (entry == NULL) {
return -1; return -1;
} }
if (!zip->getEntryInfo(entry, NULL, NULL, NULL, NULL, NULL, reinterpret_cast<long*>(crc))) { if (!zip->getEntryInfo(entry, NULL, NULL, NULL, NULL, NULL, crc)) {
return -1; return -1;
} }
zip->releaseEntry(entry); zip->releaseEntry(entry);

13
cmds/idmap/scan.cpp
View File

@@ -1,4 +1,5 @@
#include <dirent.h> #include <dirent.h>
#include <inttypes.h>
#include <sys/stat.h> #include <sys/stat.h>
#include "idmap.h" #include "idmap.h"
@@ -130,14 +131,14 @@ namespace {
ALOGW("%s: failed to find entry AndroidManifest.xml\n", __FUNCTION__); ALOGW("%s: failed to find entry AndroidManifest.xml\n", __FUNCTION__);
return -1; return -1;
} }
size_t uncompLen = 0; uint32_t uncompLen = 0;
int method; uint16_t method;
if (!zip->getEntryInfo(entry, &method, &uncompLen, NULL, NULL, NULL, NULL)) { if (!zip->getEntryInfo(entry, &method, &uncompLen, NULL, NULL, NULL, NULL)) {
ALOGW("%s: failed to read entry info\n", __FUNCTION__); ALOGW("%s: failed to read entry info\n", __FUNCTION__);
return -1; return -1;
} }
if (method != ZipFileRO::kCompressDeflated) { if (method != ZipFileRO::kCompressDeflated) {
ALOGW("%s: cannot handle zip compression method %d\n", __FUNCTION__, method); ALOGW("%s: cannot handle zip compression method %" PRIu16 "\n", __FUNCTION__, method);
return -1; return -1;
} }
FileMap *dataMap = zip->createEntryFileMap(entry); FileMap *dataMap = zip->createEntryFileMap(entry);
@@ -147,19 +148,19 @@ namespace {
} }
char *buf = new char[uncompLen]; char *buf = new char[uncompLen];
if (NULL == buf) { if (NULL == buf) {
ALOGW("%s: failed to allocate %zd byte\n", __FUNCTION__, uncompLen); ALOGW("%s: failed to allocate %" PRIu32 " byte\n", __FUNCTION__, uncompLen);
delete dataMap; delete dataMap;
return -1; return -1;
} }
StreamingZipInflater inflater(dataMap, uncompLen); StreamingZipInflater inflater(dataMap, uncompLen);
if (inflater.read(buf, uncompLen) < 0) { if (inflater.read(buf, uncompLen) < 0) {
ALOGW("%s: failed to inflate %zd byte\n", __FUNCTION__, uncompLen); ALOGW("%s: failed to inflate %" PRIu32 " byte\n", __FUNCTION__, uncompLen);
delete[] buf; delete[] buf;
delete dataMap; delete dataMap;
return -1; return -1;
} }
int priority = parse_manifest(buf, uncompLen, target_package_name); int priority = parse_manifest(buf, static_cast<size_t>(uncompLen), target_package_name);
delete[] buf; delete[] buf;
delete dataMap; delete dataMap;
return priority; return priority;

27
core/jni/com_android_internal_content_NativeLibraryHelper.cpp
View File

@@ -105,8 +105,8 @@ isFilenameSafe(const char* filename)
} }
static bool static bool
isFileDifferent(const char* filePath, size_t fileSize, time_t modifiedTime, isFileDifferent(const char* filePath, uint32_t fileSize, time_t modifiedTime,
long zipCrc, struct stat64* st) uint32_t zipCrc, struct stat64* st)
{ {
if (lstat64(filePath, st) < 0) { if (lstat64(filePath, st) < 0) {
// File is not found or cannot be read. // File is not found or cannot be read.
@@ -134,7 +134,9 @@ isFileDifferent(const char* filePath, size_t fileSize, time_t modifiedTime,
return true; return true;
} }
long crc = crc32(0L, Z_NULL, 0); // uLong comes from zlib.h. It's a bit of a wart that they're
// potentially using a 64-bit type for a 32-bit CRC.
uLong crc = crc32(0L, Z_NULL, 0);
unsigned char crcBuffer[16384]; unsigned char crcBuffer[16384];
ssize_t numBytes; ssize_t numBytes;
while ((numBytes = TEMP_FAILURE_RETRY(read(fd, crcBuffer, sizeof(crcBuffer)))) > 0) { while ((numBytes = TEMP_FAILURE_RETRY(read(fd, crcBuffer, sizeof(crcBuffer)))) > 0) {
@@ -142,9 +144,9 @@ isFileDifferent(const char* filePath, size_t fileSize, time_t modifiedTime,
} }
close(fd); close(fd);
ALOGV("%s: crc = %lx, zipCrc = %lx\n", filePath, crc, zipCrc); ALOGV("%s: crc = %lx, zipCrc = %" PRIu32 "\n", filePath, crc, zipCrc);
if (crc != zipCrc) { if (crc != static_cast<uLong>(zipCrc)) {
return true; return true;
} }
@@ -155,13 +157,13 @@ static install_status_t
sumFiles(JNIEnv*, void* arg, ZipFileRO* zipFile, ZipEntryRO zipEntry, const char*) sumFiles(JNIEnv*, void* arg, ZipFileRO* zipFile, ZipEntryRO zipEntry, const char*)
{ {
size_t* total = (size_t*) arg; size_t* total = (size_t*) arg;
size_t uncompLen; uint32_t uncompLen;
if (!zipFile->getEntryInfo(zipEntry, NULL, &uncompLen, NULL, NULL, NULL, NULL)) { if (!zipFile->getEntryInfo(zipEntry, NULL, &uncompLen, NULL, NULL, NULL, NULL)) {
return INSTALL_FAILED_INVALID_APK; return INSTALL_FAILED_INVALID_APK;
} }
*total += uncompLen; *total += static_cast<size_t>(uncompLen);
return INSTALL_SUCCEEDED; return INSTALL_SUCCEEDED;
} }
@@ -181,12 +183,11 @@ copyFileIfChanged(JNIEnv *env, void* arg, ZipFileRO* zipFile, ZipEntryRO zipEntr
ScopedUtfChars nativeLibPath(env, *javaNativeLibPath); ScopedUtfChars nativeLibPath(env, *javaNativeLibPath);
size_t uncompLen; uint32_t uncompLen;
long when; uint32_t when;
long crc; uint32_t crc;
time_t modTime;
int method; uint16_t method;
off64_t offset; off64_t offset;
if (!zipFile->getEntryInfo(zipEntry, &method, &uncompLen, NULL, &offset, &when, &crc)) { if (!zipFile->getEntryInfo(zipEntry, &method, &uncompLen, NULL, &offset, &when, &crc)) {
@@ -233,7 +234,7 @@ copyFileIfChanged(JNIEnv *env, void* arg, ZipFileRO* zipFile, ZipEntryRO zipEntr
// Only copy out the native file if it's different. // Only copy out the native file if it's different.
struct tm t; struct tm t;
ZipUtils::zipTimeToTimespec(when, &t); ZipUtils::zipTimeToTimespec(when, &t);
modTime = mktime(&t); const time_t modTime = mktime(&t);
struct stat64 st; struct stat64 st;
if (!isFileDifferent(localFileName, uncompLen, modTime, crc, &st)) { if (!isFileDifferent(localFileName, uncompLen, modTime, crc, &st)) {
return INSTALL_SUCCEEDED; return INSTALL_SUCCEEDED;

7
include/androidfw/Asset.h
View File

@@ -182,11 +182,11 @@ private:
/* /*
* Create the asset from a memory-mapped file segment with compressed * Create the asset from a memory-mapped file segment with compressed
* data. "method" is a Zip archive compression method constant. * data.
* *
* The asset takes ownership of the FileMap. * The asset takes ownership of the FileMap.
*/ */
static Asset* createFromCompressedMap(FileMap* dataMap, int method, static Asset* createFromCompressedMap(FileMap* dataMap,
size_t uncompressedLen, AccessMode mode); size_t uncompressedLen, AccessMode mode);
@@ -286,8 +286,7 @@ public:
* *
* On success, the object takes ownership of "fd". * On success, the object takes ownership of "fd".
*/ */
status_t openChunk(FileMap* dataMap, int compressionMethod, status_t openChunk(FileMap* dataMap, size_t uncompressedLen);
size_t uncompressedLen);
/* /*
* Standard Asset interfaces. * Standard Asset interfaces.

14
include/androidfw/ZipFileRO.h
View File

@@ -35,6 +35,7 @@
#include <utils/FileMap.h> #include <utils/FileMap.h>
#include <utils/threads.h> #include <utils/threads.h>
#include <stdint.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <unistd.h> #include <unistd.h>
@@ -68,9 +69,9 @@ typedef void* ZipEntryRO;
class ZipFileRO { class ZipFileRO {
public: public:
/* Zip compression methods we support */ /* Zip compression methods we support */
enum { enum : uint16_t {
kCompressStored = 0, // no compression kCompressStored = 0,
kCompressDeflated = 8, // standard deflate kCompressDeflated = 8
}; };
/* /*
@@ -111,7 +112,7 @@ public:
* -1 if "entry" is invalid, or the filename length if it didn't fit. The * -1 if "entry" is invalid, or the filename length if it didn't fit. The
* length, and the returned string, include the null-termination. * length, and the returned string, include the null-termination.
*/ */
int getEntryFileName(ZipEntryRO entry, char* buffer, int bufLen) const; int getEntryFileName(ZipEntryRO entry, char* buffer, size_t bufLen) const;
/* /*
* Get the vital stats for an entry. Pass in NULL pointers for anything * Get the vital stats for an entry. Pass in NULL pointers for anything
@@ -122,8 +123,9 @@ public:
* Returns "false" if "entry" is bogus or if the data in the Zip file * Returns "false" if "entry" is bogus or if the data in the Zip file
* appears to be bad. * appears to be bad.
*/ */
bool getEntryInfo(ZipEntryRO entry, int* pMethod, size_t* pUncompLen, bool getEntryInfo(ZipEntryRO entry, uint16_t* pMethod, uint32_t* pUncompLen,
size_t* pCompLen, off64_t* pOffset, long* pModWhen, long* pCrc32) const; uint32_t* pCompLen, off64_t* pOffset, uint32_t* pModWhen,
uint32_t* pCrc32) const;
/* /*
* Create a new FileMap object that maps a subset of the archive. For * Create a new FileMap object that maps a subset of the archive. For

5
include/androidfw/ZipUtils.h
View File

@@ -20,6 +20,7 @@
#ifndef __LIBS_ZIPUTILS_H #ifndef __LIBS_ZIPUTILS_H
#define __LIBS_ZIPUTILS_H #define __LIBS_ZIPUTILS_H
#include <stdint.h>
#include <stdio.h> #include <stdio.h>
#include <time.h> #include <time.h>
@@ -63,8 +64,8 @@ public:
/* /*
* Utility function to convert ZIP's time format to a timespec struct. * Utility function to convert ZIP's time format to a timespec struct.
*/ */
static inline void zipTimeToTimespec(long when, struct tm* timespec) { static inline void zipTimeToTimespec(uint32_t when, struct tm* timespec) {
const long date = when >> 16; const uint32_t date = when >> 16;
timespec->tm_year = ((date >> 9) & 0x7F) + 80; // Zip is years since 1980 timespec->tm_year = ((date >> 9) & 0x7F) + 80; // Zip is years since 1980
timespec->tm_mon = (date >> 5) & 0x0F; timespec->tm_mon = (date >> 5) & 0x0F;
timespec->tm_mday = date & 0x1F; timespec->tm_mday = date & 0x1F;

12
libs/androidfw/Asset.cpp
View File

@@ -296,13 +296,13 @@ Asset::~Asset(void)
* Create a new Asset from compressed data in a memory mapping. * Create a new Asset from compressed data in a memory mapping.
*/ */
/*static*/ Asset* Asset::createFromCompressedMap(FileMap* dataMap, /*static*/ Asset* Asset::createFromCompressedMap(FileMap* dataMap,
int method, size_t uncompressedLen, AccessMode mode) size_t uncompressedLen, AccessMode mode)
{ {
_CompressedAsset* pAsset; _CompressedAsset* pAsset;
status_t result; status_t result;
pAsset = new _CompressedAsset; pAsset = new _CompressedAsset;
result = pAsset->openChunk(dataMap, method, uncompressedLen); result = pAsset->openChunk(dataMap, uncompressedLen);
if (result != NO_ERROR) if (result != NO_ERROR)
return NULL; return NULL;
@@ -734,18 +734,12 @@ status_t _CompressedAsset::openChunk(int fd, off64_t offset,
* *
* Nothing is expanded until the first read call. * Nothing is expanded until the first read call.
*/ */
status_t _CompressedAsset::openChunk(FileMap* dataMap, int compressionMethod, status_t _CompressedAsset::openChunk(FileMap* dataMap, size_t uncompressedLen)
size_t uncompressedLen)
{ {
assert(mFd < 0); // no re-open assert(mFd < 0); // no re-open
assert(mMap == NULL); assert(mMap == NULL);
assert(dataMap != NULL); assert(dataMap != NULL);
if (compressionMethod != ZipFileRO::kCompressDeflated) {
assert(false);
return UNKNOWN_ERROR;
}
mMap = dataMap; mMap = dataMap;
mStart = -1; // not used mStart = -1; // not used
mCompressedLen = dataMap->getDataLength(); mCompressedLen = dataMap->getDataLength();

10
libs/androidfw/AssetManager.cpp
View File

@@ -1156,13 +1156,11 @@ Asset* AssetManager::openAssetFromZipLocked(const ZipFileRO* pZipFile,
Asset* pAsset = NULL; Asset* pAsset = NULL;
// TODO: look for previously-created shared memory slice? // TODO: look for previously-created shared memory slice?
int method; uint16_t method;
size_t uncompressedLen; uint32_t uncompressedLen;
//printf("USING Zip '%s'\n", pEntry->getFileName()); //printf("USING Zip '%s'\n", pEntry->getFileName());
//pZipFile->getEntryInfo(entry, &method, &uncompressedLen, &compressedLen,
// &offset);
if (!pZipFile->getEntryInfo(entry, &method, &uncompressedLen, NULL, NULL, if (!pZipFile->getEntryInfo(entry, &method, &uncompressedLen, NULL, NULL,
NULL, NULL)) NULL, NULL))
{ {
@@ -1181,8 +1179,8 @@ Asset* AssetManager::openAssetFromZipLocked(const ZipFileRO* pZipFile,
ALOGV("Opened uncompressed entry %s in zip %s mode %d: %p", entryName.string(), ALOGV("Opened uncompressed entry %s in zip %s mode %d: %p", entryName.string(),
dataMap->getFileName(), mode, pAsset); dataMap->getFileName(), mode, pAsset);
} else { } else {
pAsset = Asset::createFromCompressedMap(dataMap, method, pAsset = Asset::createFromCompressedMap(dataMap,
uncompressedLen, mode); static_cast<size_t>(uncompressedLen), mode);
ALOGV("Opened compressed entry %s in zip %s mode %d: %p", entryName.string(), ALOGV("Opened compressed entry %s in zip %s mode %d: %p", entryName.string(),
dataMap->getFileName(), mode, pAsset); dataMap->getFileName(), mode, pAsset);
} }

7
libs/androidfw/ZipFileRO.cpp
View File

@@ -97,8 +97,9 @@ ZipEntryRO ZipFileRO::findEntryByName(const char* entryName) const
* Returns "false" if the offsets to the fields or the contents of the fields * Returns "false" if the offsets to the fields or the contents of the fields
* appear to be bogus. * appear to be bogus.
*/ */
bool ZipFileRO::getEntryInfo(ZipEntryRO entry, int* pMethod, size_t* pUncompLen, bool ZipFileRO::getEntryInfo(ZipEntryRO entry, uint16_t* pMethod,
size_t* pCompLen, off64_t* pOffset, long* pModWhen, long* pCrc32) const uint32_t* pUncompLen, uint32_t* pCompLen, off64_t* pOffset,
uint32_t* pModWhen, uint32_t* pCrc32) const
{ {
const _ZipEntryRO* zipEntry = reinterpret_cast<_ZipEntryRO*>(entry); const _ZipEntryRO* zipEntry = reinterpret_cast<_ZipEntryRO*>(entry);
const ZipEntry& ze = zipEntry->entry; const ZipEntry& ze = zipEntry->entry;
@@ -166,7 +167,7 @@ void ZipFileRO::releaseEntry(ZipEntryRO entry) const
/* /*
* Copy the entry's filename to the buffer. * Copy the entry's filename to the buffer.
*/ */
int ZipFileRO::getEntryFileName(ZipEntryRO entry, char* buffer, int bufLen) int ZipFileRO::getEntryFileName(ZipEntryRO entry, char* buffer, size_t bufLen)
const const
{ {
const _ZipEntryRO* zipEntry = reinterpret_cast<_ZipEntryRO*>(entry); const _ZipEntryRO* zipEntry = reinterpret_cast<_ZipEntryRO*>(entry);