Add uid information in PackageListObserver

The uid information of a removed package cannot be retrieved by the
packageName anymore once it is removed. So it would be useful to provide
the uid of removed package in the onPackageAdded and onPackageRemoved
method of the PackageListObserver. This modification helps simplify the
design in PermissionMonitor.

Bug: 125396053
Test: dumpsys netd trafficcontroller

Exempt-From-Owner-Approval: Already merged in internal master
Merged-In: I2bd4bdf924687960a4fa3a47235bae68d885e445
Change-Id: I2bd4bdf924687960a4fa3a47235bae68d885e445

diff --git a/core/java/android/content/pm/PackageList.java b/core/java/android/content/pm/PackageList.java
index cfd99ab..f781758 100644 (file)
--- a/core/java/android/content/pm/PackageList.java
+++ b/core/java/android/content/pm/PackageList.java
@@ -45,16 +45,16 @@ public class PackageList implements PackageListObserver, AutoCloseable {
     }
 
     @Override
-    public void onPackageAdded(String packageName) {
+    public void onPackageAdded(String packageName, int uid) {
         if (mWrappedObserver != null) {
-            mWrappedObserver.onPackageAdded(packageName);
+            mWrappedObserver.onPackageAdded(packageName, uid);
         }
     }
 
     @Override
-    public void onPackageRemoved(String packageName) {
+    public void onPackageRemoved(String packageName, int uid) {
         if (mWrappedObserver != null) {
-            mWrappedObserver.onPackageRemoved(packageName);
+            mWrappedObserver.onPackageRemoved(packageName, uid);
         }
     }
 

diff --git a/core/java/android/content/pm/PackageManagerInternal.java b/core/java/android/content/pm/PackageManagerInternal.java
index 7c9943b..c299369 100644 (file)
--- a/core/java/android/content/pm/PackageManagerInternal.java
+++ b/core/java/android/content/pm/PackageManagerInternal.java
@@ -62,9 +62,9 @@ public abstract class PackageManagerInternal {
     /** Observer called whenever the list of packages changes */
     public interface PackageListObserver {
         /** A package was added to the system. */
-        void onPackageAdded(@NonNull String packageName);
+        void onPackageAdded(@NonNull String packageName, int uid);
         /** A package was removed from the system. */
-        void onPackageRemoved(@NonNull String packageName);
+        void onPackageRemoved(@NonNull String packageName, int uid);
     }
 
     /** Interface to override permission checks via composition */

diff --git a/services/core/java/com/android/server/connectivity/PermissionMonitor.java b/services/core/java/com/android/server/connectivity/PermissionMonitor.java
index d84a4d2..123564e 100644 (file)
--- a/services/core/java/com/android/server/connectivity/PermissionMonitor.java
+++ b/services/core/java/com/android/server/connectivity/PermissionMonitor.java
@@ -46,13 +46,11 @@ import android.util.Log;
 import android.util.Slog;
 import android.util.SparseIntArray;
 
-import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.ArrayUtils;
 import com.android.server.LocalServices;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -84,20 +82,14 @@ public class PermissionMonitor {
     // Keys are App IDs. Values are true for SYSTEM permission and false for NETWORK permission.
     private final Map<Integer, Boolean> mApps = new HashMap<>();
 
-    // Keys are App packageNames, Values are app uids. . We need to keep track of this information
-    // because PackageListObserver#onPackageRemoved does not pass the UID.
-    @GuardedBy("mPackageNameUidMap")
-    private final Map<String, Integer> mPackageNameUidMap = new HashMap<>();
-
     private class PackageListObserver implements PackageManagerInternal.PackageListObserver {
         @Override
-        public void onPackageAdded(String packageName) {
+        public void onPackageAdded(String packageName, int uid) {
             final PackageInfo app = getPackageInfo(packageName);
             if (app == null) {
                 Slog.wtf(TAG, "Failed to get information of installed package: " + packageName);
                 return;
             }
-            int uid = (app.applicationInfo != null) ? app.applicationInfo.uid : INVALID_UID;
             if (uid == INVALID_UID) {
                 Slog.wtf(TAG, "Failed to get the uid of installed package: " + packageName
                         + "uid: " + uid);
@@ -107,29 +99,21 @@ public class PermissionMonitor {
                 return;
             }
             sendPackagePermissionsForUid(uid,
-                    filterPermission(Arrays.asList(app.requestedPermissions)));
-            synchronized (mPackageNameUidMap) {
-                mPackageNameUidMap.put(packageName, uid);
-            }
+                    getNetdPermissionMask(app.requestedPermissions));
         }
 
         @Override
-        public void onPackageRemoved(String packageName) {
-            int uid;
-            synchronized (mPackageNameUidMap) {
-                if (!mPackageNameUidMap.containsKey(packageName)) {
-                    return;
-                }
-                uid = mPackageNameUidMap.get(packageName);
-                mPackageNameUidMap.remove(packageName);
-            }
+        public void onPackageRemoved(String packageName, int uid) {
             int permission = 0;
+            // If there are still packages remain under the same uid, check the permission of the
+            // remaining packages. We only remove the permission for a given uid when all packages
+            // for that uid no longer have that permission.
             String[] packages = mPackageManager.getPackagesForUid(uid);
             if (packages != null && packages.length > 0) {
                 for (String name : packages) {
                     final PackageInfo app = getPackageInfo(name);
                     if (app != null && app.requestedPermissions != null) {
-                        permission |= filterPermission(Arrays.asList(app.requestedPermissions));
+                        permission |= getNetdPermissionMask(app.requestedPermissions);
                     }
                 }
             }
@@ -184,12 +168,9 @@ public class PermissionMonitor {
 
             //TODO: unify the management of the permissions into one codepath.
             if (app.requestedPermissions != null) {
-                int otherNetdPerms = filterPermission(Arrays.asList(app.requestedPermissions));
+                int otherNetdPerms = getNetdPermissionMask(app.requestedPermissions);
                 if (otherNetdPerms != 0) {
                     netdPermsUids.put(uid, netdPermsUids.get(uid) | otherNetdPerms);
-                    synchronized (mPackageNameUidMap) {
-                        mPackageNameUidMap.put(app.applicationInfo.packageName, uid);
-                    }
                 }
             }
         }
@@ -422,13 +403,15 @@ public class PermissionMonitor {
         }
     }
 
-    private static int filterPermission(List<String> requestedPermissions) {
+    private static int getNetdPermissionMask(String[] requestedPermissions) {
         int permissions = 0;
-        if (requestedPermissions.contains(INTERNET)) {
-            permissions |= INetd.PERMISSION_INTERNET;
-        }
-        if (requestedPermissions.contains(UPDATE_DEVICE_STATS)) {
-            permissions |= INetd.PERMISSION_UPDATE_DEVICE_STATS;
+        for (String permissionName : requestedPermissions) {
+            if (permissionName.equals(INTERNET)) {
+                permissions |= INetd.PERMISSION_INTERNET;
+            }
+            if (permissionName.equals(UPDATE_DEVICE_STATS)) {
+                permissions |= INetd.PERMISSION_UPDATE_DEVICE_STATS;
+            }
         }
         return permissions;
     }
@@ -439,8 +422,6 @@ public class PermissionMonitor {
                     | MATCH_ANY_USER);
             return app;
         } catch (NameNotFoundException e) {
-            // App not found.
-            loge("NameNotFoundException " + packageName);
             return null;
         }
     }

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 2f510a7..790e6e1 100644 (file)
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -2157,7 +2157,7 @@ public class PackageManagerService extends IPackageManager.Stub
             }
 
             if (allNewUsers && !update) {
-                notifyPackageAdded(packageName);
+                notifyPackageAdded(packageName, res.uid);
             }
 
             // Log current value of "unknown sources" setting
@@ -13728,7 +13728,7 @@ public class PackageManagerService extends IPackageManager.Stub
     }
 
     @Override
-    public void notifyPackageAdded(String packageName) {
+    public void notifyPackageAdded(String packageName, int uid) {
         final PackageListObserver[] observers;
         synchronized (mPackages) {
             if (mPackageListObservers.size() == 0) {
@@ -13739,12 +13739,12 @@ public class PackageManagerService extends IPackageManager.Stub
             observers = mPackageListObservers.toArray(observerArray);
         }
         for (int i = observers.length - 1; i >= 0; --i) {
-            observers[i].onPackageAdded(packageName);
+            observers[i].onPackageAdded(packageName, uid);
         }
     }
 
     @Override
-    public void notifyPackageRemoved(String packageName) {
+    public void notifyPackageRemoved(String packageName, int uid) {
         final PackageListObserver[] observers;
         synchronized (mPackages) {
             if (mPackageListObservers.size() == 0) {
@@ -13755,7 +13755,7 @@ public class PackageManagerService extends IPackageManager.Stub
             observers = mPackageListObservers.toArray(observerArray);
         }
         for (int i = observers.length - 1; i >= 0; --i) {
-            observers[i].onPackageRemoved(packageName);
+            observers[i].onPackageRemoved(packageName, uid);
         }
     }
 
@@ -18562,7 +18562,8 @@ public class PackageManagerService extends IPackageManager.Stub
                 return;
             }
             Bundle extras = new Bundle(2);
-            extras.putInt(Intent.EXTRA_UID, removedAppId >= 0  ? removedAppId : uid);
+            final int removedUid = removedAppId >= 0  ? removedAppId : uid;
+            extras.putInt(Intent.EXTRA_UID, removedUid);
             extras.putBoolean(Intent.EXTRA_DATA_REMOVED, dataRemoved);
             extras.putBoolean(Intent.EXTRA_DONT_KILL_APP, !killApp);
             if (isUpdate || isRemovedPackageSystemUpdate) {
@@ -18583,7 +18584,7 @@ public class PackageManagerService extends IPackageManager.Stub
                         removedPackage, extras,
                         Intent.FLAG_RECEIVER_INCLUDE_BACKGROUND,
                         null, null, broadcastUsers, instantUserIds);
-                    packageSender.notifyPackageRemoved(removedPackage);
+                    packageSender.notifyPackageRemoved(removedPackage, removedUid);
                 }
             }
             if (removedAppId >= 0) {
@@ -24946,6 +24947,6 @@ interface PackageSender {
         final IIntentReceiver finishedReceiver, final int[] userIds, int[] instantUserIds);
     void sendPackageAddedForNewUsers(String packageName, boolean sendBootCompleted,
         boolean includeStopped, int appId, int[] userIds, int[] instantUserIds);
-    void notifyPackageAdded(String packageName);
-    void notifyPackageRemoved(String packageName);
+    void notifyPackageAdded(String packageName, int uid);
+    void notifyPackageRemoved(String packageName, int uid);
 }

diff --git a/services/tests/servicestests/src/com/android/server/pm/PackageManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/pm/PackageManagerServiceTest.java
index e379cd0..68728af 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/pm/PackageManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/PackageManagerServiceTest.java
@@ -56,11 +56,11 @@ public class PackageManagerServiceTest {
             }
 
             @Override
-            public void notifyPackageAdded(String packageName) {
+            public void notifyPackageAdded(String packageName, int uid) {
             }
 
             @Override
-            public void notifyPackageRemoved(String packageName) {
+            public void notifyPackageRemoved(String packageName, int uid) {
             }
         }