From 878d0b6b2407d698f9a9daddd2e827dc7d6536a0 Mon Sep 17 00:00:00 2001
From: Wei Wang <weiwa@google.com>
Date: Thu, 28 Mar 2019 18:12:18 -0700
Subject: [PATCH] Fix a few issues with foreground service location accesses.

Location access should be allowed only when the UID state is higher than
UID_STATE_FOREGROUND_SERVICE_LOCATION. UID_STATE_FOREGROUND_SERVICE
should not have location access.

Also fixed an issue where MONITOR_LOCATION is allowed for location access
from foreground services.

Since UID_STATE_FOREGROUND_SERVICE is considered as background for
location, and foreground for other ops, we cannot simply use UID state
to check the foreground status of an app. Instead, the uid_state <-> op
pair needs to be evaluated together.

Bug: 128520624
Test: Manually run GNSS logger app to verify
Change-Id: I9413e9c6009e38e3d9db57a02e8f0a275119b063
---
 core/java/android/app/AppOpsManager.java          |  4 +++-
 .../com/android/server/appop/AppOpsService.java   | 15 ++++++++-------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java
index 5ed4428b729b4..a12512de7ee81 100644
--- a/core/java/android/app/AppOpsManager.java
+++ b/core/java/android/app/AppOpsManager.java
@@ -332,7 +332,9 @@ public class AppOpsManager {
     public static int resolveFirstUnrestrictedUidState(int op) {
         switch (op) {
             case OP_FINE_LOCATION:
-            case OP_COARSE_LOCATION: {
+            case OP_COARSE_LOCATION:
+            case OP_MONITOR_LOCATION:
+            case OP_MONITOR_HIGH_POWER_LOCATION: {
                 return UID_STATE_FOREGROUND_SERVICE_LOCATION;
             }
         }
diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java
index 2e5dd3b0941e7..db1e682a96dc4 100644
--- a/services/core/java/com/android/server/appop/AppOpsService.java
+++ b/services/core/java/com/android/server/appop/AppOpsService.java
@@ -340,7 +340,7 @@ public class AppOpsService extends IAppOpsService.Stub {
 
         int evalMode(int op, int mode) {
             if (mode == AppOpsManager.MODE_FOREGROUND) {
-                return state <= AppOpsManager.resolveLastRestrictedUidState(op)
+                return state <= AppOpsManager.resolveFirstUnrestrictedUidState(op)
                         ? AppOpsManager.MODE_ALLOWED : AppOpsManager.MODE_IGNORED;
             }
             return mode;
@@ -914,9 +914,12 @@ public class AppOpsService extends IAppOpsService.Stub {
             if (uidState != null && uidState.pendingState != newState) {
                 final int oldPendingState = uidState.pendingState;
                 uidState.pendingState = newState;
-                if (newState < uidState.state || newState <= UID_STATE_MAX_LAST_NON_RESTRICTED) {
-                    // We are moving to a more important state, or the new state is in the
-                    // foreground, then always do it immediately.
+                if (newState < uidState.state
+                        || (newState <= UID_STATE_MAX_LAST_NON_RESTRICTED
+                                && uidState.state > UID_STATE_MAX_LAST_NON_RESTRICTED)) {
+                    // We are moving to a more important state, or the new state may be in the
+                    // foreground and the old state is in the background, then always do it
+                    // immediately.
                     commitUidPendingStateLocked(uidState);
                 } else if (uidState.pendingStateCommitTime == 0) {
                     // We are moving to a less important state for the first time,
@@ -2413,9 +2416,7 @@ public class AppOpsService extends IAppOpsService.Stub {
     }
 
     private void commitUidPendingStateLocked(UidState uidState) {
-        final boolean lastForeground = uidState.state <= UID_STATE_MAX_LAST_NON_RESTRICTED;
-        final boolean nowForeground = uidState.pendingState <= UID_STATE_MAX_LAST_NON_RESTRICTED;
-        if (uidState.hasForegroundWatchers && lastForeground != nowForeground) {
+        if (uidState.hasForegroundWatchers) {
             for (int fgi = uidState.foregroundOps.size() - 1; fgi >= 0; fgi--) {
                 if (!uidState.foregroundOps.valueAt(fgi)) {
                     continue;