Fix DexFile cookie check.

We use a "cookie" rather than just accepting a raw pointer so that the
VM doesn't crash if something weird gets passed in.  Unfortunately the
cookie verify function was dereferencing the cookie to compute a value
for the hash table lookup, defeating the purpose.  Since we don't deal
with opening the same DEX file in a constructive way, we can just take
the low part of the pointer itself and use that as the hash.  (Hashing
isn't really necessary given the number of DEX files we expect to have
open; mostly it's just a convenient synchronized data structure.)

Bug 3238298

Change-Id: I1a545d4154b58159acdc21809b55b098c1419644

diff --git a/vm/native/dalvik_system_DexFile.c b/vm/native/dalvik_system_DexFile.c
index 25c9dfb..19ac492 100644 (file)
--- a/vm/native/dalvik_system_DexFile.c
+++ b/vm/native/dalvik_system_DexFile.c
@@ -75,7 +75,7 @@ static bool validateCookie(int cookie)
     if (pDexOrJar == NULL)
         return false;
 
-    u4 hash = dvmComputeUtf8Hash(pDexOrJar->fileName);
+    u4 hash = cookie;
     dvmHashTableLock(gDvm.userDexFiles);
     void* result = dvmHashTableLookup(gDvm.userDexFiles, hash, pDexOrJar,
                 hashcmpDexOrJar, false);
@@ -105,6 +105,8 @@ static bool validateCookie(int cookie)
  * To optimize this away we could search for existing entries in the hash
  * table and refCount them.  Requires atomic ops or adding "synchronized"
  * to the non-native code that calls here.
+ *
+ * TODO: should be using "long" for a pointer.
  */
 static void Dalvik_dalvik_system_DexFile_openDexFile(const u4* args,
     JValue* pResult)
@@ -181,8 +183,17 @@ static void Dalvik_dalvik_system_DexFile_openDexFile(const u4* args,
     if (pDexOrJar != NULL) {
         pDexOrJar->fileName = sourceName;
 
-        /* add to hash table */
-        u4 hash = dvmComputeUtf8Hash(sourceName);
+        /*
+         * Add to hash table.
+         *
+         * Later on we will receive this pointer as an argument and need
+         * to find it in the hash table without knowing if it's valid or
+         * not, which means we can't compute a hash value from anything
+         * inside DexOrJar.  We don't share DexOrJar structs when the same
+         * file is opened multiple times, so we can just use the low 32
+         * bits of the pointer as the hash.
+         */
+        u4 hash = (u4) pDexOrJar;
         void* result;
         dvmHashTableLock(gDvm.userDexFiles);
         result = dvmHashTableLookup(gDvm.userDexFiles, hash, pDexOrJar,
@@ -213,12 +224,11 @@ static void Dalvik_dalvik_system_DexFile_closeDexFile(const u4* args,
 
     if (pDexOrJar == NULL)
         RETURN_VOID();
-
-    LOGV("Closing DEX file %p (%s)\n", pDexOrJar, pDexOrJar->fileName);
-
     if (!validateCookie(cookie))
         RETURN_VOID();
 
+    LOGV("Closing DEX file %p (%s)\n", pDexOrJar, pDexOrJar->fileName);
+
     /*
      * We can't just free arbitrary DEX files because they have bits and
      * pieces of loaded classes.  The only exception to this rule is if
@@ -228,7 +238,7 @@ static void Dalvik_dalvik_system_DexFile_closeDexFile(const u4* args,
      * them when the VM shuts down.
      */
     if (pDexOrJar->okayToFree) {
-        u4 hash = dvmComputeUtf8Hash(pDexOrJar->fileName);
+        u4 hash = (u4) pDexOrJar;
         dvmHashTableLock(gDvm.userDexFiles);
         if (!dvmHashTableRemove(gDvm.userDexFiles, hash, pDexOrJar)) {
             LOGW("WARNING: could not remove '%s' from DEX hash table\n",