Merge "Fix native crashes when APKs can't be opened."

3 files changed, 73 insertions, 44 deletions

diff --git a/core/java/com/android/internal/content/NativeLibraryHelper.java b/core/java/com/android/internal/content/NativeLibraryHelper.java
index dab3aff..832829d 100644
--- a/core/java/com/android/internal/content/NativeLibraryHelper.java
+++ b/core/java/com/android/internal/content/NativeLibraryHelper.java
@@ -19,6 +19,7 @@ package com.android.internal.content;
 import android.content.pm.PackageManager;
 import android.util.Slog;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 
@@ -39,20 +40,29 @@ public class NativeLibraryHelper {
      *
      * @hide
      */
-    public static class ApkHandle {
+    public static class ApkHandle implements Closeable {
         final String apkPath;
         final long apkHandle;
 
-        public ApkHandle(String path) {
-            apkPath = path;
-            apkHandle = nativeOpenApk(apkPath);
+        public static ApkHandle create(String path) throws IOException {
+            final long handle = nativeOpenApk(path);
+            if (handle == 0) {
+                throw new IOException("Unable to open APK: " + path);
+            }
+
+            return new ApkHandle(path, handle);
+        }
+
+        public static ApkHandle create(File path) throws IOException {
+            return create(path.getAbsolutePath());
         }
 
-        public ApkHandle(File apkFile) {
-            apkPath = apkFile.getPath();
-            apkHandle = nativeOpenApk(apkPath);
+        private ApkHandle(String apkPath, long apkHandle) {
+            this.apkPath = apkPath;
+            this.apkHandle = apkHandle;
         }
 
+        @Override
         public void close() {
             nativeClose(apkHandle);
         }
diff --git a/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java b/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java
index ec87c6e..16a0d35 100644
--- a/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java
+++ b/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java
@@ -49,6 +49,7 @@ import android.util.Slog;
 
 import com.android.internal.app.IMediaContainerService;
 import com.android.internal.content.NativeLibraryHelper;
+import com.android.internal.content.NativeLibraryHelper.ApkHandle;
 import com.android.internal.content.PackageHelper;
 
 import java.io.BufferedInputStream;
@@ -106,8 +107,27 @@ public class DefaultContainerService extends IntentService {
                 return null;
             }
 
-            return copyResourceInner(packageURI, cid, key, resFileName, publicResFileName,
-                    isExternal, isForwardLocked, abiOverride);
+
+            if (isExternal) {
+                // Make sure the sdcard is mounted.
+                String status = Environment.getExternalStorageState();
+                if (!status.equals(Environment.MEDIA_MOUNTED)) {
+                    Slog.w(TAG, "Make sure sdcard is mounted.");
+                    return null;
+                }
+            }
+
+            ApkHandle handle = null;
+            try {
+                handle = ApkHandle.create(packageURI.getPath());
+                return copyResourceInner(packageURI, cid, key, resFileName, publicResFileName,
+                        isExternal, isForwardLocked, handle, abiOverride);
+            } catch (IOException ioe) {
+                Slog.w(TAG, "Problem opening APK: " + packageURI.getPath());
+                return null;
+            } finally {
+                IoUtils.closeQuietly(handle);
+            }
         }
 
         /**
@@ -328,21 +348,11 @@ public class DefaultContainerService extends IntentService {
 
     private String copyResourceInner(Uri packageURI, String newCid, String key, String resFileName,
             String publicResFileName, boolean isExternal, boolean isForwardLocked,
-            String abiOverride) {
-
-        if (isExternal) {
-            // Make sure the sdcard is mounted.
-            String status = Environment.getExternalStorageState();
-            if (!status.equals(Environment.MEDIA_MOUNTED)) {
-                Slog.w(TAG, "Make sure sdcard is mounted.");
-                return null;
-            }
-        }
-
+            ApkHandle handle, String abiOverride) {
         // The .apk file
         String codePath = packageURI.getPath();
         File codeFile = new File(codePath);
-        NativeLibraryHelper.ApkHandle handle = new NativeLibraryHelper.ApkHandle(codePath);
+
         String[] abiList = Build.SUPPORTED_ABIS;
         if (abiOverride != null) {
             abiList = new String[] { abiOverride };
@@ -849,14 +859,14 @@ public class DefaultContainerService extends IntentService {
 
     private int calculateContainerSize(File apkFile, boolean forwardLocked,
             String abiOverride) throws IOException {
-        NativeLibraryHelper.ApkHandle handle = new NativeLibraryHelper.ApkHandle(apkFile);
-        final int abi = NativeLibraryHelper.findSupportedAbi(handle,
-                (abiOverride != null) ? new String[] { abiOverride } : Build.SUPPORTED_ABIS);
-
+        ApkHandle handle = null;
         try {
+            handle = ApkHandle.create(apkFile);
+            final int abi = NativeLibraryHelper.findSupportedAbi(handle,
+                    (abiOverride != null) ? new String[] { abiOverride } : Build.SUPPORTED_ABIS);
             return calculateContainerSize(handle, apkFile, abi, forwardLocked);
         } finally {
-            handle.close();
+            IoUtils.closeQuietly(handle);
         }
     }
 
diff --git a/services/java/com/android/server/pm/PackageManagerService.java b/services/java/com/android/server/pm/PackageManagerService.java
index 2d5e357..273813d 100755
--- a/services/java/com/android/server/pm/PackageManagerService.java
+++ b/services/java/com/android/server/pm/PackageManagerService.java
@@ -5019,8 +5019,9 @@ public class PackageManagerService extends IPackageManager.Stub {
          *        only for non-system apps and system app upgrades.
          */
         if (pkg.applicationInfo.nativeLibraryDir != null) {
-            final NativeLibraryHelper.ApkHandle handle = new NativeLibraryHelper.ApkHandle(scanFile);
+            ApkHandle handle = null;
             try {
+                handle = ApkHandle.create(scanFile.getPath());
                 // Enable gross and lame hacks for apps that are built with old
                 // SDK tools. We must scan their APKs for renderscript bitcode and
                 // not launch them if it's present. Don't bother checking on devices
@@ -5135,7 +5136,7 @@ public class PackageManagerService extends IPackageManager.Stub {
             } catch (IOException ioe) {
                 Slog.e(TAG, "Unable to get canonical file " + ioe.toString());
             } finally {
-                handle.close();
+                IoUtils.closeQuietly(handle);
             }
         }
         pkg.mScanPath = path;
@@ -8768,10 +8769,11 @@ public class PackageManagerService extends IPackageManager.Stub {
                 nativeLibraryFile.delete();
             }
 
-            final NativeLibraryHelper.ApkHandle handle = new NativeLibraryHelper.ApkHandle(codeFile);
             String[] abiList = (abiOverride != null) ?
                     new String[] { abiOverride } : Build.SUPPORTED_ABIS;
+            ApkHandle handle = null;
             try {
+                handle = ApkHandle.create(codeFile);
                 if (Build.SUPPORTED_64_BIT_ABIS.length > 0 &&
                         abiOverride == null &&
                         NativeLibraryHelper.hasRenderscriptBitcode(handle)) {
@@ -8786,7 +8788,7 @@ public class PackageManagerService extends IPackageManager.Stub {
                 Slog.e(TAG, "Copying native libraries failed", e);
                 ret = PackageManager.INSTALL_FAILED_INTERNAL_ERROR;
             } finally {
-                handle.close();
+                IoUtils.closeQuietly(handle);
             }
 
             return ret;
@@ -12229,23 +12231,30 @@ public class PackageManagerService extends IPackageManager.Stub {
                                     final File newNativeDir = new File(newNativePath);
 
                                     if (!isForwardLocked(pkg) && !isExternal(pkg)) {
-                                        // NOTE: We do not report any errors from the APK scan and library
-                                        // copy at this point.
-                                        NativeLibraryHelper.ApkHandle handle =
-                                                new NativeLibraryHelper.ApkHandle(newCodePath);
-                                        final int abi = NativeLibraryHelper.findSupportedAbi(
-                                                handle, Build.SUPPORTED_ABIS);
-                                        if (abi >= 0) {
-                                            NativeLibraryHelper.copyNativeBinariesIfNeededLI(
-                                                    handle, newNativeDir, Build.SUPPORTED_ABIS[abi]);
+                                        ApkHandle handle = null;
+                                        try {
+                                            handle = ApkHandle.create(newCodePath);
+                                            final int abi = NativeLibraryHelper.findSupportedAbi(
+                                                    handle, Build.SUPPORTED_ABIS);
+                                            if (abi >= 0) {
+                                                NativeLibraryHelper.copyNativeBinariesIfNeededLI(
+                                                        handle, newNativeDir, Build.SUPPORTED_ABIS[abi]);
+                                            }
+                                        } catch (IOException ioe) {
+                                            Slog.w(TAG, "Unable to extract native libs for package :"
+                                                    + mp.packageName, ioe);
+                                            returnCode = PackageManager.MOVE_FAILED_INTERNAL_ERROR;
+                                        } finally {
+                                            IoUtils.closeQuietly(handle);
                                         }
-                                        handle.close();
                                     }
                                     final int[] users = sUserManager.getUserIds();
-                                    for (int user : users) {
-                                        if (mInstaller.linkNativeLibraryDirectory(pkg.packageName,
-                                                newNativePath, user) < 0) {
-                                            returnCode = PackageManager.MOVE_FAILED_INSUFFICIENT_STORAGE;
+                                    if (returnCode == PackageManager.MOVE_SUCCEEDED) {
+                                        for (int user : users) {
+                                            if (mInstaller.linkNativeLibraryDirectory(pkg.packageName,
+                                                    newNativePath, user) < 0) {
+                                                returnCode = PackageManager.MOVE_FAILED_INSUFFICIENT_STORAGE;
+                                            }
                                         }
                                     }