Revert "Revert "SF: Updating permissions checking in Surface Flinger.""
This reverts commit e70e129fcb2635d48582ecf4ffda8d54b76cc146.
Reason for revert: Try 2. Updating GetDisplayColorModes to generally accessible.
Test: SF tests pass. Updating GetDisplayColorModeTest.
Change-Id: I4b4aea7b4b2e51caf217a254778821e28a6450c9
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 6b222d5..76f6d4d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1220,15 +1220,6 @@
status_t SurfaceFlinger::getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) const
NO_THREAD_SAFETY_ANALYSIS {
- IPCThreadState* ipc = IPCThreadState::self();
- const int pid = ipc->getCallingPid();
- const int uid = ipc->getCallingUid();
- if ((uid != AID_SHELL) &&
- !PermissionCache::checkPermission(sDump, pid, uid)) {
- ALOGE("Layer debug info permission denied for pid=%d, uid=%d", pid, uid);
- return PERMISSION_DENIED;
- }
-
// Try to acquire a lock for 1s, fail gracefully
const status_t err = mStateLock.timedLock(s2ns(1));
const bool locked = (err == NO_ERROR);
@@ -3375,9 +3366,8 @@
IPCThreadState* ipc = IPCThreadState::self();
const int pid = ipc->getCallingPid();
const int uid = ipc->getCallingUid();
-
if ((uid != AID_GRAPHICS && uid != AID_SYSTEM) &&
- !PermissionCache::checkPermission(sAccessSurfaceFlinger, pid, uid)) {
+ !PermissionCache::checkPermission(sAccessSurfaceFlinger, pid, uid)) {
return false;
}
return true;
@@ -4563,51 +4553,64 @@
}
status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) {
- switch (code) {
- case CREATE_CONNECTION:
- case CREATE_DISPLAY:
+#pragma clang diagnostic push
+#pragma clang diagnostic error "-Wswitch-enum"
+ switch (static_cast<ISurfaceComposerTag>(code)) {
+ // These methods should at minimum make sure that the client requested
+ // access to SF.
+ case AUTHENTICATE_SURFACE:
case BOOT_FINISHED:
case CLEAR_ANIMATION_FRAME_STATS:
- case GET_ANIMATION_FRAME_STATS:
- case SET_POWER_MODE:
- case GET_HDR_CAPABILITIES:
+ case CREATE_CONNECTION:
+ case CREATE_DISPLAY:
+ case DESTROY_DISPLAY:
case ENABLE_VSYNC_INJECTIONS:
+ case GET_ACTIVE_COLOR_MODE:
+ case GET_ANIMATION_FRAME_STATS:
+ case GET_HDR_CAPABILITIES:
+ case SET_ACTIVE_CONFIG:
+ case SET_ACTIVE_COLOR_MODE:
case INJECT_VSYNC:
- {
- // codes that require permission check
+ case SET_POWER_MODE: {
if (!callingThreadHasUnscopedSurfaceFlingerAccess()) {
IPCThreadState* ipc = IPCThreadState::self();
ALOGE("Permission Denial: can't access SurfaceFlinger pid=%d, uid=%d",
ipc->getCallingPid(), ipc->getCallingUid());
return PERMISSION_DENIED;
}
- break;
- }
- /*
- * Calling setTransactionState is safe, because you need to have been
- * granted a reference to Client* and Handle* to do anything with it.
- *
- * Creating a scoped connection is safe, as per discussion in ISurfaceComposer.h
- */
- case SET_TRANSACTION_STATE:
- case CREATE_SCOPED_CONNECTION:
- {
return OK;
}
- case CAPTURE_SCREEN:
- {
- // codes that require permission check
+ case GET_LAYER_DEBUG_INFO: {
IPCThreadState* ipc = IPCThreadState::self();
const int pid = ipc->getCallingPid();
const int uid = ipc->getCallingUid();
- if ((uid != AID_GRAPHICS) &&
- !PermissionCache::checkPermission(sReadFramebuffer, pid, uid)) {
- ALOGE("Permission Denial: can't read framebuffer pid=%d, uid=%d", pid, uid);
+ if ((uid != AID_SHELL) && !PermissionCache::checkPermission(sDump, pid, uid)) {
+ ALOGE("Layer debug info permission denied for pid=%d, uid=%d", pid, uid);
return PERMISSION_DENIED;
}
- break;
+ return OK;
}
- case CAPTURE_LAYERS: {
+ // Used by apps to hook Choreographer to SurfaceFlinger.
+ case CREATE_DISPLAY_EVENT_CONNECTION:
+ // The following calls are currently used by clients that do not
+ // request necessary permissions. However, they do not expose any secret
+ // information, so it is OK to pass them.
+ case GET_ACTIVE_CONFIG:
+ case GET_BUILT_IN_DISPLAY:
+ case GET_DISPLAY_COLOR_MODES:
+ case GET_DISPLAY_CONFIGS:
+ case GET_DISPLAY_STATS:
+ case GET_SUPPORTED_FRAME_TIMESTAMPS:
+ // Calling setTransactionState is safe, because you need to have been
+ // granted a reference to Client* and Handle* to do anything with it.
+ case SET_TRANSACTION_STATE:
+ // Creating a scoped connection is safe, as per discussion in ISurfaceComposer.h
+ case CREATE_SCOPED_CONNECTION: {
+ return OK;
+ }
+ case CAPTURE_LAYERS:
+ case CAPTURE_SCREEN: {
+ // codes that require permission check
IPCThreadState* ipc = IPCThreadState::self();
const int pid = ipc->getCallingPid();
const int uid = ipc->getCallingUid();
@@ -4616,15 +4619,37 @@
ALOGE("Permission Denial: can't read framebuffer pid=%d, uid=%d", pid, uid);
return PERMISSION_DENIED;
}
- break;
+ return OK;
+ }
+ // The following codes are deprecated and should never be allowed to access SF.
+ case CONNECT_DISPLAY_UNUSED:
+ case CREATE_GRAPHIC_BUFFER_ALLOC_UNUSED: {
+ ALOGE("Attempting to access SurfaceFlinger with unused code: %u", code);
+ return PERMISSION_DENIED;
}
}
- return OK;
+
+ // These codes are used for the IBinder protocol to either interrogate the recipient
+ // side of the transaction for its canonical interface descriptor or to dump its state.
+ // We let them pass by default.
+ if (code == IBinder::INTERFACE_TRANSACTION || code == IBinder::DUMP_TRANSACTION ||
+ code == IBinder::PING_TRANSACTION || code == IBinder::SHELL_COMMAND_TRANSACTION ||
+ code == IBinder::SYSPROPS_TRANSACTION) {
+ return OK;
+ }
+ // Numbers from 1000 to 1029 are currently use for backdoors. The code
+ // in onTransact verifies that the user is root, and has access to use SF.
+ if (code >= 1000 && code <= 1029) {
+ ALOGV("Accessing SurfaceFlinger through backdoor code: %u", code);
+ return OK;
+ }
+ ALOGE("Permission Denial: SurfaceFlinger did not recognize request code: %u", code);
+ return PERMISSION_DENIED;
+#pragma clang diagnostic pop
}
-status_t SurfaceFlinger::onTransact(
- uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags)
-{
+status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* reply,
+ uint32_t flags) {
status_t credentialCheck = CheckTransactCodeCredentials(code);
if (credentialCheck != OK) {
return credentialCheck;