Appops refcount fixup
Fix two issues in Iea630bb2bf1b54697ab6a95ee9f65db5cc4d134a
1. Fix boolean return values in the startRecording method, which
implicitly converted incorrectly.
2. Correctly handle virtual sources by not deriving their silence state
from the existing checkPerm method, which checks OP_RECORD/RECORD
perm. These sources should instead check only their associated op
state.
For the second, update the appop callback listening/handling as well, by
only following/evaluating OP_RECORD for relevant sources.
Test: CtsMediaAudioPermissionTestCases
Bug: 293603271
Bug: 374800336
Bug: 374869085
Flag: EXEMPT security
Change-Id: I8a3498a08a31543219279cb063c043fd0c767cd7
diff --git a/media/utils/ServiceUtilities.cpp b/media/utils/ServiceUtilities.cpp
index 679b111..ddb93fe 100644
--- a/media/utils/ServiceUtilities.cpp
+++ b/media/utils/ServiceUtilities.cpp
@@ -45,6 +45,7 @@
namespace android {
namespace {
+constexpr auto PERMISSION_GRANTED = permission::PermissionChecker::PERMISSION_GRANTED;
constexpr auto PERMISSION_HARD_DENIED = permission::PermissionChecker::PERMISSION_HARD_DENIED;
}
@@ -78,19 +79,32 @@
int32_t getOpForSource(audio_source_t source) {
switch (source) {
- case AUDIO_SOURCE_HOTWORD:
- return AppOpsManager::OP_RECORD_AUDIO_HOTWORD;
+ case AUDIO_SOURCE_FM_TUNER:
+ return AppOpsManager::OP_NONE;
case AUDIO_SOURCE_ECHO_REFERENCE: // fallthrough
case AUDIO_SOURCE_REMOTE_SUBMIX:
return AppOpsManager::OP_RECORD_AUDIO_OUTPUT;
case AUDIO_SOURCE_VOICE_DOWNLINK:
return AppOpsManager::OP_RECORD_INCOMING_PHONE_AUDIO;
+ case AUDIO_SOURCE_HOTWORD:
+ return AppOpsManager::OP_RECORD_AUDIO_HOTWORD;
case AUDIO_SOURCE_DEFAULT:
default:
return AppOpsManager::OP_RECORD_AUDIO;
}
}
+bool isRecordOpRequired(audio_source_t source) {
+ switch (source) {
+ case AUDIO_SOURCE_FM_TUNER:
+ case AUDIO_SOURCE_ECHO_REFERENCE: // fallthrough
+ case AUDIO_SOURCE_REMOTE_SUBMIX:
+ return false;
+ default:
+ return true;
+ }
+}
+
std::optional<AttributionSourceState> resolveAttributionSource(
const AttributionSourceState& callerAttributionSource, const uint32_t virtualDeviceId) {
AttributionSourceState nextAttributionSource = callerAttributionSource;
@@ -122,7 +136,8 @@
return std::optional<AttributionSourceState>{myAttributionSource};
}
- static int checkRecordingInternal(const AttributionSourceState &attributionSource,
+
+static int checkRecordingInternal(const AttributionSourceState &attributionSource,
const uint32_t virtualDeviceId,
const String16 &msg, bool start, audio_source_t source) {
// Okay to not track in app ops as audio server or media server is us and if
@@ -131,32 +146,47 @@
// user is active, but it is a core system service so let it through.
// TODO(b/141210120): UserManager.DISALLOW_RECORD_AUDIO should not affect system user 0
uid_t uid = VALUE_OR_FATAL(aidl2legacy_int32_t_uid_t(attributionSource.uid));
- if (isAudioServerOrMediaServerOrSystemServerOrRootUid(uid)) return true;
-
- // We specify a pid and uid here as mediaserver (aka MediaRecorder or StageFrightRecorder)
- // may open a record track on behalf of a client. Note that pid may be a tid.
- // IMPORTANT: DON'T USE PermissionCache - RUNTIME PERMISSIONS CHANGE.
- std::optional<AttributionSourceState> resolvedAttributionSource =
- resolveAttributionSource(attributionSource, virtualDeviceId);
- if (!resolvedAttributionSource.has_value()) {
- return false;
- }
+ if (isAudioServerOrMediaServerOrSystemServerOrRootUid(uid)) return PERMISSION_GRANTED;
const int32_t attributedOpCode = getOpForSource(source);
+ if (isRecordOpRequired(source)) {
+ // We specify a pid and uid here as mediaserver (aka MediaRecorder or StageFrightRecorder)
+ // may open a record track on behalf of a client. Note that pid may be a tid.
+ // IMPORTANT: DON'T USE PermissionCache - RUNTIME PERMISSIONS CHANGE.
+ std::optional<AttributionSourceState> resolvedAttributionSource =
+ resolveAttributionSource(attributionSource, virtualDeviceId);
+ if (!resolvedAttributionSource.has_value()) {
+ return PERMISSION_HARD_DENIED;
+ }
- permission::PermissionChecker permissionChecker;
- int permitted;
- if (start) {
- permitted = permissionChecker.checkPermissionForStartDataDeliveryFromDatasource(
- sAndroidPermissionRecordAudio, resolvedAttributionSource.value(), msg,
- attributedOpCode);
+ permission::PermissionChecker permissionChecker;
+ int permitted;
+ if (start) {
+ permitted = permissionChecker.checkPermissionForStartDataDeliveryFromDatasource(
+ sAndroidPermissionRecordAudio, resolvedAttributionSource.value(), msg,
+ attributedOpCode);
+ } else {
+ permitted = permissionChecker.checkPermissionForPreflightFromDatasource(
+ sAndroidPermissionRecordAudio, resolvedAttributionSource.value(), msg,
+ attributedOpCode);
+ }
+
+ return permitted;
} else {
- permitted = permissionChecker.checkPermissionForPreflightFromDatasource(
- sAndroidPermissionRecordAudio, resolvedAttributionSource.value(), msg,
- attributedOpCode);
+ if (attributedOpCode == AppOpsManager::OP_NONE) return PERMISSION_GRANTED; // nothing to do
+ AppOpsManager ap{};
+ PermissionController pc{};
+ return ap.startOpNoThrow(
+ attributedOpCode, attributionSource.uid,
+ resolveCallingPackage(pc,
+ String16{attributionSource.packageName.value_or("").c_str()},
+ attributionSource.uid),
+ false,
+ attributionSource.attributionTag.has_value()
+ ? String16{attributionSource.attributionTag.value().c_str()}
+ : String16{},
+ msg);
}
-
- return permitted;
}
static constexpr int DEVICE_ID_DEFAULT = 0;
@@ -188,19 +218,32 @@
uid_t uid = VALUE_OR_FATAL(aidl2legacy_int32_t_uid_t(attributionSource.uid));
if (isAudioServerOrMediaServerOrSystemServerOrRootUid(uid)) return;
- // We specify a pid and uid here as mediaserver (aka MediaRecorder or StageFrightRecorder)
- // may open a record track on behalf of a client. Note that pid may be a tid.
- // IMPORTANT: DON'T USE PermissionCache - RUNTIME PERMISSIONS CHANGE.
- const std::optional<AttributionSourceState> resolvedAttributionSource =
- resolveAttributionSource(attributionSource, virtualDeviceId);
- if (!resolvedAttributionSource.has_value()) {
- return;
- }
-
const int32_t attributedOpCode = getOpForSource(source);
- permission::PermissionChecker permissionChecker;
- permissionChecker.finishDataDeliveryFromDatasource(attributedOpCode,
- resolvedAttributionSource.value());
+ if (isRecordOpRequired(source)) {
+ // We specify a pid and uid here as mediaserver (aka MediaRecorder or StageFrightRecorder)
+ // may open a record track on behalf of a client. Note that pid may be a tid.
+ // IMPORTANT: DON'T USE PermissionCache - RUNTIME PERMISSIONS CHANGE.
+ const std::optional<AttributionSourceState> resolvedAttributionSource =
+ resolveAttributionSource(attributionSource, virtualDeviceId);
+ if (!resolvedAttributionSource.has_value()) {
+ return;
+ }
+
+ permission::PermissionChecker permissionChecker;
+ permissionChecker.finishDataDeliveryFromDatasource(attributedOpCode,
+ resolvedAttributionSource.value());
+ } else {
+ if (attributedOpCode == AppOpsManager::OP_NONE) return; // nothing to do
+ AppOpsManager ap{};
+ PermissionController pc{};
+ ap.finishOp(attributedOpCode, attributionSource.uid,
+ resolveCallingPackage(
+ pc, String16{attributionSource.packageName.value_or("").c_str()},
+ attributionSource.uid),
+ attributionSource.attributionTag.has_value()
+ ? String16{attributionSource.attributionTag.value().c_str()}
+ : String16{});
+ }
}
bool captureAudioOutputAllowed(const AttributionSourceState& attributionSource) {