Use pread() in ZipFileRO for Linux

AssetManager instances are created by zygote and passed to all its
children so that they don't have to individually open
frameworks-res.apk. This creates a problem for determining the current
file offset when using lseek() on those files, because you can't
guarantee the cross-process locking of a mutex. Luckily, Linux
implements pread() to get around this suckiness.

The problem is that only Linux implements this, so we have to keep the
old locking for use on host builds with aapt and friends. aapt doesn't
have this same problem of sharing file descriptors across forked
processes, so we can keep the local AutoMutex to protect accesses of
those files.

Change-Id: Ibe9f11499a53fe345f50fbaea438815ec0fd363e

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);