From 3ba0bbe12904d0dfa2245fa3abf2b92034b15db3 Mon Sep 17 00:00:00 2001 From: "mspector@google.com" Date: Mon, 8 Feb 2016 10:56:13 -0800 Subject: IOMX.cpp uninitialized pointer in BnOMX::onTransact This can lead to local code execution in media server. Fix initializes the pointer and checks the error conditions before returning Bug: 26403627 Change-Id: I7fa90682060148448dba01d6acbe3471d1ddb500 --- media/libmedia/IOMX.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 5423c2a..8f55eb9 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -446,7 +446,7 @@ public: remote()->transact(CONFIGURE_VIDEO_TUNNEL_MODE, data, &reply); status_t err = reply.readInt32(); - if (sidebandHandle) { + if (err == OK && sidebandHandle) { *sidebandHandle = (native_handle_t *)reply.readNativeHandle(); } return err; @@ -948,11 +948,13 @@ status_t BnOMX::onTransact( OMX_BOOL tunneled = (OMX_BOOL)data.readInt32(); OMX_U32 audio_hw_sync = data.readInt32(); - native_handle_t *sideband_handle; + native_handle_t *sideband_handle = NULL; status_t err = configureVideoTunnelMode( node, port_index, tunneled, audio_hw_sync, &sideband_handle); reply->writeInt32(err); - reply->writeNativeHandle(sideband_handle); + if(err == OK){ + reply->writeNativeHandle(sideband_handle); + } return NO_ERROR; } -- cgit v1.1 From 638e5bad20b9d05f0d10b5e216cf5860a18ffc70 Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Mon, 8 Feb 2016 17:52:25 -0800 Subject: Fix info leak vulnerability of IDrm bug: 26323455 Change-Id: I25bb30d3666ab38d5150496375ed2f55ecb23ba8 --- media/libmedia/IDrm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IDrm.cpp b/media/libmedia/IDrm.cpp index b1ad0c5..7c709cd 100644 --- a/media/libmedia/IDrm.cpp +++ b/media/libmedia/IDrm.cpp @@ -658,7 +658,7 @@ status_t BnDrm::onTransact( Vector request; String8 defaultUrl; - DrmPlugin::KeyRequestType keyRequestType; + DrmPlugin::KeyRequestType keyRequestType = DrmPlugin::kKeyRequestType_Unknown; status_t result = getKeyRequest(sessionId, initData, mimeType, keyType, optionalParameters, request, defaultUrl, -- cgit v1.1 From 57bf4973b57dc62e7171c1cb2e0854f9e21fd3e4 Mon Sep 17 00:00:00 2001 From: "mspector@google.com" Date: Mon, 8 Feb 2016 17:07:06 -0800 Subject: 3 uninitialized variables in IOMX.cpp Uninitialized MetadataBufferType pointer in case: SET_INPUT_SURFACE STORE_META_DATA_IN_BUFFERS CREATE_INPUT_SURFACE Fix: initialize them to kMetadataBufferTypeInvalid Bug: 26324358 Change-Id: Ib3bcac9bedc98bc65efa39f67cdbffbd90b374c9 --- media/libmedia/IOMX.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 8f55eb9..07f3697 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -844,9 +844,13 @@ status_t BnOMX::onTransact( OMX_U32 port_index = data.readInt32(); sp bufferProducer; - MetadataBufferType type; + MetadataBufferType type = kMetadataBufferTypeInvalid; status_t err = createInputSurface(node, port_index, &bufferProducer, &type); + if ((err != OK) && (type == kMetadataBufferTypeInvalid)) { + android_errorWriteLog(0x534e4554, "26324358"); + } + reply->writeInt32(type); reply->writeInt32(err); @@ -886,9 +890,13 @@ status_t BnOMX::onTransact( sp bufferConsumer = interface_cast(data.readStrongBinder()); - MetadataBufferType type; + MetadataBufferType type = kMetadataBufferTypeInvalid; status_t err = setInputSurface(node, port_index, bufferConsumer, &type); + if ((err != OK) && (type == kMetadataBufferTypeInvalid)) { + android_errorWriteLog(0x534e4554, "26324358"); + } + reply->writeInt32(type); reply->writeInt32(err); return NO_ERROR; @@ -914,8 +922,13 @@ status_t BnOMX::onTransact( OMX_U32 port_index = data.readInt32(); OMX_BOOL enable = (OMX_BOOL)data.readInt32(); - MetadataBufferType type; + MetadataBufferType type = kMetadataBufferTypeInvalid; status_t err = storeMetaDataInBuffers(node, port_index, enable, &type); + + if ((err != OK) && (type == kMetadataBufferTypeInvalid)) { + android_errorWriteLog(0x534e4554, "26324358"); + } + reply->writeInt32(type); reply->writeInt32(err); -- cgit v1.1 From 1f76ce4e2c13d0347523e8c9a27077c820715f08 Mon Sep 17 00:00:00 2001 From: "mspector@google.com" Date: Fri, 19 Feb 2016 12:10:48 -0800 Subject: Fixing safteynet logging bug introduced in ag/862848 We moved the safteynet logging into storeMetaDataInBuffers_l Bug: 26324358 Change-Id: I2171742b53192ebb71d546bcb8970bb3c68f366f --- media/libmedia/IOMX.cpp | 4 ---- 1 file changed, 4 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 07f3697..97b7ff0 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -925,10 +925,6 @@ status_t BnOMX::onTransact( MetadataBufferType type = kMetadataBufferTypeInvalid; status_t err = storeMetaDataInBuffers(node, port_index, enable, &type); - if ((err != OK) && (type == kMetadataBufferTypeInvalid)) { - android_errorWriteLog(0x534e4554, "26324358"); - } - reply->writeInt32(type); reply->writeInt32(err); -- cgit v1.1 From 3a90a02bb6c4e8352112dbb9f2316935e4dd7315 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 22 Feb 2016 13:05:15 -0800 Subject: Clear allocation to avoid info leak Bug: 26914474 Change-Id: Ie1a86e86d78058d041149fe599a4996e7f8185cf --- media/libmedia/IOMX.cpp | 59 +++++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 27 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 97b7ff0..865d575 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -692,34 +692,39 @@ status_t BnOMX::onTransact( size_t size = data.readInt64(); - void *params = malloc(size); - data.read(params, size); - - status_t err; - switch (code) { - case GET_PARAMETER: - err = getParameter(node, index, params, size); - break; - case SET_PARAMETER: - err = setParameter(node, index, params, size); - break; - case GET_CONFIG: - err = getConfig(node, index, params, size); - break; - case SET_CONFIG: - err = setConfig(node, index, params, size); - break; - case SET_INTERNAL_OPTION: - { - InternalOptionType type = - (InternalOptionType)data.readInt32(); - - err = setInternalOption(node, index, type, params, size); - break; + status_t err = NO_MEMORY; + void *params = calloc(size, 1); + if (params) { + err = data.read(params, size); + if (err != OK) { + android_errorWriteLog(0x534e4554, "26914474"); + } else { + switch (code) { + case GET_PARAMETER: + err = getParameter(node, index, params, size); + break; + case SET_PARAMETER: + err = setParameter(node, index, params, size); + break; + case GET_CONFIG: + err = getConfig(node, index, params, size); + break; + case SET_CONFIG: + err = setConfig(node, index, params, size); + break; + case SET_INTERNAL_OPTION: + { + InternalOptionType type = + (InternalOptionType)data.readInt32(); + + err = setInternalOption(node, index, type, params, size); + break; + } + + default: + TRESPASS(); + } } - - default: - TRESPASS(); } reply->writeInt32(err); -- cgit v1.1 From de2430fd7855b7941250e1c3744756ec9e699bd1 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Thu, 18 Feb 2016 08:25:47 -0800 Subject: Get service by value instead of reference to prevent a cleared service binder from being used. Bug: 26040840 Change-Id: Ifb5483c55b172d3553deb80dbe27f2204b86ecdb --- media/libmedia/mediametadataretriever.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/mediametadataretriever.cpp b/media/libmedia/mediametadataretriever.cpp index 9a76f58..08a9e6a 100644 --- a/media/libmedia/mediametadataretriever.cpp +++ b/media/libmedia/mediametadataretriever.cpp @@ -35,7 +35,7 @@ Mutex MediaMetadataRetriever::sServiceLock; sp MediaMetadataRetriever::sService; sp MediaMetadataRetriever::sDeathNotifier; -const sp& MediaMetadataRetriever::getService() +const sp MediaMetadataRetriever::getService() { Mutex::Autolock lock(sServiceLock); if (sService == 0) { @@ -62,7 +62,7 @@ const sp& MediaMetadataRetriever::getService() MediaMetadataRetriever::MediaMetadataRetriever() { ALOGV("constructor"); - const sp& service(getService()); + const sp service(getService()); if (service == 0) { ALOGE("failed to obtain MediaMetadataRetrieverService"); return; -- cgit v1.1 From 52fbd46e2b7bc185628e94178cc3ec8e2a340b40 Mon Sep 17 00:00:00 2001 From: "mspector@google.com" Date: Mon, 8 Feb 2016 10:56:13 -0800 Subject: IOMX.cpp uninitialized pointer in BnOMX::onTransact This can lead to local code execution in media server. Fix initializes the pointer and checks the error conditions before returning Bug: 26403627 Change-Id: I7fa90682060148448dba01d6acbe3471d1ddb500 --- media/libmedia/IOMX.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 5423c2a..8f55eb9 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -446,7 +446,7 @@ public: remote()->transact(CONFIGURE_VIDEO_TUNNEL_MODE, data, &reply); status_t err = reply.readInt32(); - if (sidebandHandle) { + if (err == OK && sidebandHandle) { *sidebandHandle = (native_handle_t *)reply.readNativeHandle(); } return err; @@ -948,11 +948,13 @@ status_t BnOMX::onTransact( OMX_BOOL tunneled = (OMX_BOOL)data.readInt32(); OMX_U32 audio_hw_sync = data.readInt32(); - native_handle_t *sideband_handle; + native_handle_t *sideband_handle = NULL; status_t err = configureVideoTunnelMode( node, port_index, tunneled, audio_hw_sync, &sideband_handle); reply->writeInt32(err); - reply->writeNativeHandle(sideband_handle); + if(err == OK){ + reply->writeNativeHandle(sideband_handle); + } return NO_ERROR; } -- cgit v1.1 From 7227fe0eb793c3e9e313df7454862d360c9bd360 Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Mon, 8 Feb 2016 17:52:25 -0800 Subject: Fix info leak vulnerability of IDrm bug: 26323455 Change-Id: I25bb30d3666ab38d5150496375ed2f55ecb23ba8 --- media/libmedia/IDrm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IDrm.cpp b/media/libmedia/IDrm.cpp index b1ad0c5..7c709cd 100644 --- a/media/libmedia/IDrm.cpp +++ b/media/libmedia/IDrm.cpp @@ -658,7 +658,7 @@ status_t BnDrm::onTransact( Vector request; String8 defaultUrl; - DrmPlugin::KeyRequestType keyRequestType; + DrmPlugin::KeyRequestType keyRequestType = DrmPlugin::kKeyRequestType_Unknown; status_t result = getKeyRequest(sessionId, initData, mimeType, keyType, optionalParameters, request, defaultUrl, -- cgit v1.1 From 1a5ab71d6a43a53535f37648b09d22b741265bb2 Mon Sep 17 00:00:00 2001 From: "mspector@google.com" Date: Mon, 8 Feb 2016 17:07:06 -0800 Subject: 3 uninitialized variables in IOMX.cpp Uninitialized MetadataBufferType pointer in case: SET_INPUT_SURFACE STORE_META_DATA_IN_BUFFERS CREATE_INPUT_SURFACE Fix: initialize them to kMetadataBufferTypeInvalid Bug: 26324358 Change-Id: Ib3bcac9bedc98bc65efa39f67cdbffbd90b374c9 --- media/libmedia/IOMX.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 8f55eb9..07f3697 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -844,9 +844,13 @@ status_t BnOMX::onTransact( OMX_U32 port_index = data.readInt32(); sp bufferProducer; - MetadataBufferType type; + MetadataBufferType type = kMetadataBufferTypeInvalid; status_t err = createInputSurface(node, port_index, &bufferProducer, &type); + if ((err != OK) && (type == kMetadataBufferTypeInvalid)) { + android_errorWriteLog(0x534e4554, "26324358"); + } + reply->writeInt32(type); reply->writeInt32(err); @@ -886,9 +890,13 @@ status_t BnOMX::onTransact( sp bufferConsumer = interface_cast(data.readStrongBinder()); - MetadataBufferType type; + MetadataBufferType type = kMetadataBufferTypeInvalid; status_t err = setInputSurface(node, port_index, bufferConsumer, &type); + if ((err != OK) && (type == kMetadataBufferTypeInvalid)) { + android_errorWriteLog(0x534e4554, "26324358"); + } + reply->writeInt32(type); reply->writeInt32(err); return NO_ERROR; @@ -914,8 +922,13 @@ status_t BnOMX::onTransact( OMX_U32 port_index = data.readInt32(); OMX_BOOL enable = (OMX_BOOL)data.readInt32(); - MetadataBufferType type; + MetadataBufferType type = kMetadataBufferTypeInvalid; status_t err = storeMetaDataInBuffers(node, port_index, enable, &type); + + if ((err != OK) && (type == kMetadataBufferTypeInvalid)) { + android_errorWriteLog(0x534e4554, "26324358"); + } + reply->writeInt32(type); reply->writeInt32(err); -- cgit v1.1 From a685aea781026471ad82f5729198c60bcb4f8fe9 Mon Sep 17 00:00:00 2001 From: "mspector@google.com" Date: Fri, 19 Feb 2016 12:10:48 -0800 Subject: Fixing safteynet logging bug introduced in ag/862848 We moved the safteynet logging into storeMetaDataInBuffers_l Bug: 26324358 Change-Id: I2171742b53192ebb71d546bcb8970bb3c68f366f --- media/libmedia/IOMX.cpp | 4 ---- 1 file changed, 4 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 07f3697..97b7ff0 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -925,10 +925,6 @@ status_t BnOMX::onTransact( MetadataBufferType type = kMetadataBufferTypeInvalid; status_t err = storeMetaDataInBuffers(node, port_index, enable, &type); - if ((err != OK) && (type == kMetadataBufferTypeInvalid)) { - android_errorWriteLog(0x534e4554, "26324358"); - } - reply->writeInt32(type); reply->writeInt32(err); -- cgit v1.1 From d3e5eca42feaa337cd3aa36b7eaa2b7aadbbab15 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 22 Feb 2016 13:05:15 -0800 Subject: Clear allocation to avoid info leak Bug: 26914474 Change-Id: Ie1a86e86d78058d041149fe599a4996e7f8185cf --- media/libmedia/IOMX.cpp | 59 +++++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 27 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 97b7ff0..865d575 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -692,34 +692,39 @@ status_t BnOMX::onTransact( size_t size = data.readInt64(); - void *params = malloc(size); - data.read(params, size); - - status_t err; - switch (code) { - case GET_PARAMETER: - err = getParameter(node, index, params, size); - break; - case SET_PARAMETER: - err = setParameter(node, index, params, size); - break; - case GET_CONFIG: - err = getConfig(node, index, params, size); - break; - case SET_CONFIG: - err = setConfig(node, index, params, size); - break; - case SET_INTERNAL_OPTION: - { - InternalOptionType type = - (InternalOptionType)data.readInt32(); - - err = setInternalOption(node, index, type, params, size); - break; + status_t err = NO_MEMORY; + void *params = calloc(size, 1); + if (params) { + err = data.read(params, size); + if (err != OK) { + android_errorWriteLog(0x534e4554, "26914474"); + } else { + switch (code) { + case GET_PARAMETER: + err = getParameter(node, index, params, size); + break; + case SET_PARAMETER: + err = setParameter(node, index, params, size); + break; + case GET_CONFIG: + err = getConfig(node, index, params, size); + break; + case SET_CONFIG: + err = setConfig(node, index, params, size); + break; + case SET_INTERNAL_OPTION: + { + InternalOptionType type = + (InternalOptionType)data.readInt32(); + + err = setInternalOption(node, index, type, params, size); + break; + } + + default: + TRESPASS(); + } } - - default: - TRESPASS(); } reply->writeInt32(err); -- cgit v1.1 From 5db73079cbd9bbe5bf2c6fb7d029537a085170ca Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Thu, 18 Feb 2016 08:25:47 -0800 Subject: Get service by value instead of reference to prevent a cleared service binder from being used. Bug: 26040840 Change-Id: Ifb5483c55b172d3553deb80dbe27f2204b86ecdb --- media/libmedia/mediametadataretriever.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/mediametadataretriever.cpp b/media/libmedia/mediametadataretriever.cpp index 9a76f58..08a9e6a 100644 --- a/media/libmedia/mediametadataretriever.cpp +++ b/media/libmedia/mediametadataretriever.cpp @@ -35,7 +35,7 @@ Mutex MediaMetadataRetriever::sServiceLock; sp MediaMetadataRetriever::sService; sp MediaMetadataRetriever::sDeathNotifier; -const sp& MediaMetadataRetriever::getService() +const sp MediaMetadataRetriever::getService() { Mutex::Autolock lock(sServiceLock); if (sService == 0) { @@ -62,7 +62,7 @@ const sp& MediaMetadataRetriever::getService() MediaMetadataRetriever::MediaMetadataRetriever() { ALOGV("constructor"); - const sp& service(getService()); + const sp service(getService()); if (service == 0) { ALOGE("failed to obtain MediaMetadataRetrieverService"); return; -- cgit v1.1 From a4123803d0a0e9e0c69faa4207d357cc74a65d58 Mon Sep 17 00:00:00 2001 From: Steve Kondik Date: Wed, 6 Apr 2016 18:35:02 -0700 Subject: audiopolicy: Be a little smarter with auto-attach * The edge cases, ZOMG! * Instead of relying on effects to be automatically attached, let's just always notify userspace and send a bit more information. This lets the application decide if effects should be attached rather than relying on a hard-coded configuration file. * Perform the setup in getOutputForAttr, but do it on the command thread so we don't block the client. OPO-593 Change-Id: I3900b349f2e895d51fa3a3dcc2de0c4bdf6dbc08 --- media/libmedia/AudioSystem.cpp | 6 ++++-- media/libmedia/IAudioPolicyServiceClient.cpp | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/AudioSystem.cpp b/media/libmedia/AudioSystem.cpp index 10ec495..5bd8747 100644 --- a/media/libmedia/AudioSystem.cpp +++ b/media/libmedia/AudioSystem.cpp @@ -1239,7 +1239,9 @@ void AudioSystem::AudioPolicyServiceClient::onDynamicPolicyMixStateUpdate( } void AudioSystem::AudioPolicyServiceClient::onOutputSessionEffectsUpdate( - audio_stream_type_t stream, audio_unique_id_t sessionId, bool added) + audio_stream_type_t stream, audio_session_t sessionId, + audio_output_flags_t flags, audio_channel_mask_t channelMask, + uid_t uid, bool added) { ALOGV("AudioPolicyServiceClient::onOutputSessionEffectsUpdate(%d, %d, %d)", stream, sessionId, added); effect_session_callback cb = NULL; @@ -1249,7 +1251,7 @@ void AudioSystem::AudioPolicyServiceClient::onOutputSessionEffectsUpdate( } if (cb != NULL) { - cb(AUDIO_OUTPUT_SESSION_EFFECTS_UPDATE, stream, sessionId, added); + cb(AUDIO_OUTPUT_SESSION_EFFECTS_UPDATE, stream, sessionId, flags, channelMask, uid, added); } } diff --git a/media/libmedia/IAudioPolicyServiceClient.cpp b/media/libmedia/IAudioPolicyServiceClient.cpp index d6207ce..ecf0d1f 100644 --- a/media/libmedia/IAudioPolicyServiceClient.cpp +++ b/media/libmedia/IAudioPolicyServiceClient.cpp @@ -66,12 +66,18 @@ public: } void onOutputSessionEffectsUpdate(audio_stream_type_t stream, - audio_unique_id_t sessionId, bool added) + audio_session_t sessionId, + audio_output_flags_t flags, + audio_channel_mask_t channelMask, + uid_t uid, bool added) { Parcel data, reply; data.writeInterfaceToken(IAudioPolicyServiceClient::getInterfaceDescriptor()); data.writeInt32(stream); data.writeInt32(sessionId); + data.writeInt32(flags); + data.writeInt32(channelMask); + data.writeInt32(uid); data.writeInt32(added ? 1 : 0); remote()->transact(OUTPUT_SESSION_EFFECTS_UPDATE, data, &reply, IBinder::FLAG_ONEWAY); } @@ -105,9 +111,12 @@ status_t BnAudioPolicyServiceClient::onTransact( case OUTPUT_SESSION_EFFECTS_UPDATE: { CHECK_INTERFACE(IAudioPolicyServiceClient, data, reply); audio_stream_type_t stream = static_cast(data.readInt32()); - audio_unique_id_t sessionId = static_cast(data.readInt32()); + audio_session_t sessionId = static_cast(data.readInt32()); + audio_output_flags_t flags = static_cast(data.readInt32()); + audio_channel_mask_t channelMask = static_cast(data.readInt32()); + uid_t uid = static_cast(data.readInt32()); bool added = data.readInt32() > 0; - onOutputSessionEffectsUpdate(stream, sessionId, added); + onOutputSessionEffectsUpdate(stream, sessionId, flags, channelMask, uid, added); return NO_ERROR; } default: -- cgit v1.1 From 804eeffb2da5f954177a297dd54966eb865424a5 Mon Sep 17 00:00:00 2001 From: William Clark Date: Thu, 22 Oct 2015 18:21:08 -0700 Subject: Update Instrumentation code Add support for enhancements in QSSP instrumentation code. Change-Id: I00aa0b2bee5044d91eeeddaa43d3d28518a5ba93 --- media/libmedia/AudioRecord.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp index 40f6c44..fc35a4c 100644 --- a/media/libmedia/AudioRecord.cpp +++ b/media/libmedia/AudioRecord.cpp @@ -299,7 +299,7 @@ status_t AudioRecord::set( status_t AudioRecord::start(AudioSystem::sync_event_t event, int triggerSession) { ALOGV("start, sync event %d trigger session %d", event, triggerSession); - SEEMPLOG_RECORD(89,""); + SEEMPLOG_RECORD(71,""); AutoMutex lock(mLock); if (mActive) { @@ -347,7 +347,6 @@ status_t AudioRecord::start(AudioSystem::sync_event_t event, int triggerSession) void AudioRecord::stop() { - SEEMPLOG_RECORD(90,""); AutoMutex lock(mLock); if (!mActive) { return; -- cgit v1.1 From 5153cdc35e816a2224da1dac0e4d316bf847473d Mon Sep 17 00:00:00 2001 From: Zhou Song Date: Tue, 22 Mar 2016 16:13:09 +0800 Subject: libmedia: Initialize mState/mActive value to avoid possible crash If track creation fails on server side, mState/mActive is left unitialized. After this a crash can happen when track stop() is called because of a reference to a NULL object. Initialize mState/mActive to specific value in initialized list of track constructor. Change-Id: If8c3611e3229c0c1b14b81285e07b9357fec7658 CRs-Fixed: 992608 --- media/libmedia/AudioRecord.cpp | 6 +++--- media/libmedia/AudioTrack.cpp | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp index fc35a4c..40cad59 100644 --- a/media/libmedia/AudioRecord.cpp +++ b/media/libmedia/AudioRecord.cpp @@ -67,7 +67,7 @@ status_t AudioRecord::getMinFrameCount( // --------------------------------------------------------------------------- AudioRecord::AudioRecord(const String16 &opPackageName) - : mStatus(NO_INIT), mOpPackageName(opPackageName), mSessionId(AUDIO_SESSION_ALLOCATE), + : mActive(false), mStatus(NO_INIT), mOpPackageName(opPackageName), mSessionId(AUDIO_SESSION_ALLOCATE), mPreviousPriority(ANDROID_PRIORITY_NORMAL), mPreviousSchedulingGroup(SP_DEFAULT), mSelectedDeviceId(AUDIO_PORT_HANDLE_NONE) { @@ -89,7 +89,8 @@ AudioRecord::AudioRecord( int uid, pid_t pid, const audio_attributes_t* pAttributes) - : mStatus(NO_INIT), + : mActive(false), + mStatus(NO_INIT), mOpPackageName(opPackageName), mSessionId(AUDIO_SESSION_ALLOCATE), mPreviousPriority(ANDROID_PRIORITY_NORMAL), @@ -273,7 +274,6 @@ status_t AudioRecord::set( } mStatus = NO_ERROR; - mActive = false; mUserData = user; // TODO: add audio hardware input latency here if (mTransfer == TRANSFER_CALLBACK || diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp index d7256f8..ae016ef 100644 --- a/media/libmedia/AudioTrack.cpp +++ b/media/libmedia/AudioTrack.cpp @@ -165,6 +165,7 @@ status_t AudioTrack::getMinFrameCount( AudioTrack::AudioTrack() : mStatus(NO_INIT), + mState(STATE_STOPPED), mIsTimed(false), mPreviousPriority(ANDROID_PRIORITY_NORMAL), mPreviousSchedulingGroup(SP_DEFAULT), @@ -196,6 +197,7 @@ AudioTrack::AudioTrack( const audio_attributes_t* pAttributes, bool doNotReconnect) : mStatus(NO_INIT), + mState(STATE_STOPPED), mIsTimed(false), mPreviousPriority(ANDROID_PRIORITY_NORMAL), mPreviousSchedulingGroup(SP_DEFAULT), @@ -227,6 +229,7 @@ AudioTrack::AudioTrack( const audio_attributes_t* pAttributes, bool doNotReconnect) : mStatus(NO_INIT), + mState(STATE_STOPPED), mIsTimed(false), mPreviousPriority(ANDROID_PRIORITY_NORMAL), mPreviousSchedulingGroup(SP_DEFAULT), @@ -478,7 +481,6 @@ status_t AudioTrack::set( } mStatus = NO_ERROR; - mState = STATE_STOPPED; mUserData = user; mLoopCount = 0; mLoopStart = 0; -- cgit v1.1 From 295c883fe3105b19bcd0f9e07d54c6b589fc5bff Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 29 Feb 2016 12:47:20 -0800 Subject: DO NOT MERGE Verify OMX buffer sizes prior to access Bug: 27207275 Change-Id: I4412825d1ee233d993af0a67708bea54304ff62d --- media/libmedia/IOMX.cpp | 96 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 66 insertions(+), 30 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 865d575..84925f1 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -18,6 +18,8 @@ #define LOG_TAG "IOMX" #include +#include + #include #include #include @@ -692,38 +694,70 @@ status_t BnOMX::onTransact( size_t size = data.readInt64(); - status_t err = NO_MEMORY; - void *params = calloc(size, 1); - if (params) { - err = data.read(params, size); - if (err != OK) { - android_errorWriteLog(0x534e4554, "26914474"); + status_t err = NOT_ENOUGH_DATA; + void *params = NULL; + size_t pageSize = 0; + size_t allocSize = 0; + if (code != SET_INTERNAL_OPTION && size < 8) { + // we expect the structure to contain at least the size and + // version, 8 bytes total + ALOGE("b/27207275 (%zu)", size); + android_errorWriteLog(0x534e4554, "27207275"); + } else { + err = NO_MEMORY; + pageSize = (size_t) sysconf(_SC_PAGE_SIZE); + if (size > SIZE_MAX - (pageSize * 2)) { + ALOGE("requested param size too big"); } else { - switch (code) { - case GET_PARAMETER: - err = getParameter(node, index, params, size); - break; - case SET_PARAMETER: - err = setParameter(node, index, params, size); - break; - case GET_CONFIG: - err = getConfig(node, index, params, size); - break; - case SET_CONFIG: - err = setConfig(node, index, params, size); - break; - case SET_INTERNAL_OPTION: - { - InternalOptionType type = - (InternalOptionType)data.readInt32(); - - err = setInternalOption(node, index, type, params, size); - break; + allocSize = (size + pageSize * 2) & ~(pageSize - 1); + params = mmap(NULL, allocSize, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1 /* fd */, 0 /* offset */); + } + if (params != MAP_FAILED) { + err = data.read(params, size); + if (err != OK) { + android_errorWriteLog(0x534e4554, "26914474"); + } else { + err = NOT_ENOUGH_DATA; + OMX_U32 declaredSize = *(OMX_U32*)params; + if (code != SET_INTERNAL_OPTION && declaredSize > size) { + // the buffer says it's bigger than it actually is + ALOGE("b/27207275 (%u/%zu)", declaredSize, size); + android_errorWriteLog(0x534e4554, "27207275"); + } else { + // mark the last page as inaccessible, to avoid exploitation + // of codecs that access past the end of the allocation because + // they didn't check the size + mprotect((char*)params + allocSize - pageSize, pageSize, PROT_NONE); + switch (code) { + case GET_PARAMETER: + err = getParameter(node, index, params, size); + break; + case SET_PARAMETER: + err = setParameter(node, index, params, size); + break; + case GET_CONFIG: + err = getConfig(node, index, params, size); + break; + case SET_CONFIG: + err = setConfig(node, index, params, size); + break; + case SET_INTERNAL_OPTION: + { + InternalOptionType type = + (InternalOptionType)data.readInt32(); + + err = setInternalOption(node, index, type, params, size); + break; + } + + default: + TRESPASS(); + } } - - default: - TRESPASS(); } + } else { + ALOGE("couldn't map: %s", strerror(errno)); } } @@ -733,7 +767,9 @@ status_t BnOMX::onTransact( reply->write(params, size); } - free(params); + if (params) { + munmap(params, allocSize); + } params = NULL; return NO_ERROR; -- cgit v1.1 From 0bb5ced60304da7f61478ffd359e7ba65d72f181 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Thu, 10 Mar 2016 15:02:13 -0800 Subject: Fix size check for OMX_IndexParamConsumerUsageBits since it doesn't follow the OMX convention. And remove support for the kClientNeedsFrameBuffer flag. Bug: 27207275 Change-Id: Ia2c119e2456ebf9e2f4e1de5104ef9032a212255 --- media/libmedia/IOMX.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 84925f1..9be9b41 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -24,6 +24,7 @@ #include #include #include +#include namespace android { @@ -698,7 +699,8 @@ status_t BnOMX::onTransact( void *params = NULL; size_t pageSize = 0; size_t allocSize = 0; - if (code != SET_INTERNAL_OPTION && size < 8) { + if ((index == (OMX_INDEXTYPE) OMX_IndexParamConsumerUsageBits && size < 4) || + (code != SET_INTERNAL_OPTION && size < 8)) { // we expect the structure to contain at least the size and // version, 8 bytes total ALOGE("b/27207275 (%zu)", size); @@ -720,7 +722,9 @@ status_t BnOMX::onTransact( } else { err = NOT_ENOUGH_DATA; OMX_U32 declaredSize = *(OMX_U32*)params; - if (code != SET_INTERNAL_OPTION && declaredSize > size) { + if (code != SET_INTERNAL_OPTION && + index != (OMX_INDEXTYPE) OMX_IndexParamConsumerUsageBits && + declaredSize > size) { // the buffer says it's bigger than it actually is ALOGE("b/27207275 (%u/%zu)", declaredSize, size); android_errorWriteLog(0x534e4554, "27207275"); -- cgit v1.1 From db829699d3293f254a7387894303451a91278986 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 21 Mar 2016 11:31:53 -0700 Subject: Fix OMX_IndexParamConsumerUsageBits size check Bug: 27207275 Change-Id: I9a7c9fb22a0e84a490ff09c151bd2f88141fdbc0 --- media/libmedia/IOMX.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 9be9b41..b082fe4 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -699,11 +699,12 @@ status_t BnOMX::onTransact( void *params = NULL; size_t pageSize = 0; size_t allocSize = 0; - if ((index == (OMX_INDEXTYPE) OMX_IndexParamConsumerUsageBits && size < 4) || - (code != SET_INTERNAL_OPTION && size < 8)) { + bool isUsageBits = (index == (OMX_INDEXTYPE) OMX_IndexParamConsumerUsageBits); + if ((isUsageBits && size < 4) || + (!isUsageBits && code != SET_INTERNAL_OPTION && size < 8)) { // we expect the structure to contain at least the size and // version, 8 bytes total - ALOGE("b/27207275 (%zu)", size); + ALOGE("b/27207275 (%zu) (%d/%d)", size, int(index), int(code)); android_errorWriteLog(0x534e4554, "27207275"); } else { err = NO_MEMORY; -- cgit v1.1 From 244deea89aaf3c5dfa8bd369a845276ae501cb5a Mon Sep 17 00:00:00 2001 From: Steve Kondik Date: Fri, 22 Apr 2016 18:26:43 -0700 Subject: audiopolicy: Revert all session callback patches. * This has been rearchitected in a better way, as this feature turns out to be more difficult than it seems. * Reverting all of this stuff and rolling it into a single commit. This reverts commit c27a16c33c78a36482336a16199b1b8be794cea4. This reverts commit 32ef0556ae58ff6b7c6fe6fb0a17d3ff7f01de31. This reverts commit 489c9fb62f02e1d23d6d6c89b22f7d19c596e65e. This reverts commit a4123803d0a0e9e0c69faa4207d357cc74a65d58. This reverts commit e13b58b988ab642d4ae5ca6d0a89013510714956. This reverts commit 47f8c7303c9e2054f1492b02b6c7472385c52dc9. This reverts commit 0479d7c79a7fd6f112e8dc7e45c009cf6602dbaa. Change-Id: Iaed9f198d806aa414c95960713e8187c98db248b --- media/libmedia/AudioSystem.cpp | 34 +--------------------------- media/libmedia/IAudioPolicyService.cpp | 20 ---------------- media/libmedia/IAudioPolicyServiceClient.cpp | 31 +------------------------ 3 files changed, 2 insertions(+), 83 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/AudioSystem.cpp b/media/libmedia/AudioSystem.cpp index 5bd8747..9d645f0 100644 --- a/media/libmedia/AudioSystem.cpp +++ b/media/libmedia/AudioSystem.cpp @@ -37,7 +37,7 @@ sp AudioSystem::gAudioFlinger; sp AudioSystem::gAudioFlingerClient; audio_error_callback AudioSystem::gAudioErrorCallback = NULL; dynamic_policy_callback AudioSystem::gDynPolicyCallback = NULL; -effect_session_callback AudioSystem::gEffectSessionCallback = NULL; + // establish binder interface to AudioFlinger service const sp AudioSystem::get_audio_flinger() @@ -652,21 +652,6 @@ status_t AudioSystem::AudioFlingerClient::removeAudioDeviceCallback( gDynPolicyCallback = cb; } -/*static*/ status_t AudioSystem::setEffectSessionCallback(effect_session_callback cb) -{ - const sp& aps = AudioSystem::get_audio_policy_service(); - if (aps == 0) return PERMISSION_DENIED; - - Mutex::Autolock _l(gLock); - gEffectSessionCallback = cb; - - status_t status = aps->setEffectSessionCallbacksEnabled(cb != NULL); - if (status != OK) { - gEffectSessionCallback = NULL; - } - return status; -} - // client singleton for AudioPolicyService binder interface // protected by gLockAPS sp AudioSystem::gAudioPolicyService; @@ -1238,23 +1223,6 @@ void AudioSystem::AudioPolicyServiceClient::onDynamicPolicyMixStateUpdate( } } -void AudioSystem::AudioPolicyServiceClient::onOutputSessionEffectsUpdate( - audio_stream_type_t stream, audio_session_t sessionId, - audio_output_flags_t flags, audio_channel_mask_t channelMask, - uid_t uid, bool added) -{ - ALOGV("AudioPolicyServiceClient::onOutputSessionEffectsUpdate(%d, %d, %d)", stream, sessionId, added); - effect_session_callback cb = NULL; - { - Mutex::Autolock _l(AudioSystem::gLock); - cb = gEffectSessionCallback; - } - - if (cb != NULL) { - cb(AUDIO_OUTPUT_SESSION_EFFECTS_UPDATE, stream, sessionId, flags, channelMask, uid, added); - } -} - void AudioSystem::AudioPolicyServiceClient::binderDied(const wp& who __unused) { { diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp index 6ff8149..76b5924 100644 --- a/media/libmedia/IAudioPolicyService.cpp +++ b/media/libmedia/IAudioPolicyService.cpp @@ -74,7 +74,6 @@ enum { START_AUDIO_SOURCE, STOP_AUDIO_SOURCE, SET_AUDIO_PORT_CALLBACK_ENABLED, - SET_EFFECT_SESSION_CALLBACK_ENABLED, }; #define MAX_ITEMS_PER_LIST 1024 @@ -656,18 +655,6 @@ public: remote()->transact(SET_AUDIO_PORT_CALLBACK_ENABLED, data, &reply); } - virtual status_t setEffectSessionCallbacksEnabled(bool enabled) - { - Parcel data, reply; - data.writeInterfaceToken(IAudioPolicyService::getInterfaceDescriptor()); - data.writeInt32(enabled ? 1 : 0); - status_t status = remote()->transact(SET_EFFECT_SESSION_CALLBACK_ENABLED, data, &reply); - if (status != NO_ERROR) { - return status; - } - return (status_t)reply.readInt32(); - } - virtual status_t acquireSoundTriggerSession(audio_session_t *session, audio_io_handle_t *ioHandle, audio_devices_t *device) @@ -1251,13 +1238,6 @@ status_t BnAudioPolicyService::onTransact( return NO_ERROR; } break; - case SET_EFFECT_SESSION_CALLBACK_ENABLED: { - CHECK_INTERFACE(IAudioPolicyService, data, reply); - status_t status = setEffectSessionCallbacksEnabled(data.readInt32() == 1); - reply->writeInt32(status); - return NO_ERROR; - } break; - case ACQUIRE_SOUNDTRIGGER_SESSION: { CHECK_INTERFACE(IAudioPolicyService, data, reply); sp client = interface_cast( diff --git a/media/libmedia/IAudioPolicyServiceClient.cpp b/media/libmedia/IAudioPolicyServiceClient.cpp index ecf0d1f..65cc7d6 100644 --- a/media/libmedia/IAudioPolicyServiceClient.cpp +++ b/media/libmedia/IAudioPolicyServiceClient.cpp @@ -30,8 +30,7 @@ namespace android { enum { PORT_LIST_UPDATE = IBinder::FIRST_CALL_TRANSACTION, PATCH_LIST_UPDATE, - MIX_STATE_UPDATE, - OUTPUT_SESSION_EFFECTS_UPDATE + MIX_STATE_UPDATE }; class BpAudioPolicyServiceClient : public BpInterface @@ -64,23 +63,6 @@ public: data.writeInt32(state); remote()->transact(MIX_STATE_UPDATE, data, &reply, IBinder::FLAG_ONEWAY); } - - void onOutputSessionEffectsUpdate(audio_stream_type_t stream, - audio_session_t sessionId, - audio_output_flags_t flags, - audio_channel_mask_t channelMask, - uid_t uid, bool added) - { - Parcel data, reply; - data.writeInterfaceToken(IAudioPolicyServiceClient::getInterfaceDescriptor()); - data.writeInt32(stream); - data.writeInt32(sessionId); - data.writeInt32(flags); - data.writeInt32(channelMask); - data.writeInt32(uid); - data.writeInt32(added ? 1 : 0); - remote()->transact(OUTPUT_SESSION_EFFECTS_UPDATE, data, &reply, IBinder::FLAG_ONEWAY); - } }; IMPLEMENT_META_INTERFACE(AudioPolicyServiceClient, "android.media.IAudioPolicyServiceClient"); @@ -108,17 +90,6 @@ status_t BnAudioPolicyServiceClient::onTransact( onDynamicPolicyMixStateUpdate(regId, state); return NO_ERROR; } - case OUTPUT_SESSION_EFFECTS_UPDATE: { - CHECK_INTERFACE(IAudioPolicyServiceClient, data, reply); - audio_stream_type_t stream = static_cast(data.readInt32()); - audio_session_t sessionId = static_cast(data.readInt32()); - audio_output_flags_t flags = static_cast(data.readInt32()); - audio_channel_mask_t channelMask = static_cast(data.readInt32()); - uid_t uid = static_cast(data.readInt32()); - bool added = data.readInt32() > 0; - onOutputSessionEffectsUpdate(stream, sessionId, flags, channelMask, uid, added); - return NO_ERROR; - } default: return BBinder::onTransact(code, data, reply, flags); } -- cgit v1.1 From 3f9eb321481de3e118632a594bf1b0c9001c281c Mon Sep 17 00:00:00 2001 From: Steve Kondik Date: Fri, 22 Apr 2016 18:32:39 -0700 Subject: audiopolicy: Add AudioSessionInfo API * This patch introduces a new API which allows applications to query the state of the audio effects system, and receive callbacks with the necessary information to attach effects to any stream. * In the future, this may come as part of the AudioPort system, but since that's an active area of development by Google, we will dodge it for now. * The policy now simply keeps a refcounted list of objects which hold various bits of stream metadata. Callbacks are sent on stream open/close to applications which might be listening for them. Change-Id: I2d554d36e1378f4eb7b276010a3bfe8345c22ecd --- media/libmedia/AudioSystem.cpp | 39 +++++++++++++++++++++++++- media/libmedia/IAudioPolicyService.cpp | 42 ++++++++++++++++++++++++++++ media/libmedia/IAudioPolicyServiceClient.cpp | 30 +++++++++++++++++++- 3 files changed, 109 insertions(+), 2 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/AudioSystem.cpp b/media/libmedia/AudioSystem.cpp index 9d645f0..2e9fca9 100644 --- a/media/libmedia/AudioSystem.cpp +++ b/media/libmedia/AudioSystem.cpp @@ -37,7 +37,7 @@ sp AudioSystem::gAudioFlinger; sp AudioSystem::gAudioFlingerClient; audio_error_callback AudioSystem::gAudioErrorCallback = NULL; dynamic_policy_callback AudioSystem::gDynPolicyCallback = NULL; - +audio_session_callback AudioSystem::gAudioSessionCallback = NULL; // establish binder interface to AudioFlinger service const sp AudioSystem::get_audio_flinger() @@ -652,6 +652,17 @@ status_t AudioSystem::AudioFlingerClient::removeAudioDeviceCallback( gDynPolicyCallback = cb; } +/*static*/ status_t AudioSystem::setAudioSessionCallback(audio_session_callback cb) +{ + const sp& aps = AudioSystem::get_audio_policy_service(); + if (aps == 0) return PERMISSION_DENIED; + + Mutex::Autolock _l(gLock); + gAudioSessionCallback = cb; + + return NO_ERROR; +} + // client singleton for AudioPolicyService binder interface // protected by gLockAPS sp AudioSystem::gAudioPolicyService; @@ -1223,6 +1234,32 @@ void AudioSystem::AudioPolicyServiceClient::onDynamicPolicyMixStateUpdate( } } +// --------------------------------------------------------------------------- + +status_t AudioSystem::listAudioSessions(audio_stream_type_t stream, + Vector< sp> &sessions) +{ + const sp& aps = AudioSystem::get_audio_policy_service(); + if (aps == 0) return PERMISSION_DENIED; + return aps->listAudioSessions(stream, sessions); +} + +void AudioSystem::AudioPolicyServiceClient::onOutputSessionEffectsUpdate( + sp& info, bool added) +{ + ALOGV("AudioPolicyServiceClient::onOutputSessionEffectsUpdate(%d, %d, %d)", + info->mStream, info->mSessionId, added); + audio_session_callback cb = NULL; + { + Mutex::Autolock _l(AudioSystem::gLock); + cb = gAudioSessionCallback; + } + + if (cb != NULL) { + cb(AUDIO_OUTPUT_SESSION_EFFECTS_UPDATE, info, added); + } +} + void AudioSystem::AudioPolicyServiceClient::binderDied(const wp& who __unused) { { diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp index 76b5924..abae614 100644 --- a/media/libmedia/IAudioPolicyService.cpp +++ b/media/libmedia/IAudioPolicyService.cpp @@ -74,6 +74,7 @@ enum { START_AUDIO_SOURCE, STOP_AUDIO_SOURCE, SET_AUDIO_PORT_CALLBACK_ENABLED, + LIST_AUDIO_SESSIONS, }; #define MAX_ITEMS_PER_LIST 1024 @@ -767,6 +768,30 @@ public: status = (status_t)reply.readInt32(); return status; } + + virtual status_t listAudioSessions(audio_stream_type_t streams, + Vector< sp> &sessions) + { + Parcel data, reply; + data.writeInterfaceToken(IAudioPolicyService::getInterfaceDescriptor()); + data.writeInt32(streams); + status_t status = remote()->transact(LIST_AUDIO_SESSIONS, data, &reply); + if (status != NO_ERROR) { + return status; + } + + status = reply.readInt32(); + if (status == NO_ERROR) { + size_t size = (size_t)reply.readUint32(); + for (size_t i = 0; i < size && reply.dataAvail() > 0; i++) { + sp info = new AudioSessionInfo(); + info->readFromParcel(reply); + sessions.push_back(info); + } + } + return status; + } + }; IMPLEMENT_META_INTERFACE(AudioPolicyService, "android.media.IAudioPolicyService"); @@ -1238,6 +1263,23 @@ status_t BnAudioPolicyService::onTransact( return NO_ERROR; } break; + case LIST_AUDIO_SESSIONS: { + CHECK_INTERFACE(IAudioPolicyService, data, reply); + audio_stream_type_t streams = (audio_stream_type_t)data.readInt32(); + + Vector< sp> sessions; + status_t status = listAudioSessions(streams, sessions); + + reply->writeInt32(status); + if (status == NO_ERROR) { + reply->writeUint32(static_cast(sessions.size())); + for (size_t i = 0; i < sessions.size(); i++) { + sessions[i]->writeToParcel(reply); + } + } + return NO_ERROR; + } + case ACQUIRE_SOUNDTRIGGER_SESSION: { CHECK_INTERFACE(IAudioPolicyService, data, reply); sp client = interface_cast( diff --git a/media/libmedia/IAudioPolicyServiceClient.cpp b/media/libmedia/IAudioPolicyServiceClient.cpp index 65cc7d6..a325996 100644 --- a/media/libmedia/IAudioPolicyServiceClient.cpp +++ b/media/libmedia/IAudioPolicyServiceClient.cpp @@ -30,7 +30,8 @@ namespace android { enum { PORT_LIST_UPDATE = IBinder::FIRST_CALL_TRANSACTION, PATCH_LIST_UPDATE, - MIX_STATE_UPDATE + MIX_STATE_UPDATE, + OUTPUT_SESSION_EFFECTS_UPDATE }; class BpAudioPolicyServiceClient : public BpInterface @@ -63,6 +64,19 @@ public: data.writeInt32(state); remote()->transact(MIX_STATE_UPDATE, data, &reply, IBinder::FLAG_ONEWAY); } + + void onOutputSessionEffectsUpdate(sp& info, bool added) + { + Parcel data, reply; + data.writeInterfaceToken(IAudioPolicyServiceClient::getInterfaceDescriptor()); + data.writeInt32(info->mStream); + data.writeInt32(info->mSessionId); + data.writeInt32(info->mFlags); + data.writeInt32(info->mChannelMask); + data.writeInt32(info->mUid); + data.writeInt32(added ? 1 : 0); + remote()->transact(OUTPUT_SESSION_EFFECTS_UPDATE, data, &reply, IBinder::FLAG_ONEWAY); + } }; IMPLEMENT_META_INTERFACE(AudioPolicyServiceClient, "android.media.IAudioPolicyServiceClient"); @@ -90,6 +104,20 @@ status_t BnAudioPolicyServiceClient::onTransact( onDynamicPolicyMixStateUpdate(regId, state); return NO_ERROR; } + case OUTPUT_SESSION_EFFECTS_UPDATE: { + CHECK_INTERFACE(IAudioPolicyServiceClient, data, reply); + audio_stream_type_t stream = static_cast(data.readInt32()); + audio_session_t sessionId = static_cast(data.readInt32()); + audio_output_flags_t flags = static_cast(data.readInt32()); + audio_channel_mask_t channelMask = static_cast(data.readInt32()); + uid_t uid = static_cast(data.readInt32()); + bool added = data.readInt32() > 0; + + sp info = new AudioSessionInfo( + sessionId, stream, flags, channelMask, uid); + onOutputSessionEffectsUpdate(info, added); + return NO_ERROR; + } default: return BBinder::onTransact(code, data, reply, flags); } -- cgit v1.1 From 6fdee2a83432b3b150d6a34f231c4e2f7353c01e Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Fri, 13 May 2016 10:39:23 -0700 Subject: limit mediaserver memory Limit mediaserver using rlimit, to prevent it from bringing down the system via the low memory killer. Default max is 65% of total RAM, but can be customized via system property. Bug: 28471206 Bug: 28615448 Change-Id: Ic84137435d1ef0a6883e9789a4b4f399e4283f05 --- media/libmedia/Android.mk | 1 + media/libmedia/MediaUtils.cpp | 74 +++++++++++++++++++++++++++++++++++++++++++ media/libmedia/MediaUtils.h | 35 ++++++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 media/libmedia/MediaUtils.cpp create mode 100644 media/libmedia/MediaUtils.h (limited to 'media/libmedia') diff --git a/media/libmedia/Android.mk b/media/libmedia/Android.mk index a3c3d3c..9f836f0 100644 --- a/media/libmedia/Android.mk +++ b/media/libmedia/Android.mk @@ -44,6 +44,7 @@ LOCAL_SRC_FILES:= \ IResourceManagerService.cpp \ IStreamSource.cpp \ MediaCodecInfo.cpp \ + MediaUtils.cpp \ Metadata.cpp \ mediarecorder.cpp \ IMediaMetadataRetriever.cpp \ diff --git a/media/libmedia/MediaUtils.cpp b/media/libmedia/MediaUtils.cpp new file mode 100644 index 0000000..a02ca65 --- /dev/null +++ b/media/libmedia/MediaUtils.cpp @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "MediaUtils" +#define LOG_NDEBUG 0 +#include + +#include +#include +#include + +#include "MediaUtils.h" + +namespace android { + +void limitProcessMemory( + const char *property, + size_t numberOfBytes, + size_t percentageOfTotalMem) { + + long pageSize = sysconf(_SC_PAGESIZE); + long numPages = sysconf(_SC_PHYS_PAGES); + size_t maxMem = SIZE_MAX; + + if (pageSize > 0 && numPages > 0) { + if (size_t(numPages) < SIZE_MAX / size_t(pageSize)) { + maxMem = size_t(numPages) * size_t(pageSize); + } + ALOGV("physMem: %zu", maxMem); + if (percentageOfTotalMem > 100) { + ALOGW("requested %zu%% of total memory, using 100%%", percentageOfTotalMem); + percentageOfTotalMem = 100; + } + maxMem = maxMem / 100 * percentageOfTotalMem; + if (numberOfBytes < maxMem) { + maxMem = numberOfBytes; + } + ALOGV("requested limit: %zu", maxMem); + } else { + ALOGW("couldn't determine total RAM"); + } + + int64_t propVal = property_get_int64(property, maxMem); + if (propVal > 0 && uint64_t(propVal) <= SIZE_MAX) { + maxMem = propVal; + } + ALOGV("actual limit: %zu", maxMem); + + struct rlimit limit; + getrlimit(RLIMIT_AS, &limit); + ALOGV("original limits: %lld/%lld", (long long)limit.rlim_cur, (long long)limit.rlim_max); + limit.rlim_cur = maxMem; + setrlimit(RLIMIT_AS, &limit); + limit.rlim_cur = -1; + limit.rlim_max = -1; + getrlimit(RLIMIT_AS, &limit); + ALOGV("new limits: %lld/%lld", (long long)limit.rlim_cur, (long long)limit.rlim_max); + +} + +} // namespace android diff --git a/media/libmedia/MediaUtils.h b/media/libmedia/MediaUtils.h new file mode 100644 index 0000000..f80dd30 --- /dev/null +++ b/media/libmedia/MediaUtils.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef _MEDIA_UTILS_H +#define _MEDIA_UTILS_H + +namespace android { + +/** + Limit the amount of memory a process can allocate using setrlimit(RLIMIT_AS). + The value to use will be read from the specified system property, or if the + property doesn't exist it will use the specified number of bytes or the + specified percentage of total memory, whichever is smaller. +*/ +void limitProcessMemory( + const char *property, + size_t numberOfBytes, + size_t percentageOfTotalMem); + +} // namespace android + +#endif // _MEDIA_UTILS_H -- cgit v1.1 From 42a25c46b844518ff0d0b920c20c519e1417be69 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 7 Jun 2016 12:26:43 -0700 Subject: Don't use sp<>& because they may end up pointing to NULL after a NULL check was performed. Bug: 28166152 Change-Id: Iab2ea30395b620628cc6f3d067dd4f6fcda824fe --- media/libmedia/IMediaDeathNotifier.cpp | 2 +- media/libmedia/mediaplayer.cpp | 6 +++--- media/libmedia/mediarecorder.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IMediaDeathNotifier.cpp b/media/libmedia/IMediaDeathNotifier.cpp index d4360ea..c43ef66 100644 --- a/media/libmedia/IMediaDeathNotifier.cpp +++ b/media/libmedia/IMediaDeathNotifier.cpp @@ -31,7 +31,7 @@ sp IMediaDeathNotifier::sDeathNotifier; SortedVector< wp > IMediaDeathNotifier::sObitRecipients; // establish binder interface to MediaPlayerService -/*static*/const sp& +/*static*/const sp IMediaDeathNotifier::getMediaPlayerService() { ALOGV("getMediaPlayerService"); diff --git a/media/libmedia/mediaplayer.cpp b/media/libmedia/mediaplayer.cpp index 502ab2d..dcce3cc 100644 --- a/media/libmedia/mediaplayer.cpp +++ b/media/libmedia/mediaplayer.cpp @@ -152,7 +152,7 @@ status_t MediaPlayer::setDataSource( ALOGV("setDataSource(%s)", url); status_t err = BAD_VALUE; if (url != NULL) { - const sp& service(getMediaPlayerService()); + const sp service(getMediaPlayerService()); if (service != 0) { sp player(service->create(this, mAudioSessionId)); if ((NO_ERROR != doSetRetransmitEndpoint(player)) || @@ -169,7 +169,7 @@ status_t MediaPlayer::setDataSource(int fd, int64_t offset, int64_t length) { ALOGV("setDataSource(%d, %" PRId64 ", %" PRId64 ")", fd, offset, length); status_t err = UNKNOWN_ERROR; - const sp& service(getMediaPlayerService()); + const sp service(getMediaPlayerService()); if (service != 0) { sp player(service->create(this, mAudioSessionId)); if ((NO_ERROR != doSetRetransmitEndpoint(player)) || @@ -185,7 +185,7 @@ status_t MediaPlayer::setDataSource(const sp &source) { ALOGV("setDataSource"); status_t err = UNKNOWN_ERROR; - const sp& service(getMediaPlayerService()); + const sp service(getMediaPlayerService()); if (service != 0) { sp player(service->create(this, mAudioSessionId)); if ((NO_ERROR != doSetRetransmitEndpoint(player)) || diff --git a/media/libmedia/mediarecorder.cpp b/media/libmedia/mediarecorder.cpp index 8bbd8f1..0eb7de5 100644 --- a/media/libmedia/mediarecorder.cpp +++ b/media/libmedia/mediarecorder.cpp @@ -617,7 +617,7 @@ MediaRecorder::MediaRecorder(const String16& opPackageName) : mSurfaceMediaSourc { ALOGV("constructor"); - const sp& service(getMediaPlayerService()); + const sp service(getMediaPlayerService()); if (service != NULL) { mMediaRecorder = service->createMediaRecorder(opPackageName); } -- cgit v1.1 From bc8a45f506a8be33250c523d71fab637a5fdaf81 Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Tue, 2 Aug 2016 07:07:05 -0700 Subject: IOMX: work against metadata buffer spoofing - Prohibit direct set/getParam/Settings for extensions meant for OMXNodeInstance alone. This disallows enabling metadata mode without the knowledge of OMXNodeInstance. - Do not share metadata mode buffers cross process. - Disallow setting up metadata mode/tunneling/input surface after first sendCommand. - Disallow store-meta for input cross process. - Disallow emptyBuffer for surface input (via IOMX). - Fix checking for input surface. Bug: 29422020 Change-Id: I801c77b80e703903f62e42d76fd2e76a34e4bc8e (cherry picked from commit f8a4cb410115045278f534e54b41ac78d6bf6c07) --- media/libmedia/IOMX.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index b082fe4..c28eac8 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -248,7 +248,7 @@ public: virtual status_t useBuffer( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL /* crossProcess */) { Parcel data, reply; data.writeInterfaceToken(IOMX::getInterfaceDescriptor()); data.writeInt32((int32_t)node); @@ -481,7 +481,7 @@ public: virtual status_t allocateBufferWithBackup( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL /* crossProcess */) { Parcel data, reply; data.writeInterfaceToken(IOMX::getInterfaceDescriptor()); data.writeInt32((int32_t)node); @@ -834,7 +834,8 @@ status_t BnOMX::onTransact( OMX_U32 allottedSize = data.readInt32(); buffer_id buffer; - status_t err = useBuffer(node, port_index, params, &buffer, allottedSize); + status_t err = useBuffer( + node, port_index, params, &buffer, allottedSize, OMX_TRUE /* crossProcess */); reply->writeInt32(err); if (err == OK) { @@ -969,7 +970,10 @@ status_t BnOMX::onTransact( OMX_BOOL enable = (OMX_BOOL)data.readInt32(); MetadataBufferType type = kMetadataBufferTypeInvalid; - status_t err = storeMetaDataInBuffers(node, port_index, enable, &type); + status_t err = + // only control output metadata via Binder + port_index != 1 /* kOutputPortIndex */ ? BAD_VALUE : + storeMetaDataInBuffers(node, port_index, enable, &type); reply->writeInt32(type); reply->writeInt32(err); @@ -1054,7 +1058,7 @@ status_t BnOMX::onTransact( buffer_id buffer; status_t err = allocateBufferWithBackup( - node, port_index, params, &buffer, allottedSize); + node, port_index, params, &buffer, allottedSize, OMX_TRUE /* crossProcess */); reply->writeInt32(err); -- cgit v1.1 From ace612c5f838944d1b88be18e317709e640becc2 Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Fri, 2 Sep 2016 11:36:46 -0700 Subject: Fix stack content leak vulnerability in mediaserver bug: 30875060 Change-Id: I03f4d08b7b31ac5b507cfc9e65e5607c73972d95 (cherry picked from commit 9a6861cbd3bb0e1b8fe4c105795256ee032f9664) --- media/libmedia/IDrm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IDrm.cpp b/media/libmedia/IDrm.cpp index 7c709cd..6f6530b 100644 --- a/media/libmedia/IDrm.cpp +++ b/media/libmedia/IDrm.cpp @@ -912,7 +912,7 @@ status_t BnDrm::onTransact( readVector(data, keyId); readVector(data, message); readVector(data, signature); - bool match; + bool match = false; uint32_t result = verify(sessionId, keyId, message, signature, match); reply->writeInt32(match); reply->writeInt32(result); -- cgit v1.1 From c2dd82bfd6d7aea5b6760efc0712ae11d7a52e6b Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 20 Sep 2016 13:36:40 -0700 Subject: Check mprotect result mprotect can theoretically fail, which could then let one exploit a vulnerable codec if one exists on the device. Bug: 31350239 Change-Id: I7b99c190619f0fb2eb93119596e6da0d2deb8ba5 (cherry picked from commit 866c800c0624bb13eee44973cc8a2ecd0012de6e) --- media/libmedia/IOMX.cpp | 52 ++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 24 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index c28eac8..365d9ac 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -733,31 +733,35 @@ status_t BnOMX::onTransact( // mark the last page as inaccessible, to avoid exploitation // of codecs that access past the end of the allocation because // they didn't check the size - mprotect((char*)params + allocSize - pageSize, pageSize, PROT_NONE); - switch (code) { - case GET_PARAMETER: - err = getParameter(node, index, params, size); - break; - case SET_PARAMETER: - err = setParameter(node, index, params, size); - break; - case GET_CONFIG: - err = getConfig(node, index, params, size); - break; - case SET_CONFIG: - err = setConfig(node, index, params, size); - break; - case SET_INTERNAL_OPTION: - { - InternalOptionType type = - (InternalOptionType)data.readInt32(); - - err = setInternalOption(node, index, type, params, size); - break; + if (mprotect((char*)params + allocSize - pageSize, pageSize, + PROT_NONE) != 0) { + ALOGE("mprotect failed: %s", strerror(errno)); + } else { + switch (code) { + case GET_PARAMETER: + err = getParameter(node, index, params, size); + break; + case SET_PARAMETER: + err = setParameter(node, index, params, size); + break; + case GET_CONFIG: + err = getConfig(node, index, params, size); + break; + case SET_CONFIG: + err = setConfig(node, index, params, size); + break; + case SET_INTERNAL_OPTION: + { + InternalOptionType type = + (InternalOptionType)data.readInt32(); + + err = setInternalOption(node, index, type, params, size); + break; + } + + default: + TRESPASS(); } - - default: - TRESPASS(); } } } -- cgit v1.1