diff options
| -rw-r--r-- | include/utils/ZipFileRO.h | 24 | ||||
| -rw-r--r-- | libs/utils/ZipFileRO.cpp | 33 |
2 files changed, 49 insertions, 8 deletions
diff --git a/include/utils/ZipFileRO.h b/include/utils/ZipFileRO.h index e1ff780..3c1f3ca 100644 --- a/include/utils/ZipFileRO.h +++ b/include/utils/ZipFileRO.h @@ -14,13 +14,19 @@ * limitations under the License. */ -// -// Read-only access to Zip archives, with minimal heap allocation. -// -// This is similar to the more-complete ZipFile class, but no attempt -// has been made to make them interchangeable. This class operates under -// a very different set of assumptions and constraints. -// +/* + * Read-only access to Zip archives, with minimal heap allocation. + * + * This is similar to the more-complete ZipFile class, but no attempt + * has been made to make them interchangeable. This class operates under + * a very different set of assumptions and constraints. + * + * One such assumption is that if you're getting file descriptors for + * use with this class as a child of a fork() operation, you must be on + * a pread() to guarantee correct operation. This is because pread() can + * atomically read at a file offset without worrying about a lock around an + * lseek() + read() pair. + */ #ifndef __LIBS_ZIPFILERO_H #define __LIBS_ZIPFILERO_H @@ -55,6 +61,10 @@ typedef void* ZipEntryRO; * the record structure. However, this requires a private mapping of * every page that the Central Directory touches. Easier to tuck a copy * of the string length into the hash table entry. + * + * NOTE: If this is used on file descriptors inherited from a fork() operation, + * you must be on a platform that implements pread() to guarantee correctness + * on the shared file descriptors. */ class ZipFileRO { public: diff --git a/libs/utils/ZipFileRO.cpp b/libs/utils/ZipFileRO.cpp index bee86b2..9b1f82f 100644 --- a/libs/utils/ZipFileRO.cpp +++ b/libs/utils/ZipFileRO.cpp @@ -508,6 +508,36 @@ bool ZipFileRO::getEntryInfo(ZipEntryRO entry, int* pMethod, size_t* pUncompLen, unsigned char lfhBuf[kLFHLen]; +#ifdef HAVE_PREAD + /* + * This file descriptor might be from zygote's preloaded assets, + * so we need to do an pread() instead of a lseek() + read() to + * guarantee atomicity across the processes with the shared file + * descriptors. + */ + ssize_t actual = + TEMP_FAILURE_RETRY(pread(mFd, lfhBuf, sizeof(lfhBuf), localHdrOffset)); + + if (actual != sizeof(lfhBuf)) { + LOGW("failed reading lfh from offset %ld\n", localHdrOffset); + return false; + } + + if (get4LE(lfhBuf) != kLFHSignature) { + LOGW("didn't find signature at start of lfh; wanted: offset=%ld data=0x%08x; " + "got: data=0x%08lx\n", + localHdrOffset, kLFHSignature, get4LE(lfhBuf)); + return false; + } +#else /* HAVE_PREAD */ + /* + * For hosts don't have pread() we cannot guarantee atomic reads from + * an offset in a file. Android should never run on those platforms. + * File descriptors inherited from a fork() share file offsets and + * there would be nothing to protect from two different processes + * calling lseek() concurrently. + */ + { AutoMutex _l(mFdLock); @@ -517,7 +547,7 @@ bool ZipFileRO::getEntryInfo(ZipEntryRO entry, int* pMethod, size_t* pUncompLen, } ssize_t actual = - TEMP_FAILURE_RETRY(read(mFd, lfhBuf, sizeof(lfhBuf))); + TEMP_FAILURE_RETRY(read(mFd, lfhBuf, sizeof(lfhBuf))); if (actual != sizeof(lfhBuf)) { LOGW("failed reading lfh from offset %ld\n", localHdrOffset); return false; @@ -531,6 +561,7 @@ bool ZipFileRO::getEntryInfo(ZipEntryRO entry, int* pMethod, size_t* pUncompLen, return false; } } +#endif /* HAVE_PREAD */ off_t dataOffset = localHdrOffset + kLFHLen + get2LE(lfhBuf + kLFHNameLen) + get2LE(lfhBuf + kLFHExtraLen); |