Fix audio AppOps refcount mismatch
The state of the appop corresponding to a recording must be consistent
with the APS notion of whether a recording is silenced.
Ensure that when we start recording, we only un-silence the track if the
op is allowed from the appop side. This requires plumbing the exact perm
state through the startRecording API.
Test: atest AudioRecordPermissionTest.java
Bug: 293603271
Flag: EXEMPT security
Change-Id: Iea630bb2bf1b54697ab6a95ee9f65db5cc4d134a
diff --git a/media/utils/ServiceUtilities.cpp b/media/utils/ServiceUtilities.cpp
index 0315ac9..e13f8f7 100644
--- a/media/utils/ServiceUtilities.cpp
+++ b/media/utils/ServiceUtilities.cpp
@@ -41,6 +41,10 @@
namespace android {
+namespace {
+constexpr auto PERMISSION_HARD_DENIED = permission::PermissionChecker::PERMISSION_HARD_DENIED;
+}
+
using content::AttributionSourceState;
static const String16 sAndroidPermissionRecordAudio("android.permission.RECORD_AUDIO");
@@ -115,7 +119,7 @@
return std::optional<AttributionSourceState>{myAttributionSource};
}
- static bool 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
@@ -138,15 +142,15 @@
const int32_t attributedOpCode = getOpForSource(source);
permission::PermissionChecker permissionChecker;
- bool permitted = false;
+ int permitted;
if (start) {
- permitted = (permissionChecker.checkPermissionForStartDataDeliveryFromDatasource(
+ permitted = permissionChecker.checkPermissionForStartDataDeliveryFromDatasource(
sAndroidPermissionRecordAudio, resolvedAttributionSource.value(), msg,
- attributedOpCode) != permission::PermissionChecker::PERMISSION_HARD_DENIED);
+ attributedOpCode);
} else {
- permitted = (permissionChecker.checkPermissionForPreflightFromDatasource(
+ permitted = permissionChecker.checkPermissionForPreflightFromDatasource(
sAndroidPermissionRecordAudio, resolvedAttributionSource.value(), msg,
- attributedOpCode) != permission::PermissionChecker::PERMISSION_HARD_DENIED);
+ attributedOpCode);
}
return permitted;
@@ -156,17 +160,17 @@
bool recordingAllowed(const AttributionSourceState &attributionSource, audio_source_t source) {
return checkRecordingInternal(attributionSource, DEVICE_ID_DEFAULT, String16(), /*start*/ false,
- source);
+ source) != PERMISSION_HARD_DENIED;
}
bool recordingAllowed(const AttributionSourceState &attributionSource,
const uint32_t virtualDeviceId,
audio_source_t source) {
return checkRecordingInternal(attributionSource, virtualDeviceId,
- String16(), /*start*/ false, source);
+ String16(), /*start*/ false, source) != PERMISSION_HARD_DENIED;
}
-bool startRecording(const AttributionSourceState& attributionSource,
+int startRecording(const AttributionSourceState& attributionSource,
const uint32_t virtualDeviceId,
const String16& msg,
audio_source_t source) {
diff --git a/media/utils/include/mediautils/ServiceUtilities.h b/media/utils/include/mediautils/ServiceUtilities.h
index b365648..2631469 100644
--- a/media/utils/include/mediautils/ServiceUtilities.h
+++ b/media/utils/include/mediautils/ServiceUtilities.h
@@ -92,7 +92,7 @@
bool recordingAllowed(const AttributionSourceState &attributionSource,
uint32_t virtualDeviceId,
audio_source_t source);
-bool startRecording(const AttributionSourceState& attributionSource, uint32_t virtualDeviceId,
+int startRecording(const AttributionSourceState& attributionSource, uint32_t virtualDeviceId,
const String16& msg, audio_source_t source);
void finishRecording(const AttributionSourceState& attributionSource, uint32_t virtualDeviceId,
audio_source_t source);
diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
index bb41d00..3932a39 100644
--- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
+++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
@@ -86,6 +86,10 @@
using media::audio::common::Int;
constexpr int kDefaultVirtualDeviceId = 0;
+namespace {
+constexpr auto PERMISSION_HARD_DENIED = permission::PermissionChecker::PERMISSION_HARD_DENIED;
+constexpr auto PERMISSION_GRANTED = permission::PermissionChecker::PERMISSION_GRANTED;
+}
const std::vector<audio_usage_t>& SYSTEM_USAGES = {
AUDIO_USAGE_CALL_ASSISTANT,
@@ -900,13 +904,13 @@
std::stringstream msg;
msg << "Audio recording on session " << client->session;
+ const auto permitted = startRecording(client->attributionSource, client->virtualDeviceId,
+ String16(msg.str().c_str()), client->attributes.source);
// check calling permissions
- if (!(startRecording(client->attributionSource, client->virtualDeviceId,
- String16(msg.str().c_str()), client->attributes.source)
- || client->attributes.source == AUDIO_SOURCE_FM_TUNER
- || client->attributes.source == AUDIO_SOURCE_REMOTE_SUBMIX
- || client->attributes.source == AUDIO_SOURCE_ECHO_REFERENCE)) {
+ if (permitted == PERMISSION_HARD_DENIED && client->attributes.source != AUDIO_SOURCE_FM_TUNER
+ && client->attributes.source != AUDIO_SOURCE_REMOTE_SUBMIX
+ && client->attributes.source != AUDIO_SOURCE_ECHO_REFERENCE) {
ALOGE("%s permission denied: recording not allowed for attribution source %s",
__func__, client->attributionSource.toString().c_str());
return binderStatusFromStatusT(PERMISSION_DENIED);
@@ -926,13 +930,17 @@
return binderStatusFromStatusT(INVALID_OPERATION);
}
- // Force the possibly silenced client to be unsilenced since we just called
- // startRecording (i.e. we have assumed it is unsilenced).
- // At this point in time, the client is inactive, so no calls to appops are sent in
- // setAppState_l.
- // This ensures existing clients have the same behavior as new clients (starting unsilenced).
+ // Force the possibly silenced client to match the state on the appops side
+ // following the call to startRecording (i.e. unsilenced iff call succeeded)
+ // At this point in time, the client is inactive, so no calls to appops are
+ // sent in setAppState_l. This ensures existing clients have the same
+ // behavior as new clients.
// TODO(b/282076713)
- setAppState_l(client, APP_STATE_TOP);
+ if (permitted == PERMISSION_GRANTED) {
+ setAppState_l(client, APP_STATE_TOP);
+ } else {
+ setAppState_l(client, APP_STATE_IDLE);
+ }
client->active = true;
client->startTimeNs = systemTime();
@@ -1018,8 +1026,10 @@
client->active = false;
client->startTimeNs = 0;
updateUidStates_l();
- finishRecording(client->attributionSource, client->virtualDeviceId,
- client->attributes.source);
+ if (!client->silenced) {
+ finishRecording(client->attributionSource, client->virtualDeviceId,
+ client->attributes.source);
+ }
}
return binderStatusFromStatusT(status);
@@ -1048,7 +1058,11 @@
updateUidStates_l();
// finish the recording app op
- finishRecording(client->attributionSource, client->virtualDeviceId, client->attributes.source);
+ if (!client->silenced) {
+ finishRecording(client->attributionSource, client->virtualDeviceId,
+ client->attributes.source);
+ }
+
AutoCallerClear acc;
return binderStatusFromStatusT(mAudioPolicyManager->stopInput(portId));
}
diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp
index 2e0aa3c..40fdd49 100644
--- a/services/audiopolicy/service/AudioPolicyService.cpp
+++ b/services/audiopolicy/service/AudioPolicyService.cpp
@@ -61,6 +61,10 @@
static const String16 sManageAudioPolicyPermission("android.permission.MANAGE_AUDIO_POLICY");
+namespace {
+constexpr auto PERMISSION_GRANTED = permission::PermissionChecker::PERMISSION_GRANTED;
+}
+
// Creates an association between Binder code to name for IAudioPolicyService.
#define IAUDIOPOLICYSERVICE_BINDER_METHOD_MACRO_LIST \
BINDER_METHOD_ENTRY(onNewAudioModulesAvailable) \
@@ -1218,9 +1222,10 @@
} else {
std::stringstream msg;
msg << "Audio recording un-silenced on session " << client->session;
- if (!startRecording(client->attributionSource, client->virtualDeviceId,
- String16(msg.str().c_str()), client->attributes.source)) {
- silenced = true;
+ if (startRecording(client->attributionSource, client->virtualDeviceId,
+ String16(msg.str().c_str()), client->attributes.source)
+ != PERMISSION_GRANTED) {
+ return;
}
}
}