From f1047e87767be1acd2c32f4d36028d1d0014f4c0 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Mon, 16 Apr 2018 19:18:20 -0700 Subject: [PATCH] audioflinger: filter out reserved keys from setParameters() Only allow setParameters() for reserved keys when received from audioserver UID. For instance, keys used to control routing or audio stream configuration are reserved for use by audio policy manager. Also use multiuser_get_app_id() instead of duplicated code to extract application ID from UID. Bug: 77869640 Test: manual audio smoke tests. Change-Id: I88852e8fddf7f705e4a084fc02d9ced1f4b0de92 --- media/libaudioclient/IAudioFlinger.cpp | 4 +- media/libaudioclient/IAudioPolicyService.cpp | 5 +- services/audioflinger/AudioFlinger.cpp | 58 ++++++++++++++++--- services/audioflinger/AudioFlinger.h | 2 + .../service/AudioPolicyService.cpp | 7 ++- 5 files changed, 61 insertions(+), 15 deletions(-) diff --git a/media/libaudioclient/IAudioFlinger.cpp b/media/libaudioclient/IAudioFlinger.cpp index 77cfe4dfe6..8c9d3c1b15 100644 --- a/media/libaudioclient/IAudioFlinger.cpp +++ b/media/libaudioclient/IAudioFlinger.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -904,8 +905,7 @@ status_t BnAudioFlinger::onTransact( case SET_MIC_MUTE: case SET_LOW_RAM_DEVICE: case SYSTEM_READY: { - uid_t multiUserClientUid = IPCThreadState::self()->getCallingUid() % AID_USER_OFFSET; - if (multiUserClientUid >= AID_APP_START) { + if (multiuser_get_app_id(IPCThreadState::self()->getCallingUid()) >= AID_APP_START) { ALOGW("%s: transaction %d received from PID %d unauthorized UID %d", __func__, code, IPCThreadState::self()->getCallingPid(), IPCThreadState::self()->getCallingUid()); diff --git a/media/libaudioclient/IAudioPolicyService.cpp b/media/libaudioclient/IAudioPolicyService.cpp index f87fcc4be3..35f972736d 100644 --- a/media/libaudioclient/IAudioPolicyService.cpp +++ b/media/libaudioclient/IAudioPolicyService.cpp @@ -24,7 +24,7 @@ #include #include - +#include #include #include #include @@ -875,8 +875,7 @@ status_t BnAudioPolicyService::onTransact( case SET_MASTER_MONO: case START_AUDIO_SOURCE: case STOP_AUDIO_SOURCE: { - uid_t multiUserClientUid = IPCThreadState::self()->getCallingUid() % AID_USER_OFFSET; - if (multiUserClientUid >= AID_APP_START) { + if (multiuser_get_app_id(IPCThreadState::self()->getCallingUid()) >= AID_APP_START) { ALOGW("%s: transaction %d received from PID %d unauthorized UID %d", __func__, code, IPCThreadState::self()->getCallingPid(), IPCThreadState::self()->getCallingUid()); diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index b38d37fac2..a3d01c5e8f 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -1180,16 +1181,59 @@ void AudioFlinger::broacastParametersToRecordThreads_l(const String8& keyValuePa } } +// Filter reserved keys from setParameters() before forwarding to audio HAL or acting upon. +// Some keys are used for audio routing and audio path configuration and should be reserved for use +// by audio policy and audio flinger for functional, privacy and security reasons. +void AudioFlinger::filterReservedParameters(String8& keyValuePairs, uid_t callingUid) +{ + static const String8 kReservedParameters[] = { + String8(AudioParameter::keyRouting), + String8(AudioParameter::keySamplingRate), + String8(AudioParameter::keyFormat), + String8(AudioParameter::keyChannels), + String8(AudioParameter::keyFrameCount), + String8(AudioParameter::keyInputSource), + String8(AudioParameter::keyMonoOutput), + String8(AudioParameter::keyStreamConnect), + String8(AudioParameter::keyStreamDisconnect), + String8(AudioParameter::keyStreamSupportedFormats), + String8(AudioParameter::keyStreamSupportedChannels), + String8(AudioParameter::keyStreamSupportedSamplingRates), + }; + + // multiuser friendly app ID check for requests coming from audioserver + if (multiuser_get_app_id(callingUid) == AID_AUDIOSERVER) { + return; + } + + AudioParameter param = AudioParameter(keyValuePairs); + String8 value; + for (auto& key : kReservedParameters) { + if (param.get(key, value) == NO_ERROR) { + ALOGW("%s: filtering key %s value %s from uid %d", + __func__, key.string(), value.string(), callingUid); + param.remove(key); + } + } + keyValuePairs = param.toString(); +} + status_t AudioFlinger::setParameters(audio_io_handle_t ioHandle, const String8& keyValuePairs) { - ALOGV("setParameters(): io %d, keyvalue %s, calling pid %d", - ioHandle, keyValuePairs.string(), IPCThreadState::self()->getCallingPid()); + ALOGV("setParameters(): io %d, keyvalue %s, calling pid %d calling uid %d", + ioHandle, keyValuePairs.string(), + IPCThreadState::self()->getCallingPid(), IPCThreadState::self()->getCallingUid()); // check calling permissions if (!settingsAllowed()) { return PERMISSION_DENIED; } + String8 filteredKeyValuePairs = keyValuePairs; + filterReservedParameters(filteredKeyValuePairs, IPCThreadState::self()->getCallingUid()); + + ALOGV("%s: filtered keyvalue %s", __func__, filteredKeyValuePairs.string()); + // AUDIO_IO_HANDLE_NONE means the parameters are global to the audio hardware interface if (ioHandle == AUDIO_IO_HANDLE_NONE) { Mutex::Autolock _l(mLock); @@ -1200,7 +1244,7 @@ status_t AudioFlinger::setParameters(audio_io_handle_t ioHandle, const String8& mHardwareStatus = AUDIO_HW_SET_PARAMETER; for (size_t i = 0; i < mAudioHwDevs.size(); i++) { sp dev = mAudioHwDevs.valueAt(i)->hwDevice(); - status_t result = dev->setParameters(keyValuePairs); + status_t result = dev->setParameters(filteredKeyValuePairs); // return success if at least one audio device accepts the parameters as not all // HALs are requested to support all parameters. If no audio device supports the // requested parameters, the last error is reported. @@ -1211,7 +1255,7 @@ status_t AudioFlinger::setParameters(audio_io_handle_t ioHandle, const String8& mHardwareStatus = AUDIO_HW_IDLE; } // disable AEC and NS if the device is a BT SCO headset supporting those pre processings - AudioParameter param = AudioParameter(keyValuePairs); + AudioParameter param = AudioParameter(filteredKeyValuePairs); String8 value; if (param.get(String8(AudioParameter::keyBtNrec), value) == NO_ERROR) { bool btNrecIsOff = (value == AudioParameter::valueOff); @@ -1244,16 +1288,16 @@ status_t AudioFlinger::setParameters(audio_io_handle_t ioHandle, const String8& } } else if (thread == primaryPlaybackThread_l()) { // indicate output device change to all input threads for pre processing - AudioParameter param = AudioParameter(keyValuePairs); + AudioParameter param = AudioParameter(filteredKeyValuePairs); int value; if ((param.getInt(String8(AudioParameter::keyRouting), value) == NO_ERROR) && (value != 0)) { - broacastParametersToRecordThreads_l(keyValuePairs); + broacastParametersToRecordThreads_l(filteredKeyValuePairs); } } } if (thread != 0) { - return thread->setParameters(keyValuePairs); + return thread->setParameters(filteredKeyValuePairs); } return BAD_VALUE; } diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 33028688a7..963a87da58 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -798,6 +798,8 @@ private: status_t checkStreamType(audio_stream_type_t stream) const; + void filterReservedParameters(String8& keyValuePairs, uid_t callingUid); + #ifdef TEE_SINK // all record threads serially share a common tee sink, which is re-created on format change sp mRecordTeeSink; diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp index 13bf60589a..f3cddc33da 100644 --- a/services/audiopolicy/service/AudioPolicyService.cpp +++ b/services/audiopolicy/service/AudioPolicyService.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -274,7 +275,7 @@ void AudioPolicyService::NotificationClient::onAudioPatchListUpdate() void AudioPolicyService::NotificationClient::onDynamicPolicyMixStateUpdate( const String8& regId, int32_t state) { - if (mAudioPolicyServiceClient != 0 && (mUid % AID_USER_OFFSET) < AID_APP_START) { + if (mAudioPolicyServiceClient != 0 && multiuser_get_app_id(mUid) < AID_APP_START) { mAudioPolicyServiceClient->onDynamicPolicyMixStateUpdate(regId, state); } } @@ -284,7 +285,7 @@ void AudioPolicyService::NotificationClient::onRecordingConfigurationUpdate( const audio_config_base_t *clientConfig, const audio_config_base_t *deviceConfig, audio_patch_handle_t patchHandle) { - if (mAudioPolicyServiceClient != 0 && (mUid % AID_USER_OFFSET) < AID_APP_START) { + if (mAudioPolicyServiceClient != 0 && multiuser_get_app_id(mUid) < AID_APP_START) { mAudioPolicyServiceClient->onRecordingConfigurationUpdate(event, clientInfo, clientConfig, deviceConfig, patchHandle); } @@ -577,7 +578,7 @@ void AudioPolicyService::UidPolicy::onUidIdle(uid_t uid, __unused bool disabled) } bool AudioPolicyService::UidPolicy::isServiceUid(uid_t uid) const { - return uid % AID_USER_OFFSET < AID_APP_START; + return multiuser_get_app_id(uid) < AID_APP_START; } void AudioPolicyService::UidPolicy::notifyService(uid_t uid, bool active) {