Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4cfe0030 authored by Philip P. Moltmann's avatar Philip P. Moltmann Committed by Android (Google) Code Review
Browse files

Merge "Only kill when needed when changing permissions"

parents f99be9ad 7380bf8d
Loading
Loading
Loading
Loading
+69 −28
Original line number Diff line number Diff line
@@ -26,7 +26,6 @@ import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE;
import static android.app.AppOpsManager.MODE_ALLOWED;
import static android.app.AppOpsManager.MODE_FOREGROUND;
import static android.app.AppOpsManager.MODE_IGNORED;
import static android.content.pm.PackageManager.FLAG_PERMISSION_SYSTEM_FIXED;
import static android.content.pm.PackageManager.PERMISSION_GRANTED;

import android.app.ActivityManager;
@@ -628,6 +627,25 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
        return grantRuntimePermissions(fixedByTheUser, null);
    }

    /**
     * Set mode of an app-op if needed.
     *
     * @param op The op to set
     * @param uid The uid the app-op belongs top
     * @param mode The new mode
     *
     * @return {@code true} iff app-op was changed
     */
    private boolean setAppOpMode(@NonNull String op, int uid, int mode) {
        int currentMode = mAppOps.unsafeCheckOpRaw(op, uid, mPackageInfo.packageName);
        if (currentMode == mode) {
            return false;
        }

        mAppOps.setUidMode(op, uid, mode);
        return true;
    }

    /**
     * Allow the app op for a permission/uid.
     *
@@ -649,8 +667,12 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
     *
     * @param permission The permission which has an appOps that should be allowed
     * @param uid        The uid of the process the app op if for
     *
     * @return {@code true} iff app-op was changed
     */
    private void allowAppOp(Permission permission, int uid) {
    private boolean allowAppOp(Permission permission, int uid) {
        boolean wasChanged = false;

        if (permission.isBackgroundPermission()) {
            ArrayList<Permission> foregroundPermissions = permission.getForegroundPermissions();

@@ -658,7 +680,7 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
            for (int i = 0; i < numForegroundPermissions; i++) {
                Permission foregroundPermission = foregroundPermissions.get(i);
                if (foregroundPermission.isAppOpAllowed()) {
                    mAppOps.setUidMode(foregroundPermission.getAppOp(), uid, MODE_ALLOWED);
                    wasChanged |= setAppOpMode(foregroundPermission.getAppOp(), uid, MODE_ALLOWED);
                }
            }
        } else {
@@ -669,18 +691,20 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
                    // The app requested a permission that has a background permission but it did
                    // not request the background permission, hence it can never get background
                    // access
                    mAppOps.setUidMode(permission.getAppOp(), uid, MODE_FOREGROUND);
                    wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_FOREGROUND);
                } else {
                    if (backgroundPermission.isAppOpAllowed()) {
                        mAppOps.setUidMode(permission.getAppOp(), uid, MODE_ALLOWED);
                        wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_ALLOWED);
                    } else {
                        mAppOps.setUidMode(permission.getAppOp(), uid, MODE_FOREGROUND);
                        wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_FOREGROUND);
                    }
                }
            } else {
                mAppOps.setUidMode(permission.getAppOp(), uid, MODE_ALLOWED);
                wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_ALLOWED);
            }
        }

        return wasChanged;
    }

    /**
@@ -846,8 +870,12 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
     *
     * @param permission The permission which has an appOps that should be disallowed
     * @param uid        The uid of the process the app op if for
     *
     * @return {@code true} iff app-op was changed
     */
    private void disallowAppOp(Permission permission, int uid) {
    private boolean disallowAppOp(Permission permission, int uid) {
        boolean wasChanged = false;

        if (permission.isBackgroundPermission()) {
            ArrayList<Permission> foregroundPermissions = permission.getForegroundPermissions();

@@ -855,12 +883,15 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
            for (int i = 0; i < numForegroundPermissions; i++) {
                Permission foregroundPermission = foregroundPermissions.get(i);
                if (foregroundPermission.isAppOpAllowed()) {
                    mAppOps.setUidMode(foregroundPermission.getAppOp(), uid, MODE_FOREGROUND);
                    wasChanged |= setAppOpMode(foregroundPermission.getAppOp(), uid,
                            MODE_FOREGROUND);
                }
            }
        } else {
            mAppOps.setUidMode(permission.getAppOp(), uid, MODE_IGNORED);
            wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_IGNORED);
        }

        return wasChanged;
    }

    /**
@@ -1134,7 +1165,9 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
     *                                     app ops change. If this is set to {@code false} the
     *                                     caller has to make sure to kill the app if needed.
     */
    public void persistChanges(boolean mayKillBecauseOfAppOpsChange) {
    void persistChanges(boolean mayKillBecauseOfAppOpsChange) {
        int uid = mPackageInfo.applicationInfo.uid;

        int numPermissions = mPermissions.size();
        boolean shouldKillApp = false;
        boolean shouldUpdateStorage = false;
@@ -1147,10 +1180,15 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
                    mPackageManager.grantRuntimePermission(mPackageInfo.packageName,
                            permission.getName(), mUserHandle);
                } else {
                    boolean isCurrentlyGranted = mContext.checkPermission(permission.getName(), -1,
                            uid) == PERMISSION_GRANTED;

                    if (isCurrentlyGranted) {
                        mPackageManager.revokeRuntimePermission(mPackageInfo.packageName,
                                permission.getName(), mUserHandle);
                    }
                }
            }

            int flags = (permission.isUserSet() ? PackageManager.FLAG_PERMISSION_USER_SET : 0)
                    | (permission.isUserFixed() ? PackageManager.FLAG_PERMISSION_USER_FIXED : 0)
@@ -1171,16 +1209,14 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>

            if (permission.affectsAppOp()) {
                if (!permission.isSystemFixed()) {
                    // Enabling/Disabling an app op may put the app in a situation in which it has
                    // a handle to state it shouldn't have, so we have to kill the app. This matches
                    // the revoke runtime permission behavior.
                    if (permission.isAppOpAllowed()) {
                        allowAppOp(permission, mPackageInfo.applicationInfo.uid);
                        shouldKillApp |= allowAppOp(permission, uid);
                    } else {
                        disallowAppOp(permission, mPackageInfo.applicationInfo.uid);
                        shouldKillApp |= disallowAppOp(permission, uid);
                    }

                    // Enabling/Disabling an app op may put the app in a situation in which it has a
                    // handle to state it shouldn't have, so we have to kill the app. This matches
                    // the revoke runtime permission behavior.
                    shouldKillApp = true;
                }
            }

@@ -1201,12 +1237,12 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
        // case, whenever any of the new split permissions are granted to an
        // app, we also grant them the legacy "Storage" permission.
        if (StorageManager.hasIsolatedStorage() && shouldUpdateStorage) {
            boolean audioGranted = mPackageManager.checkPermission(READ_MEDIA_AUDIO,
                    mPackageInfo.packageName) == PERMISSION_GRANTED;
            boolean videoGranted = mPackageManager.checkPermission(READ_MEDIA_VIDEO,
                    mPackageInfo.packageName) == PERMISSION_GRANTED;
            boolean imagesGranted = mPackageManager.checkPermission(READ_MEDIA_IMAGES,
                    mPackageInfo.packageName) == PERMISSION_GRANTED;
            boolean audioGranted = mContext.checkPermission(READ_MEDIA_AUDIO,
                    -1, uid) == PERMISSION_GRANTED;
            boolean videoGranted = mContext.checkPermission(READ_MEDIA_VIDEO,
                    -1, uid) == PERMISSION_GRANTED;
            boolean imagesGranted = mContext.checkPermission(READ_MEDIA_IMAGES,
                    -1, uid) == PERMISSION_GRANTED;

            if (!ArrayUtils.isEmpty(mPackageInfo.requestedPermissions)) {
                for (String permission : mPackageInfo.requestedPermissions) {
@@ -1216,6 +1252,10 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
                            mPackageManager.grantRuntimePermission(mPackageInfo.packageName,
                                    permission, mUserHandle);
                        } else {
                            boolean isCurrentlyGranted = mContext.checkPermission(permission, -1,
                                    uid) == PERMISSION_GRANTED;

                            if (isCurrentlyGranted) {
                                mPackageManager.revokeRuntimePermission(mPackageInfo.packageName,
                                        permission, mUserHandle);
                            }
@@ -1223,6 +1263,7 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup>
                    }
                }
            }
        }

        if (mayKillBecauseOfAppOpsChange && shouldKillApp) {
            killApp(KILL_REASON_APP_OP_CHANGE);
+7 −3
Original line number Diff line number Diff line
@@ -179,18 +179,22 @@ public final class AppPermissions {

    /**
     * If the changes to the permission groups were delayed, persist them now.
     *
     * @param mayKillBecauseOfAppOpsChange If the app may be killed if app ops change. If this is
     *                                     set to {@code false} the caller has to make sure to kill
     *                                     the app if needed.
     */
    public void persistChanges() {
    public void persistChanges(boolean mayKillBecauseOfAppOpsChange) {
        if (mDelayChanges) {
            int numGroups = mGroups.size();

            for (int i = 0; i < numGroups; i++) {
                AppPermissionGroup group = mGroups.get(i);
                group.persistChanges(true);
                group.persistChanges(mayKillBecauseOfAppOpsChange);

                AppPermissionGroup backgroundGroup = group.getBackgroundPermissions();
                if (backgroundGroup != null) {
                    backgroundGroup.persistChanges(true);
                    backgroundGroup.persistChanges(mayKillBecauseOfAppOpsChange);
                }
            }
        }
+1 −3
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@
package com.android.packageinstaller.permission.service;

import static android.content.Context.MODE_PRIVATE;
import static android.content.pm.PackageManager.FLAG_PERMISSION_GRANTED_BY_DEFAULT;
import static android.content.pm.PackageManager.FLAG_PERMISSION_POLICY_FIXED;
import static android.content.pm.PackageManager.FLAG_PERMISSION_SYSTEM_FIXED;
import static android.content.pm.PackageManager.GET_PERMISSIONS;
@@ -53,7 +52,6 @@ import org.xmlpull.v1.XmlPullParser;
import org.xmlpull.v1.XmlPullParserException;
import org.xmlpull.v1.XmlSerializer;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.OutputStream;
@@ -750,7 +748,7 @@ public class BackupHelper {
                }
            }

            appPerms.persistChanges();
            appPerms.persistChanges(true);
        }
    }
}
+18 −10
Original line number Diff line number Diff line
@@ -286,7 +286,7 @@ public final class PermissionControllerServiceImpl extends PermissionControllerS
        if (!doDryRun) {
            int numChangedApps = appsWithRevokedPerms.size();
            for (int i = 0; i < numChangedApps; i++) {
                appsWithRevokedPerms.get(i).persistChanges();
                appsWithRevokedPerms.get(i).persistChanges(true);
            }
        }

@@ -528,33 +528,41 @@ public final class PermissionControllerServiceImpl extends PermissionControllerS
                Collections.singletonList(unexpandedPermission),
                callerPkgInfo.applicationInfo.targetSdkVersion);

        AppPermissions app = new AppPermissions(this, pkgInfo, false, null);

        int numPerms = expandedPermissions.size();
        for (int i = 0; i < numPerms; i++) {
            String perm = expandedPermissions.get(i);
            AppPermissionGroup group = AppPermissionGroup.create(this, pkgInfo, perm, true);
            String permName = expandedPermissions.get(i);
            AppPermissionGroup group = app.getGroupForPermission(permName);
            if (group == null || group.isSystemFixed()) {
                continue;
            }

            Permission perm = group.getPermission(permName);
            if (perm == null) {
                continue;
            }

            switch (grantState) {
                case PERMISSION_GRANT_STATE_GRANTED:
                    group.getPermission(perm).setPolicyFixed(true);
                    group.grantRuntimePermissions(false, new String[]{perm});
                    perm.setPolicyFixed(true);
                    group.grantRuntimePermissions(false, new String[]{permName});
                    break;
                case PERMISSION_GRANT_STATE_DENIED:
                    group.getPermission(perm).setPolicyFixed(true);
                    group.revokeRuntimePermissions(false, new String[]{perm});
                    perm.setPolicyFixed(true);
                    group.revokeRuntimePermissions(false, new String[]{permName});
                    break;
                case PERMISSION_GRANT_STATE_DEFAULT:
                    group.getPermission(perm).setPolicyFixed(false);
                    perm.setPolicyFixed(false);
                    break;
                default:
                    return false;
            }

            group.persistChanges(!callerPackageName.equals(packageName));
        }

        app.persistChanges(grantState == PERMISSION_GRANT_STATE_DENIED
                || !callerPackageName.equals(packageName));

        return true;
    }
}
+1 −1
Original line number Diff line number Diff line
@@ -211,7 +211,7 @@ public final class ReviewPermissionsFragment extends PreferenceFragmentCompat
            }
        }

        mAppPermissions.persistChanges();
        mAppPermissions.persistChanges(true);
    }

    private void bindUi() {