From dbf6f272a2cad97f7ce7fa600ffd2220483c80d1 Mon Sep 17 00:00:00 2001
From: Kenny Root <kroot@google.com>
Date: Fri, 1 Oct 2010 18:28:28 -0700
Subject: [PATCH] ZipFileRO: moar logging and wrap close

There is apparently still a race upon reading the entry Local File
Header that can't be tracked down, so move the LFH check inside the
mutex-protected block so we can call lseek again to see where we are
when we log an error.

Also, close() can fail so use TEMP_FAILURE_RETRY on it so we don't
unwittingly leak file descriptors when Mean Mr. EINTR comes a-knocking.

Change-Id: I753abad0bd882fe28f7281c406fa76f64393ef4c
---
 include/utils/ZipFileRO.h | 11 ++---------
 libs/utils/ZipFileRO.cpp  | 26 +++++++++++++++++++-------
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/include/utils/ZipFileRO.h b/include/utils/ZipFileRO.h
index 9668bdeef..e1ff780ab 100644
--- a/include/utils/ZipFileRO.h
+++ b/include/utils/ZipFileRO.h
@@ -64,15 +64,8 @@ public:
           mNumEntries(-1), mDirectoryOffset(-1),
           mHashTableSize(-1), mHashTable(NULL)
         {}
-    ~ZipFileRO() {
-        free(mHashTable);
-        if (mDirectoryMap)
-            mDirectoryMap->release();
-        if (mFd >= 0)
-            close(mFd);
-        if (mFileName)
-            free(mFileName);
-    }
+
+    ~ZipFileRO();
 
     /*
      * Open an archive.
diff --git a/libs/utils/ZipFileRO.cpp b/libs/utils/ZipFileRO.cpp
index 9fcae7238..bee86b2bb 100644
--- a/libs/utils/ZipFileRO.cpp
+++ b/libs/utils/ZipFileRO.cpp
@@ -86,6 +86,16 @@ using namespace android;
  */
 #define kZipEntryAdj        10000
 
+ZipFileRO::~ZipFileRO() {
+    free(mHashTable);
+    if (mDirectoryMap)
+        mDirectoryMap->release();
+    if (mFd >= 0)
+        TEMP_FAILURE_RETRY(close(mFd));
+    if (mFileName)
+        free(mFileName);
+}
+
 /*
  * Convert a ZipEntryRO to a hash table index, verifying that it's in a
  * valid range.
@@ -122,7 +132,7 @@ status_t ZipFileRO::open(const char* zipFileName)
 
     mFileLength = lseek(fd, 0, SEEK_END);
     if (mFileLength < kEOCDLen) {
-        close(fd);
+        TEMP_FAILURE_RETRY(close(fd));
         return UNKNOWN_ERROR;
     }
 
@@ -152,7 +162,7 @@ status_t ZipFileRO::open(const char* zipFileName)
 bail:
     free(mFileName);
     mFileName = NULL;
-    close(fd);
+    TEMP_FAILURE_RETRY(close(fd));
     return UNKNOWN_ERROR;
 }
 
@@ -512,12 +522,14 @@ bool ZipFileRO::getEntryInfo(ZipEntryRO entry, int* pMethod, size_t* pUncompLen,
                 LOGW("failed reading lfh from offset %ld\n", localHdrOffset);
                 return false;
             }
-        }
 
-        if (get4LE(lfhBuf) != kLFHSignature) {
-            LOGW("didn't find signature at start of lfh, offset=%ld (got 0x%08lx, expected 0x%08x)\n",
-                localHdrOffset, get4LE(lfhBuf), kLFHSignature);
-            return false;
+            if (get4LE(lfhBuf) != kLFHSignature) {
+                off_t actualOffset = lseek(mFd, 0, SEEK_CUR);
+                LOGW("didn't find signature at start of lfh; wanted: offset=%ld data=0x%08x; "
+                        "got: offset=%zd data=0x%08lx\n",
+                        localHdrOffset, kLFHSignature, (size_t)actualOffset, get4LE(lfhBuf));
+                return false;
+            }
         }
 
         off_t dataOffset = localHdrOffset + kLFHLen