diff options
5 files changed, 66 insertions, 17 deletions
diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp index 7114dfc79a84..9556333dbf86 100644 --- a/core/jni/android_util_Binder.cpp +++ b/core/jni/android_util_Binder.cpp @@ -156,8 +156,9 @@ static struct thread_dispatch_offsets_t static constexpr int32_t PROXY_WARN_INTERVAL = 5000; static constexpr uint32_t GC_INTERVAL = 1000; -static std::atomic<uint32_t> gNumProxies(0); -static std::atomic<uint32_t> gProxiesWarned(0); +// Protected by gProxyLock. We warn if this gets too large. +static int32_t gNumProxies = 0; +static int32_t gProxiesWarned = 0; // Number of GlobalRefs held by JavaBBinders. static std::atomic<uint32_t> gNumLocalRefsCreated(0); @@ -659,6 +660,12 @@ BinderProxyNativeData* getBPNativeData(JNIEnv* env, jobject obj) { return (BinderProxyNativeData *) env->GetLongField(obj, gBinderProxyOffsets.mNativeData); } +static Mutex gProxyLock; + +// We may cache a single BinderProxyNativeData node to avoid repeat allocation. +// All fields are null. Protected by gProxyLock. +static BinderProxyNativeData *gNativeDataCache; + // If the argument is a JavaBBinder, return the Java object that was used to create it. // Otherwise return a BinderProxy for the IBinder. If a previous call was passed the // same IBinder, and the original BinderProxy is still alive, return the same BinderProxy. @@ -673,31 +680,36 @@ jobject javaObjectForIBinder(JNIEnv* env, const sp<IBinder>& val) return object; } - BinderProxyNativeData* nativeData = new BinderProxyNativeData(); - nativeData->mOrgue = new DeathRecipientList; - nativeData->mObject = val; + // For the rest of the function we will hold this lock, to serialize + // looking/creation/destruction of Java proxies for native Binder proxies. + AutoMutex _l(gProxyLock); + BinderProxyNativeData* nativeData = gNativeDataCache; + if (nativeData == nullptr) { + nativeData = new BinderProxyNativeData(); + } + // gNativeDataCache is now logically empty. jobject object = env->CallStaticObjectMethod(gBinderProxyOffsets.mClass, gBinderProxyOffsets.mGetInstance, (jlong) nativeData, (jlong) val.get()); if (env->ExceptionCheck()) { // In the exception case, getInstance still took ownership of nativeData. + gNativeDataCache = nullptr; return NULL; } BinderProxyNativeData* actualNativeData = getBPNativeData(env, object); if (actualNativeData == nativeData) { - // Created a new Proxy - uint32_t numProxies = gNumProxies.fetch_add(1, std::memory_order_relaxed); - uint32_t numLastWarned = gProxiesWarned.load(std::memory_order_relaxed); - if (numProxies >= numLastWarned + PROXY_WARN_INTERVAL) { - // Multiple threads can get here, make sure only one of them gets to - // update the warn counter. - if (gProxiesWarned.compare_exchange_strong(numLastWarned, - numLastWarned + PROXY_WARN_INTERVAL, std::memory_order_relaxed)) { - ALOGW("Unexpectedly many live BinderProxies: %d\n", numProxies); - } + // New BinderProxy; we still have exclusive access. + nativeData->mOrgue = new DeathRecipientList; + nativeData->mObject = val; + gNativeDataCache = nullptr; + ++gNumProxies; + if (gNumProxies >= gProxiesWarned + PROXY_WARN_INTERVAL) { + ALOGW("Unexpectedly many live BinderProxies: %d\n", gNumProxies); + gProxiesWarned = gNumProxies; } } else { - delete nativeData; + // nativeData wasn't used. Reuse it the next time. + gNativeDataCache = nativeData; } return object; @@ -977,7 +989,8 @@ jint android_os_Debug_getLocalObjectCount(JNIEnv* env, jobject clazz) jint android_os_Debug_getProxyObjectCount(JNIEnv* env, jobject clazz) { - return gNumProxies.load(); + AutoMutex _l(gProxyLock); + return gNumProxies; } jint android_os_Debug_getDeathObjectCount(JNIEnv* env, jobject clazz) @@ -1372,6 +1385,9 @@ static jboolean android_os_BinderProxy_unlinkToDeath(JNIEnv* env, jobject obj, static void BinderProxy_destroy(void* rawNativeData) { + // Don't race with construction/initialization + AutoMutex _l(gProxyLock); + BinderProxyNativeData * nativeData = (BinderProxyNativeData *) rawNativeData; LOGDEATH("Destroying BinderProxy: binder=%p drl=%p\n", nativeData->mObject.get(), nativeData->mOrgue.get()); diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 58a1dc146808..0188f7c8bfaa 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -2189,6 +2189,11 @@ public class NotificationManagerService extends SystemService { @Override public boolean areNotificationsEnabledForPackage(String pkg, int uid) { checkCallerIsSystemOrSameApp(pkg); + if (UserHandle.getCallingUserId() != UserHandle.getUserId(uid)) { + getContext().enforceCallingPermission( + android.Manifest.permission.INTERACT_ACROSS_USERS, + "canNotifyAsPackage for uid " + uid); + } return mRankingHelper.getImportance(pkg, uid) != IMPORTANCE_NONE; } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 9bb417c50905..f938b65b48c3 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -18305,6 +18305,12 @@ public class PackageManagerService extends IPackageManager.Stub @Override public boolean isPackageDeviceAdminOnAnyUser(String packageName) { final int callingUid = Binder.getCallingUid(); + if (checkUidPermission(android.Manifest.permission.MANAGE_USERS, callingUid) + != PERMISSION_GRANTED) { + EventLog.writeEvent(0x534e4554, "128599183", -1, ""); + throw new SecurityException(android.Manifest.permission.MANAGE_USERS + + " permission is required to call this API"); + } if (getInstantAppPackageName(callingUid) != null && !isCallerSameApp(packageName, callingUid)) { return false; diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 9c6b52fb2349..d3b25fd1afa0 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -3934,6 +3934,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { @Override public boolean isSeparateProfileChallengeAllowed(int userHandle) { + if (!isCallerWithSystemUid()) { + throw new SecurityException("Caller must be system"); + } ComponentName profileOwner = getProfileOwner(userHandle); // Profile challenge is supported on N or newer release. return profileOwner != null && diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index f02c3f062f35..5622622e925e 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -34,6 +34,7 @@ import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_SCREEN_ON import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_STATUS_BAR; import static android.content.pm.PackageManager.FEATURE_WATCH; import static android.content.pm.PackageManager.PERMISSION_DENIED; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.os.Build.VERSION_CODES.O_MR1; import static android.os.Build.VERSION_CODES.P; @@ -106,6 +107,7 @@ import android.testing.AndroidTestingRunner; import android.testing.TestableContext; import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; +import android.testing.TestablePermissions; import android.text.Html; import android.util.ArrayMap; import android.util.AtomicFile; @@ -3145,4 +3147,21 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals(0, captor.getValue().getNotification().flags); } + + @Test + public void testAreNotificationsEnabledForPackage_crossUser() throws Exception { + try { + mBinderService.areNotificationsEnabledForPackage(mContext.getPackageName(), + mUid + UserHandle.PER_USER_RANGE); + fail("Cannot call cross user without permission"); + } catch (SecurityException e) { + // pass + } + + // cross user, with permission, no problem + TestablePermissions perms = mContext.getTestablePermissions(); + perms.setPermission(android.Manifest.permission.INTERACT_ACROSS_USERS, PERMISSION_GRANTED); + mBinderService.areNotificationsEnabledForPackage(mContext.getPackageName(), + mUid + UserHandle.PER_USER_RANGE); + } } |