Restore support for DEX on FAT.

The recent change to mmap(read-write)+mprotect(read-only) doesn't seem
to work on FAT filesystems like /sdcard.  This caused problems for the
code that opens Zip files and the code that opens DEX files.

This change splits the "map file" function into "read only" and
"writable read only" versions, using the former for Zip and the latter
for DEX.  Further, failure to mprotect(read-only) is now considered a
soft failure and only causes a warning.

The only apps that will be affected by this are those using /sdcard to
hold optimized DEX data for "plugin" APKs.

Also: moved the non-HAVE_POSIX_FILEMAP implementation of file mapping
into a shared function.  (Could probably go away entirely.)

Also: fixed the expected output for test 071.

6 files changed, 80 insertions, 32 deletions

diff --git a/libdex/CmdUtils.c b/libdex/CmdUtils.c
index 7dfee87cc..102664cd5 100644
--- a/libdex/CmdUtils.c
+++ b/libdex/CmdUtils.c
@@ -162,7 +162,7 @@ UnzipToFileResult dexOpenAndMap(const char* fileName, const char* tempFileName,
         goto bail;
     }
 
-    if (sysMapFileInShmem(fd, pMap) != 0) {
+    if (sysMapFileInShmemReadOnly(fd, pMap) != 0) {
         fprintf(stderr, "ERROR: Unable to map %s\n", fileName);
         close(fd);
         goto bail;
diff --git a/libdex/SysUtil.c b/libdex/SysUtil.c
index 08dc67c8b..eaa612b68 100644
--- a/libdex/SysUtil.c
+++ b/libdex/SysUtil.c
@@ -160,7 +160,46 @@ int sysLoadFileInShmem(int fd, MemMapping* pMap)
  * On success, returns 0 and fills out "pMap".  On failure, returns a nonzero
  * value and does not disturb "pMap".
  */
-int sysMapFileInShmem(int fd, MemMapping* pMap)
+int sysMapFileInShmemReadOnly(int fd, MemMapping* pMap)
+{
+#ifdef HAVE_POSIX_FILEMAP
+    off_t start;
+    size_t length;
+    void* memPtr;
+
+    assert(pMap != NULL);
+
+    if (getFileStartAndLength(fd, &start, &length) < 0)
+        return -1;
+
+    memPtr = mmap(NULL, length, PROT_READ, MAP_FILE | MAP_SHARED, fd, start);
+    if (memPtr == MAP_FAILED) {
+        LOGW("mmap(%d, RO, FILE|SHARED, %d, %d) failed: %s\n", (int) length,
+            fd, (int) start, strerror(errno));
+        return -1;
+    }
+
+    pMap->baseAddr = pMap->addr = memPtr;
+    pMap->baseLength = pMap->length = length;
+
+    return 0;
+#else
+    sysFakeMapFile(fd, pMap);
+#endif
+}
+
+/*
+ * Map a file (from fd's current offset) into a private, read-write memory
+ * segment that will be marked read-only (a/k/a "writable read-only").  The
+ * file offset must be a multiple of the system page size.
+ *
+ * In some cases the mapping will be fully writable (e.g. for files on
+ * FAT filesystems).
+ *
+ * On success, returns 0 and fills out "pMap".  On failure, returns a nonzero
+ * value and does not disturb "pMap".
+ */
+int sysMapFileInShmemWritableReadOnly(int fd, MemMapping* pMap)
 {
 #ifdef HAVE_POSIX_FILEMAP
     off_t start;
@@ -172,13 +211,6 @@ int sysMapFileInShmem(int fd, MemMapping* pMap)
     if (getFileStartAndLength(fd, &start, &length) < 0)
         return -1;
 
-    /*
-     * This was originally (PROT_READ, MAP_SHARED), but we want to be able
-     * to make local edits for verification errors and debugger breakpoints.
-     * So we map it read-write and private, but use mprotect to mark the
-     * pages read-only.  This should yield identical results so long as the
-     * pages are left read-only.
-     */
     memPtr = mmap(NULL, length, PROT_READ | PROT_WRITE, MAP_FILE | MAP_PRIVATE,
             fd, start);
     if (memPtr == MAP_FAILED) {
@@ -187,10 +219,11 @@ int sysMapFileInShmem(int fd, MemMapping* pMap)
         return -1;
     }
     if (mprotect(memPtr, length, PROT_READ) < 0) {
-        LOGW("mprotect(%p, %d, PROT_READ) failed: %s\n",
-            memPtr, length, strerror(errno));
-        (void) munmap(memPtr, length);
-        return -1;
+        /* this fails with EACCESS on FAT filesystems, e.g. /sdcard */
+        int err = errno;
+        LOGV("mprotect(%p, %d, PROT_READ) failed: %s\n",
+            memPtr, length, strerror(err));
+        LOGD("mprotect(RO) failed (%d), file will remain read-write\n", err);
     }
 
     pMap->baseAddr = pMap->addr = memPtr;
@@ -198,6 +231,13 @@ int sysMapFileInShmem(int fd, MemMapping* pMap)
 
     return 0;
 #else
+    sysFakeMapFile(fd, pMap);
+#endif
+}
+
+#ifndef HAVE_POSIX_FILEMAP
+int sysFakeMapFile(int fd, MemMapping* pMap)
+{
     /* No MMAP, just fake it by copying the bits.
        For Win32 we could use MapViewOfFile if really necessary
        (see libs/utils/FileMap.cpp).
@@ -222,8 +262,8 @@ int sysMapFileInShmem(int fd, MemMapping* pMap)
     pMap->baseLength = pMap->length = length;
 
     return 0;
-#endif
 }
+#endif
 
 /*
  * Map part of a file (from fd's current offset) into a shared, read-only
@@ -314,7 +354,7 @@ int sysChangeMapAccess(void* addr, size_t length, int wantReadWrite,
     int prot = wantReadWrite ? (PROT_READ|PROT_WRITE) : (PROT_READ);
     if (mprotect(alignAddr, alignLength, prot) != 0) {
         int err = errno;
-        LOGW("mprotect (%p,%zd,%d) failed: %s\n",
+        LOGV("mprotect (%p,%zd,%d) failed: %s\n",
             alignAddr, alignLength, prot, strerror(errno));
         return (errno != 0) ? errno : -1;
     }
diff --git a/libdex/SysUtil.h b/libdex/SysUtil.h
index 6ee352b80..b300a7b3a 100644
--- a/libdex/SysUtil.h
+++ b/libdex/SysUtil.h
@@ -66,10 +66,19 @@ int sysLoadFileInShmem(int fd, MemMapping* pMap);
  *
  * On success, "pMap" is filled in, and zero is returned.
  */
-int sysMapFileInShmem(int fd, MemMapping* pMap);
+int sysMapFileInShmemReadOnly(int fd, MemMapping* pMap);
 
 /*
- * Like sysMapFileInShmem, but on only part of a file.
+ * Map a file (from fd's current offset) into a shared, read-only memory
+ * segment that can be made writable.  (In some cases, such as when
+ * mapping a file on a FAT filesystem, the result may be fully writable.)
+ *
+ * On success, "pMap" is filled in, and zero is returned.
+ */
+int sysMapFileInShmemWritableReadOnly(int fd, MemMapping* pMap);
+
+/*
+ * Like sysMapFileInShmemReadOnly, but on only part of a file.
  */
 int sysMapFileSegmentInShmem(int fd, off_t start, long length,
     MemMapping* pMap);
diff --git a/libdex/ZipArchive.c b/libdex/ZipArchive.c
index 3f88e7d4b..7c7e18e95 100644
--- a/libdex/ZipArchive.c
+++ b/libdex/ZipArchive.c
@@ -310,7 +310,7 @@ int dexZipPrepArchive(int fd, const char* debugFileName, ZipArchive* pArchive)
 
     pArchive->mFd = fd;
 
-    if (sysMapFileInShmem(pArchive->mFd, &map) != 0) {
+    if (sysMapFileInShmemReadOnly(pArchive->mFd, &map) != 0) {
         err = -1;
         LOGW("Map of '%s' failed\n", debugFileName);
         goto bail;
diff --git a/tests/071-dexfile/expected.txt b/tests/071-dexfile/expected.txt
index 8aa4061d3..b7af75ed2 100644
--- a/tests/071-dexfile/expected.txt
+++ b/tests/071-dexfile/expected.txt
@@ -1,4 +1,3 @@
-My path is: test-ex.jar
 Constructing another
 Got expected ULE
 done
diff --git a/vm/DvmDex.c b/vm/DvmDex.c
index 20a4376e6..258d768d2 100644
--- a/vm/DvmDex.c
+++ b/vm/DvmDex.c
@@ -130,7 +130,7 @@ int dvmDexFileOpenFromFd(int fd, DvmDex** ppDvmDex)
         goto bail;
     }
 
-    if (sysMapFileInShmem(fd, &memMap) != 0) {
+    if (sysMapFileInShmemWritableReadOnly(fd, &memMap) != 0) {
         LOGE("Unable to map file\n");
         goto bail;
     }
@@ -245,16 +245,16 @@ bool dvmDexChangeDex1(DvmDex* pDvmDex, u1* addr, u1 newVal)
     }
 
     LOGV("+++ change byte at %p from 0x%02x to 0x%02x\n", addr, *addr, newVal);
-    if (sysChangeMapAccess(addr, 1, true, &pDvmDex->memMap) < 0) {
-        LOGE("access change failed\n");
-        return false;
+    if (sysChangeMapAccess(addr, 1, true, &pDvmDex->memMap) != 0) {
+        LOGD("NOTE: DEX page access change (->RW) failed\n");
+        /* expected on files mounted from FAT; keep going (may crash) */
     }
 
     *addr = newVal;
 
-    if (sysChangeMapAccess(addr, 1, false, &pDvmDex->memMap) < 0) {
-        LOGW("WARNING: unable to restore read-only access on mapping\n");
-        /* not fatal, keep going */
+    if (sysChangeMapAccess(addr, 1, false, &pDvmDex->memMap) != 0) {
+        LOGD("NOTE: DEX page access change (->RO) failed\n");
+        /* expected on files mounted from FAT; keep going */
     }
 
     return true;
@@ -274,16 +274,16 @@ bool dvmDexChangeDex2(DvmDex* pDvmDex, u2* addr, u2 newVal)
     }
 
     LOGV("+++ change 2byte at %p from 0x%04x to 0x%04x\n", addr, *addr, newVal);
-    if (sysChangeMapAccess(addr, 2, true, &pDvmDex->memMap) < 0) {
-        LOGE("access change failed\n");
-        return false;
+    if (sysChangeMapAccess(addr, 2, true, &pDvmDex->memMap) != 0) {
+        LOGD("NOTE: DEX page access change (->RW) failed\n");
+        /* expected on files mounted from FAT; keep going (may crash) */
     }
 
     *addr = newVal;
 
-    if (sysChangeMapAccess(addr, 2, false, &pDvmDex->memMap) < 0) {
-        LOGW("WARNING: unable to restore read-only access on mapping\n");
-        /* not fatal, keep going */
+    if (sysChangeMapAccess(addr, 2, false, &pDvmDex->memMap) != 0) {
+        LOGD("NOTE: DEX page access change (->RO) failed\n");
+        /* expected on files mounted from FAT; keep going */
     }
 
     return true;