Snap for 7097357 from 411d5e27156ed21258795af9306ab89e44620b13 to rvc-platform-releaseandroid-platform-11.0.0_r4

Change-Id: I6c867a70d9d571d4b5ef253f71118dd6e8d8a9de

17 files changed, 393 insertions, 108 deletions

diff --git a/core/java/android/accounts/GrantCredentialsPermissionActivity.java b/core/java/android/accounts/GrantCredentialsPermissionActivity.java
index af74b036a796..5dc6e602e5d6 100644
--- a/core/java/android/accounts/GrantCredentialsPermissionActivity.java
+++ b/core/java/android/accounts/GrantCredentialsPermissionActivity.java
@@ -16,16 +16,23 @@
 package android.accounts;
 
 import android.app.Activity;
-import android.content.res.Resources;
-import android.os.Bundle;
-import android.widget.TextView;
-import android.widget.LinearLayout;
-import android.view.View;
-import android.view.LayoutInflater;
+import android.app.ActivityTaskManager;
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageManager;
+import android.content.res.Resources;
+import android.os.Bundle;
+import android.os.IBinder;
+import android.os.Process;
+import android.os.RemoteException;
+import android.os.UserHandle;
 import android.text.TextUtils;
+import android.util.Log;
+import android.view.LayoutInflater;
+import android.view.View;
+import android.widget.LinearLayout;
+import android.widget.TextView;
+
 import com.android.internal.R;
 
 import java.io.IOException;
@@ -42,11 +49,15 @@ public class GrantCredentialsPermissionActivity extends Activity implements View
     private Account mAccount;
     private String mAuthTokenType;
     private int mUid;
+    private int mCallingUid;
     private Bundle mResultBundle = null;
     protected LayoutInflater mInflater;
 
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
+        getWindow().addSystemFlags(
+                android.view.WindowManager.LayoutParams
+                        .SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS);
         setContentView(R.layout.grant_credentials_permission);
         setTitle(R.string.grant_permissions_header_text);
 
@@ -74,6 +85,20 @@ public class GrantCredentialsPermissionActivity extends Activity implements View
             return;
         }
 
+        try {
+            IBinder activityToken = getActivityToken();
+            mCallingUid = ActivityTaskManager.getService().getLaunchedFromUid(activityToken);
+        } catch (RemoteException re) {
+            // Couldn't figure out caller details
+            Log.w(getClass().getSimpleName(), "Unable to get caller identity \n" + re);
+        }
+
+        if (!UserHandle.isSameApp(mCallingUid, Process.SYSTEM_UID) && mCallingUid != mUid) {
+            setResult(Activity.RESULT_CANCELED);
+            finish();
+            return;
+        }
+
         String accountTypeLabel;
         try {
             accountTypeLabel = getAccountLabel(mAccount);
diff --git a/core/java/android/app/slice/SliceProvider.java b/core/java/android/app/slice/SliceProvider.java
index bd1eea51f8af..46be54814dc9 100644
--- a/core/java/android/app/slice/SliceProvider.java
+++ b/core/java/android/app/slice/SliceProvider.java
@@ -153,6 +153,7 @@ public abstract class SliceProvider extends ContentProvider {
      */
     public static final String EXTRA_PKG = "pkg";
     /**
+     * @Deprecated provider pkg is now being extracted in SlicePermissionActivity
      * @hide
      */
     public static final String EXTRA_PROVIDER_PKG = "provider_pkg";
diff --git a/core/java/com/android/internal/app/ResolverActivity.java b/core/java/com/android/internal/app/ResolverActivity.java
index 233231cfcfdf..fadc15912dc1 100644
--- a/core/java/com/android/internal/app/ResolverActivity.java
+++ b/core/java/com/android/internal/app/ResolverActivity.java
@@ -1210,25 +1210,6 @@ public class ResolverActivity extends Activity implements
                         if (TextUtils.isEmpty(packageName)) {
                             pm.setDefaultBrowserPackageNameAsUser(ri.activityInfo.packageName, userId);
                         }
-                    } else {
-                        // Update Domain Verification status
-                        ComponentName cn = intent.getComponent();
-                        String packageName = cn.getPackageName();
-                        String dataScheme = (data != null) ? data.getScheme() : null;
-
-                        boolean isHttpOrHttps = (dataScheme != null) &&
-                                (dataScheme.equals(IntentFilter.SCHEME_HTTP) ||
-                                        dataScheme.equals(IntentFilter.SCHEME_HTTPS));
-
-                        boolean isViewAction = (action != null) && action.equals(Intent.ACTION_VIEW);
-                        boolean hasCategoryBrowsable = (categories != null) &&
-                                categories.contains(Intent.CATEGORY_BROWSABLE);
-
-                        if (isHttpOrHttps && isViewAction && hasCategoryBrowsable) {
-                            pm.updateIntentVerificationStatusAsUser(packageName,
-                                    PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ALWAYS,
-                                    userId);
-                        }
                     }
                 } else {
                     try {
diff --git a/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java b/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java
index be778e92787e..94829b506f95 100755
--- a/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java
+++ b/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java
@@ -17,6 +17,7 @@
 package com.android.packageinstaller;
 
 import static android.app.AppOpsManager.MODE_ALLOWED;
+import static android.view.WindowManager.LayoutParams.SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS;
 
 import static com.android.packageinstaller.PackageUtil.getMaxTargetSdkVersionForUid;
 
@@ -87,6 +88,8 @@ public class UninstallerActivity extends Activity {
 
     @Override
     public void onCreate(Bundle icicle) {
+        getWindow().addSystemFlags(SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS);
+
         // Never restore any state, esp. never create any fragments. The data in the fragment might
         // be stale, if e.g. the app was uninstalled while the activity was destroyed.
         super.onCreate(null);
diff --git a/packages/SettingsProvider/src/android/provider/settings/validators/SystemSettingsValidators.java b/packages/SettingsProvider/src/android/provider/settings/validators/SystemSettingsValidators.java
index c5d4fa9f1b40..cb610fc61142 100644
--- a/packages/SettingsProvider/src/android/provider/settings/validators/SystemSettingsValidators.java
+++ b/packages/SettingsProvider/src/android/provider/settings/validators/SystemSettingsValidators.java
@@ -89,15 +89,7 @@ public class SystemSettingsValidators {
                         return value == null || value.length() < MAX_LENGTH;
                     }
                 });
-        VALIDATORS.put(
-                System.FONT_SCALE,
-                value -> {
-                    try {
-                        return Float.parseFloat(value) >= 0;
-                    } catch (NumberFormatException | NullPointerException e) {
-                        return false;
-                    }
-                });
+        VALIDATORS.put(System.FONT_SCALE, new InclusiveFloatRangeValidator(0.85f, 1.3f));
         VALIDATORS.put(System.DIM_SCREEN, BOOLEAN_VALIDATOR);
         VALIDATORS.put(
                 System.DISPLAY_COLOR_MODE,
diff --git a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java
index 449ed8c3bcdb..57e656827f1c 100644
--- a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java
+++ b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java
@@ -16,6 +16,7 @@ package com.android.systemui;
 
 import static android.view.WindowManager.LayoutParams.SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS;
 
+import android.annotation.Nullable;
 import android.app.Activity;
 import android.app.AlertDialog;
 import android.app.slice.SliceManager;
@@ -29,6 +30,7 @@ import android.content.pm.PackageManager.NameNotFoundException;
 import android.net.Uri;
 import android.os.Bundle;
 import android.text.BidiFormatter;
+import android.util.EventLog;
 import android.util.Log;
 import android.widget.CheckBox;
 import android.widget.TextView;
@@ -50,10 +52,12 @@ public class SlicePermissionActivity extends Activity implements OnClickListener
 
         mUri = getIntent().getParcelableExtra(SliceProvider.EXTRA_BIND_URI);
         mCallingPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PKG);
-        mProviderPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PROVIDER_PKG);
 
         try {
             PackageManager pm = getPackageManager();
+            mProviderPkg = pm.resolveContentProvider(mUri.getAuthority(),
+                    PackageManager.GET_META_DATA).applicationInfo.packageName;
+            verifyCallingPkg();
             CharSequence app1 = BidiFormatter.getInstance().unicodeWrap(pm.getApplicationInfo(
                     mCallingPkg, 0).loadSafeLabel(pm, PackageItemInfo.DEFAULT_MAX_LABEL_SIZE_PX,
                     PackageItemInfo.SAFE_LABEL_FLAG_TRIM
@@ -97,4 +101,27 @@ public class SlicePermissionActivity extends Activity implements OnClickListener
     public void onDismiss(DialogInterface dialog) {
         finish();
     }
+
+    private void verifyCallingPkg() {
+        final String providerPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PROVIDER_PKG);
+        if (providerPkg == null || mProviderPkg.equals(providerPkg)) return;
+        final String callingPkg = getCallingPkg();
+        EventLog.writeEvent(0x534e4554, "159145361", getUid(callingPkg));
+    }
+
+    @Nullable
+    private String getCallingPkg() {
+        final Uri referrer = getReferrer();
+        if (referrer == null) return null;
+        return referrer.getHost();
+    }
+
+    private int getUid(@Nullable final String pkg) {
+        if (pkg == null) return -1;
+        try {
+            return getPackageManager().getApplicationInfo(pkg, 0).uid;
+        } catch (NameNotFoundException e) {
+        }
+        return -1;
+    }
 }
diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java
index d6759b3e2cca..29fc1674bab9 100644
--- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java
+++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java
@@ -372,7 +372,10 @@ public class CompanionDeviceManagerService extends SystemService implements Bind
 
             checkArgument(getCallingUserId() == userId,
                     "Must be called by either same user or system");
-            mAppOpsManager.checkPackage(Binder.getCallingUid(), pkg);
+            int callingUid = Binder.getCallingUid();
+            if (mAppOpsManager.checkPackage(callingUid, pkg) != AppOpsManager.MODE_ALLOWED) {
+                throw new SecurityException(pkg + " doesn't belong to uid " + callingUid);
+            }
         }
 
         @Override
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 3b8518004309..7acb1bf47e78 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -7142,67 +7142,68 @@ public class ActivityManagerService extends IActivityManager.Stub
                         "getContentProviderImpl: after checkContentProviderPermission");
 
                 final long origId = Binder.clearCallingIdentity();
+                try {
+                    checkTime(startTime, "getContentProviderImpl: incProviderCountLocked");
+
+                    // Return the provider instance right away since it already exists.
+                    conn = incProviderCountLocked(r, cpr, token, callingUid, callingPackage,
+                            callingTag, stable);
+                    if (conn != null && (conn.stableCount+conn.unstableCount) == 1) {
+                        if (cpr.proc != null
+                                && r != null && r.setAdj <= ProcessList.PERCEPTIBLE_LOW_APP_ADJ) {
+                            // If this is a perceptible app accessing the provider,
+                            // make sure to count it as being accessed and thus
+                            // back up on the LRU list.  This is good because
+                            // content providers are often expensive to start.
+                            checkTime(startTime, "getContentProviderImpl: before updateLruProcess");
+                            mProcessList.updateLruProcessLocked(cpr.proc, false, null);
+                            checkTime(startTime, "getContentProviderImpl: after updateLruProcess");
+                        }
+                    }
 
-                checkTime(startTime, "getContentProviderImpl: incProviderCountLocked");
-
-                // In this case the provider instance already exists, so we can
-                // return it right away.
-                conn = incProviderCountLocked(r, cpr, token, callingUid, callingPackage, callingTag,
-                        stable);
-                if (conn != null && (conn.stableCount+conn.unstableCount) == 1) {
-                    if (cpr.proc != null
-                            && r != null && r.setAdj <= ProcessList.PERCEPTIBLE_LOW_APP_ADJ) {
-                        // If this is a perceptible app accessing the provider,
-                        // make sure to count it as being accessed and thus
-                        // back up on the LRU list.  This is good because
-                        // content providers are often expensive to start.
-                        checkTime(startTime, "getContentProviderImpl: before updateLruProcess");
-                        mProcessList.updateLruProcessLocked(cpr.proc, false, null);
-                        checkTime(startTime, "getContentProviderImpl: after updateLruProcess");
-                    }
-                }
-
-                checkTime(startTime, "getContentProviderImpl: before updateOomAdj");
-                final int verifiedAdj = cpr.proc.verifiedAdj;
-                boolean success = updateOomAdjLocked(cpr.proc, true,
-                        OomAdjuster.OOM_ADJ_REASON_GET_PROVIDER);
-                // XXX things have changed so updateOomAdjLocked doesn't actually tell us
-                // if the process has been successfully adjusted.  So to reduce races with
-                // it, we will check whether the process still exists.  Note that this doesn't
-                // completely get rid of races with LMK killing the process, but should make
-                // them much smaller.
-                if (success && verifiedAdj != cpr.proc.setAdj && !isProcessAliveLocked(cpr.proc)) {
-                    success = false;
-                }
-                maybeUpdateProviderUsageStatsLocked(r, cpr.info.packageName, name);
-                checkTime(startTime, "getContentProviderImpl: after updateOomAdj");
-                if (DEBUG_PROVIDER) Slog.i(TAG_PROVIDER, "Adjust success: " + success);
-                // NOTE: there is still a race here where a signal could be
-                // pending on the process even though we managed to update its
-                // adj level.  Not sure what to do about this, but at least
-                // the race is now smaller.
-                if (!success) {
-                    // Uh oh...  it looks like the provider's process
-                    // has been killed on us.  We need to wait for a new
-                    // process to be started, and make sure its death
-                    // doesn't kill our process.
-                    Slog.wtf(TAG, "Existing provider " + cpr.name.flattenToShortString()
-                            + " is crashing; detaching " + r);
-                    boolean lastRef = decProviderCountLocked(conn, cpr, token, stable);
-                    if (!lastRef) {
-                        // This wasn't the last ref our process had on
-                        // the provider...  we will be killed during cleaning up, bail.
-                        return null;
+                    checkTime(startTime, "getContentProviderImpl: before updateOomAdj");
+                    final int verifiedAdj = cpr.proc.verifiedAdj;
+                    boolean success = updateOomAdjLocked(cpr.proc, true,
+                            OomAdjuster.OOM_ADJ_REASON_GET_PROVIDER);
+                    // XXX things have changed so updateOomAdjLocked doesn't actually tell us
+                    // if the process has been successfully adjusted.  So to reduce races with
+                    // it, we will check whether the process still exists.  Note that this doesn't
+                    // completely get rid of races with LMK killing the process, but should make
+                    // them much smaller.
+                    if (success && verifiedAdj != cpr.proc.setAdj
+                            && !isProcessAliveLocked(cpr.proc)) {
+                        success = false;
+                    }
+                    maybeUpdateProviderUsageStatsLocked(r, cpr.info.packageName, name);
+                    checkTime(startTime, "getContentProviderImpl: after updateOomAdj");
+                    if (DEBUG_PROVIDER) Slog.i(TAG_PROVIDER, "Adjust success: " + success);
+                    // NOTE: there is still a race here where a signal could be
+                    // pending on the process even though we managed to update its
+                    // adj level.  Not sure what to do about this, but at least
+                    // the race is now smaller.
+                    if (!success) {
+                        // Uh oh...  it looks like the provider's process
+                        // has been killed on us.  We need to wait for a new
+                        // process to be started, and make sure its death
+                        // doesn't kill our process.
+                        Slog.wtf(TAG, "Existing provider " + cpr.name.flattenToShortString()
+                                + " is crashing; detaching " + r);
+                        boolean lastRef = decProviderCountLocked(conn, cpr, token, stable);
+                        if (!lastRef) {
+                            // This wasn't the last ref our process had on
+                            // the provider...  we will be killed during cleaning up, bail.
+                            return null;
+                        }
+                        // We'll just start a new process to host the content provider
+                        providerRunning = false;
+                        conn = null;
+                        dyingProc = cpr.proc;
+                    } else {
+                        cpr.proc.verifiedAdj = cpr.proc.setAdj;
                     }
-                    // We'll just start a new process to host the content provider
-                    providerRunning = false;
-                    conn = null;
-                    dyingProc = cpr.proc;
-                } else {
-                    cpr.proc.verifiedAdj = cpr.proc.setAdj;
+                } finally {
+                    Binder.restoreCallingIdentity(origId);
                 }
-
-                Binder.restoreCallingIdentity(origId);
             }
 
             if (!providerRunning) {
@@ -10459,12 +10460,10 @@ public class ActivityManagerService extends IActivityManager.Stub
         } finally {
             Binder.restoreCallingIdentity(identity);
         }
-        if (uid == Process.INVALID_UID) {
-            return Process.INVALID_UID;
-        }
+        // If the uid is Process.INVALID_UID, the below 'if' check will be always true
         if (UserHandle.getAppId(uid) != UserHandle.getAppId(callingUid)) {
             // Requires the DUMP permission if the target package doesn't belong
-            // to the caller.
+            // to the caller or it doesn't exist.
             enforceCallingPermission(android.Manifest.permission.DUMP, function);
         }
         return uid;
diff --git a/services/core/java/com/android/server/biometrics/AuthenticationClient.java b/services/core/java/com/android/server/biometrics/AuthenticationClient.java
index edc8f15a9a03..8ca37b813b63 100644
--- a/services/core/java/com/android/server/biometrics/AuthenticationClient.java
+++ b/services/core/java/com/android/server/biometrics/AuthenticationClient.java
@@ -16,16 +16,22 @@
 
 package com.android.server.biometrics;
 
+import android.app.ActivityManager;
+import android.app.ActivityTaskManager;
+import android.content.ComponentName;
 import android.content.Context;
+import android.content.pm.ApplicationInfo;
 import android.hardware.biometrics.BiometricAuthenticator;
 import android.hardware.biometrics.BiometricConstants;
 import android.hardware.biometrics.BiometricsProtoEnums;
 import android.os.IBinder;
 import android.os.RemoteException;
 import android.security.KeyStore;
+import android.util.EventLog;
 import android.util.Slog;
 
 import java.util.ArrayList;
+import java.util.List;
 
 /**
  * A class to keep track of the authentication state for a given client.
@@ -148,7 +154,54 @@ public abstract class AuthenticationClient extends ClientMonitor {
                     + ", requireConfirmation: " + mRequireConfirmation
                     + ", user: " + getTargetUserId());
 
+            // Ensure authentication only succeeds if the client activity is on top or is keyguard.
+            boolean isBackgroundAuth = false;
+            if (authenticated && !Utils.isKeyguard(getContext(), getOwnerString())) {
+                try {
+                    final List<ActivityManager.RunningTaskInfo> tasks =
+                            ActivityTaskManager.getService().getTasks(1);
+                    if (tasks == null || tasks.isEmpty()) {
+                        Slog.e(TAG, "No running tasks reported");
+                        isBackgroundAuth = true;
+                    } else {
+                        final ComponentName topActivity = tasks.get(0).topActivity;
+                        if (topActivity == null) {
+                            Slog.e(TAG, "Unable to get top activity");
+                            isBackgroundAuth = true;
+                        } else {
+                            final String topPackage = topActivity.getPackageName();
+                            if (!topPackage.contentEquals(getOwnerString())) {
+                                Slog.e(TAG, "Background authentication detected, top: " + topPackage
+                                        + ", client: " + this);
+                                isBackgroundAuth = true;
+                            }
+                        }
+                    }
+                } catch (RemoteException e) {
+                    Slog.e(TAG, "Unable to get running tasks", e);
+                    isBackgroundAuth = true;
+                }
+            }
+
+            // Fail authentication if we can't confirm the client activity is on top.
+            if (isBackgroundAuth) {
+                Slog.e(TAG, "Failing possible background authentication");
+                authenticated = false;
+
+                // SafetyNet logging for exploitation attempts of b/159249069.
+                final ApplicationInfo appInfo = getContext().getApplicationInfo();
+                EventLog.writeEvent(0x534e4554, "159249069", appInfo != null ? appInfo.uid : -1,
+                        "Attempted background authentication");
+            }
+
             if (authenticated) {
+                // SafetyNet logging for b/159249069 if constraint is violated.
+                if (isBackgroundAuth) {
+                    final ApplicationInfo appInfo = getContext().getApplicationInfo();
+                    EventLog.writeEvent(0x534e4554, "159249069", appInfo != null ? appInfo.uid : -1,
+                            "Successful background authentication!");
+                }
+
                 mAlreadyDone = true;
 
                 if (listener != null) {
diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java
index 330f99523507..9f0efa5fad83 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerService.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerService.java
@@ -299,6 +299,10 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
         final ArraySet<File> unclaimedStages = newArraySet(
                 stagingDir.listFiles(sStageFilter));
 
+        // We also need to clean up orphaned staging directory for staged sessions
+        final File stagedSessionStagingDir = Environment.getDataStagingDirectory(volumeUuid);
+        unclaimedStages.addAll(newArraySet(stagedSessionStagingDir.listFiles()));
+
         // Ignore stages claimed by active sessions
         for (int i = 0; i < mSessions.size(); i++) {
             final PackageInstallerSession session = mSessions.valueAt(i);
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index ea53132ae409..c9f09545c3e6 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -1581,6 +1581,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         destroyInternal();
         // Dispatch message to remove session from PackageInstallerService.
         dispatchSessionFinished(error, detailMessage, null);
+        // TODO(b/173194203): clean up staged session in destroyInternal() call instead
+        if (isStaged() && stageDir != null) {
+            cleanStageDir();
+        }
     }
 
     private void onStorageUnhealthy() {
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index c20a912152cd..99e1388dcd52 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -330,6 +330,7 @@ import com.android.internal.os.SomeArgs;
 import com.android.internal.os.Zygote;
 import com.android.internal.telephony.CarrierAppUtils;
 import com.android.internal.util.ArrayUtils;
+import com.android.internal.util.CollectionUtils;
 import com.android.internal.util.ConcurrentUtils;
 import com.android.internal.util.DumpUtils;
 import com.android.internal.util.FastXmlSerializer;
@@ -12441,12 +12442,17 @@ public class PackageManagerService extends IPackageManager.Stub
                 mPermissionManager.addAllPermissionGroups(pkg, chatty);
             }
 
+            // If a permission has had its defining app changed, or it has had its protection
+            // upgraded, we need to revoke apps that hold it
+            final List<String> permissionsWithChangedDefinition;
             // Don't allow ephemeral applications to define new permissions.
             if ((scanFlags & SCAN_AS_INSTANT_APP) != 0) {
+                permissionsWithChangedDefinition = null;
                 Slog.w(TAG, "Permissions from package " + pkg.getPackageName()
                         + " ignored: instant apps cannot define new permissions.");
             } else {
-                mPermissionManager.addAllPermissions(pkg, chatty);
+                permissionsWithChangedDefinition =
+                        mPermissionManager.addAllPermissions(pkg, chatty);
             }
 
             int collectionSize = ArrayUtils.size(pkg.getInstrumentations());
@@ -12475,7 +12481,10 @@ public class PackageManagerService extends IPackageManager.Stub
                 }
             }
 
-            if (oldPkg != null) {
+            boolean hasOldPkg = oldPkg != null;
+            boolean hasPermissionDefinitionChanges =
+                    !CollectionUtils.isEmpty(permissionsWithChangedDefinition);
+            if (hasOldPkg || hasPermissionDefinitionChanges) {
                 // We need to call revokeRuntimePermissionsIfGroupChanged async as permission
                 // revoke callbacks from this method might need to kill apps which need the
                 // mPackages lock on a different thread. This would dead lock.
@@ -12486,9 +12495,16 @@ public class PackageManagerService extends IPackageManager.Stub
                 // won't be granted yet, hence new packages are no problem.
                 final ArrayList<String> allPackageNames = new ArrayList<>(mPackages.keySet());
 
-                AsyncTask.execute(() ->
+                AsyncTask.execute(() -> {
+                    if (hasOldPkg) {
                         mPermissionManager.revokeRuntimePermissionsIfGroupChanged(pkg, oldPkg,
-                                allPackageNames));
+                                allPackageNames);
+                    }
+                    if (hasPermissionDefinitionChanges) {
+                        mPermissionManager.revokeRuntimePermissionsIfPermissionDefinitionChanged(
+                                permissionsWithChangedDefinition, allPackageNames);
+                    }
+                });
             }
         }
 
diff --git a/services/core/java/com/android/server/pm/permission/BasePermission.java b/services/core/java/com/android/server/pm/permission/BasePermission.java
index cfa0449aaf33..5e04171a3bca 100644
--- a/services/core/java/com/android/server/pm/permission/BasePermission.java
+++ b/services/core/java/com/android/server/pm/permission/BasePermission.java
@@ -83,6 +83,8 @@ public final class BasePermission {
 
     final @PermissionType int type;
 
+    private boolean mPermissionDefinitionChanged;
+
     String sourcePackageName;
 
     int protectionLevel;
@@ -126,6 +128,11 @@ public final class BasePermission {
     public String getSourcePackageName() {
         return sourcePackageName;
     }
+
+    public boolean isPermissionDefinitionChanged() {
+        return mPermissionDefinitionChanged;
+    }
+
     public int getType() {
         return type;
     }
@@ -140,6 +147,10 @@ public final class BasePermission {
         this.perm = perm;
     }
 
+    public void setPermissionDefinitionChanged(boolean shouldOverride) {
+        mPermissionDefinitionChanged = shouldOverride;
+    }
+
     public int[] computeGids(int userId) {
         if (perUser) {
             final int[] userGids = new int[gids.length];
@@ -322,6 +333,7 @@ public final class BasePermission {
         final PackageSettingBase pkgSetting =
                 (PackageSettingBase) packageManagerInternal.getPackageSetting(pkg.getPackageName());
         // Allow system apps to redefine non-system permissions
+        boolean ownerChanged = false;
         if (bp != null && !Objects.equals(bp.sourcePackageName, p.getPackageName())) {
             final boolean currentOwnerIsSystem;
             if (bp.perm == null) {
@@ -347,6 +359,7 @@ public final class BasePermission {
                     String msg = "New decl " + pkg + " of permission  "
                             + p.getName() + " is system; overriding " + bp.sourcePackageName;
                     PackageManagerService.reportSettingsProblem(Log.WARN, msg);
+                    ownerChanged = true;
                     bp = null;
                 }
             }
@@ -354,6 +367,7 @@ public final class BasePermission {
         if (bp == null) {
             bp = new BasePermission(p.getName(), p.getPackageName(), TYPE_NORMAL);
         }
+        boolean wasNonRuntime = !bp.isRuntime();
         StringBuilder r = null;
         if (bp.perm == null) {
             if (bp.sourcePackageName == null
@@ -397,6 +411,11 @@ public final class BasePermission {
                 && Objects.equals(bp.perm.getName(), p.getName())) {
             bp.protectionLevel = p.getProtectionLevel();
         }
+        if (bp.isRuntime() && (ownerChanged || wasNonRuntime)) {
+            // If this is a runtime permission and the owner has changed, or this was a normal
+            // permission, then permission state should be cleaned up
+            bp.mPermissionDefinitionChanged = true;
+        }
         if (PackageManagerService.DEBUG_PACKAGE_SCANNING && r != null) {
             Log.d(TAG, "  Permissions: " + r);
         }
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
index 66d8b5974261..3ffca028b1c0 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
@@ -2344,8 +2344,74 @@ public class PermissionManagerService extends IPermissionManager.Stub {
         }
     }
 
-    private void addAllPermissions(AndroidPackage pkg, boolean chatty) {
+    /**
+     * If permissions are upgraded to runtime, or their owner changes to the system, then any
+     * granted permissions must be revoked.
+     *
+     * @param permissionsToRevoke A list of permission names to revoke
+     * @param allPackageNames All package names
+     * @param permissionCallback Callback for permission changed
+     */
+    private void revokeRuntimePermissionsIfPermissionDefinitionChanged(
+            @NonNull List<String> permissionsToRevoke,
+            @NonNull ArrayList<String> allPackageNames,
+            @NonNull PermissionCallback permissionCallback) {
+
+        final int[] userIds = mUserManagerInt.getUserIds();
+        final int numPermissions = permissionsToRevoke.size();
+        final int numUserIds = userIds.length;
+        final int numPackages = allPackageNames.size();
+        final int callingUid = Binder.getCallingUid();
+
+        for (int permNum = 0; permNum < numPermissions; permNum++) {
+            String permName = permissionsToRevoke.get(permNum);
+            BasePermission bp = mSettings.getPermission(permName);
+            if (bp == null || !bp.isRuntime()) {
+                continue;
+            }
+            for (int userIdNum = 0; userIdNum < numUserIds; userIdNum++) {
+                final int userId = userIds[userIdNum];
+                for (int packageNum = 0; packageNum < numPackages; packageNum++) {
+                    final String packageName = allPackageNames.get(packageNum);
+                    final int uid = mPackageManagerInt.getPackageUid(packageName, 0, userId);
+                    if (uid < Process.FIRST_APPLICATION_UID) {
+                        // do not revoke from system apps
+                        continue;
+                    }
+                    final int permissionState = checkPermissionImpl(permName, packageName,
+                            userId);
+                    final int flags = getPermissionFlags(permName, packageName, userId);
+                    final int flagMask = FLAG_PERMISSION_SYSTEM_FIXED
+                            | FLAG_PERMISSION_POLICY_FIXED
+                            | FLAG_PERMISSION_GRANTED_BY_DEFAULT
+                            | FLAG_PERMISSION_GRANTED_BY_ROLE;
+                    if (permissionState == PackageManager.PERMISSION_GRANTED
+                            && (flags & flagMask) == 0) {
+                        EventLog.writeEvent(0x534e4554, "154505240", uid,
+                                "Revoking permission " + permName + " from package "
+                                        + packageName + " due to definition change");
+                        EventLog.writeEvent(0x534e4554, "168319670", uid,
+                                "Revoking permission " + permName + " from package "
+                                        + packageName + " due to definition change");
+                        Slog.e(TAG, "Revoking permission " + permName + " from package "
+                                + packageName + " due to definition change");
+                        try {
+                            revokeRuntimePermissionInternal(permName, packageName,
+                                    false, callingUid, userId, null, permissionCallback);
+                        } catch (Exception e) {
+                            Slog.e(TAG, "Could not revoke " + permName + " from "
+                                    + packageName, e);
+                        }
+                    }
+                }
+            }
+            bp.setPermissionDefinitionChanged(false);
+        }
+    }
+
+    private List<String> addAllPermissions(AndroidPackage pkg, boolean chatty) {
         final int N = ArrayUtils.size(pkg.getPermissions());
+        ArrayList<String> definitionChangedPermissions = new ArrayList<>();
         for (int i=0; i<N; i++) {
             ParsedPermission p = pkg.getPermissions().get(i);
 
@@ -2367,21 +2433,26 @@ public class PermissionManagerService extends IPermissionManager.Stub {
                     }
                 }
 
+                final BasePermission bp;
                 if (p.isTree()) {
-                    final BasePermission bp = BasePermission.createOrUpdate(
+                    bp = BasePermission.createOrUpdate(
                             mPackageManagerInt,
                             mSettings.getPermissionTreeLocked(p.getName()), p, pkg,
                             mSettings.getAllPermissionTreesLocked(), chatty);
                     mSettings.putPermissionTreeLocked(p.getName(), bp);
                 } else {
-                    final BasePermission bp = BasePermission.createOrUpdate(
+                    bp = BasePermission.createOrUpdate(
                             mPackageManagerInt,
                             mSettings.getPermissionLocked(p.getName()),
                             p, pkg, mSettings.getAllPermissionTreesLocked(), chatty);
                     mSettings.putPermissionLocked(p.getName(), bp);
                 }
+                if (bp.isPermissionDefinitionChanged()) {
+                    definitionChangedPermissions.add(p.getName());
+                }
             }
         }
+        return definitionChangedPermissions;
     }
 
     private void addAllPermissionGroups(AndroidPackage pkg, boolean chatty) {
@@ -4672,9 +4743,18 @@ public class PermissionManagerService extends IPermissionManager.Stub {
             PermissionManagerService.this.revokeRuntimePermissionsIfGroupChanged(newPackage,
                     oldPackage, allPackageNames, mDefaultPermissionCallback);
         }
+
+        @Override
+        public void revokeRuntimePermissionsIfPermissionDefinitionChanged(
+                @NonNull List<String> permissionsToRevoke,
+                @NonNull ArrayList<String> allPackageNames) {
+            PermissionManagerService.this.revokeRuntimePermissionsIfPermissionDefinitionChanged(
+                    permissionsToRevoke, allPackageNames, mDefaultPermissionCallback);
+        }
+
         @Override
-        public void addAllPermissions(AndroidPackage pkg, boolean chatty) {
-            PermissionManagerService.this.addAllPermissions(pkg, chatty);
+        public List<String> addAllPermissions(AndroidPackage pkg, boolean chatty) {
+            return PermissionManagerService.this.addAllPermissions(pkg, chatty);
         }
         @Override
         public void addAllPermissionGroups(AndroidPackage pkg, boolean chatty) {
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java
index 2e83b23f57d8..31a65ba35f5e 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java
@@ -254,12 +254,26 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager
             @NonNull ArrayList<String> allPackageNames);
 
     /**
+     * Some permissions might have been owned by a non-system package, and the system then defined
+     * said permission. Some other permissions may one have been install permissions, but are now
+     * runtime or higher. These permissions should be revoked.
+     *
+     * @param permissionsToRevoke A list of permission names to revoke
+     * @param allPackageNames All packages
+     */
+    public abstract void revokeRuntimePermissionsIfPermissionDefinitionChanged(
+            @NonNull List<String> permissionsToRevoke,
+            @NonNull ArrayList<String> allPackageNames);
+
+    /**
      * Add all permissions in the given package.
      * <p>
      * NOTE: argument {@code groupTEMP} is temporary until mPermissionGroups is moved to
      * the permission settings.
+     *
+     * @return A list of BasePermissions that were updated, and need to be revoked from packages
      */
-    public abstract void addAllPermissions(@NonNull AndroidPackage pkg, boolean chatty);
+    public abstract List<String> addAllPermissions(@NonNull AndroidPackage pkg, boolean chatty);
     public abstract void addAllPermissionGroups(@NonNull AndroidPackage pkg, boolean chatty);
     public abstract void removeAllPermissions(@NonNull AndroidPackage pkg, boolean chatty);
 
diff --git a/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java b/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java
index 02597d548361..e67354982b05 100644
--- a/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java
+++ b/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java
@@ -96,6 +96,19 @@ public class StagedInstallInternalTest {
         assertSessionReady(sessionId);
     }
 
+    @Test
+    public void testStagedInstallationShouldCleanUpOnValidationFailure() throws Exception {
+        InstallUtils.commitExpectingFailure(AssertionError.class, "INSTALL_FAILED_INVALID_APK",
+                Install.single(TestApp.AIncompleteSplit).setStaged());
+    }
+
+    @Test
+    public void testStagedInstallationShouldCleanUpOnValidationFailureMultiPackage()
+            throws Exception {
+        InstallUtils.commitExpectingFailure(AssertionError.class, "INSTALL_FAILED_INVALID_APK",
+                Install.multi(TestApp.AIncompleteSplit, TestApp.B1, TestApp.Apex1).setStaged());
+    }
+
     private static void assertSessionReady(int sessionId) {
         assertSessionState(sessionId,
                 (session) -> assertThat(session.isStagedSessionReady()).isTrue());
diff --git a/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java b/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java
index 9b432f7d0ca5..28a5424199a4 100644
--- a/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java
+++ b/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java
@@ -21,6 +21,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertTrue;
 
 import com.android.ddmlib.Log;
+import com.android.tradefed.device.DeviceNotAvailableException;
 import com.android.tradefed.testtype.DeviceJUnit4ClassRunner;
 import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
 import com.android.tradefed.util.ProcessInfo;
@@ -30,6 +31,10 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
 @RunWith(DeviceJUnit4ClassRunner.class)
 public class StagedInstallInternalTest extends BaseHostJUnit4Test {
 
@@ -87,6 +92,52 @@ public class StagedInstallInternalTest extends BaseHostJUnit4Test {
         runPhase("testSystemServerRestartDoesNotAffectStagedSessions_Verify");
     }
 
+    @Test
+    public void testStagedInstallationShouldCleanUpOnValidationFailure() throws Exception {
+        List<String> before = getStagingDirectories();
+        runPhase("testStagedInstallationShouldCleanUpOnValidationFailure");
+        List<String> after = getStagingDirectories();
+        assertThat(after).isEqualTo(before);
+    }
+
+    @Test
+    public void testStagedInstallationShouldCleanUpOnValidationFailureMultiPackage()
+            throws Exception {
+        List<String> before = getStagingDirectories();
+        runPhase("testStagedInstallationShouldCleanUpOnValidationFailureMultiPackage");
+        List<String> after = getStagingDirectories();
+        assertThat(after).isEqualTo(before);
+    }
+
+    @Test
+    public void testOrphanedStagingDirectoryGetsCleanedUpOnReboot() throws Exception {
+        //create random directories in /data/app-staging folder
+        getDevice().enableAdbRoot();
+        getDevice().executeShellCommand("mkdir /data/app-staging/session_123");
+        getDevice().executeShellCommand("mkdir /data/app-staging/random_name");
+        getDevice().disableAdbRoot();
+
+        assertThat(getStagingDirectories()).isNotEmpty();
+        getDevice().reboot();
+        assertThat(getStagingDirectories()).isEmpty();
+    }
+
+    private List<String> getStagingDirectories() throws DeviceNotAvailableException {
+        String baseDir = "/data/app-staging";
+        try {
+            getDevice().enableAdbRoot();
+            return getDevice().getFileEntry(baseDir).getChildren(false)
+                    .stream().filter(entry -> entry.getName().matches("session_\\d+"))
+                    .map(entry -> entry.getName())
+                    .collect(Collectors.toList());
+        } catch (Exception e) {
+            // Return an empty list if any error
+            return Collections.EMPTY_LIST;
+        } finally {
+            getDevice().disableAdbRoot();
+        }
+    }
+
     private void restartSystemServer() throws Exception {
         // Restart the system server
         long oldStartTime = getDevice().getProcessByName("system_server").getStartTime();