From cd5482bfac57ad358b663dff6adcc3582038c51a Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Thu, 15 Dec 2016 14:46:09 -0800 Subject: DO NOT MERGE - audioflinger: fix recursive mutex lock in EffectHandle. Bug: 33661708 Bug: 32707507 Bug: 32095713 Test: run CTS AudioEffectTest#test5_0Command, Custom binder test CVE-2017-0479 CVE-2017-0480 Change-Id: I03f674f126c191143bd8bdfe236f793e975826a5 (cherry picked from commit 31a4598a1908b3ccac7ddb33c511ce66840aa911) (cherry picked from commit 8415635765380be496da9b4578d8f134a527d86b) --- services/audioflinger/Effects.cpp | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp index fb9e157..8a8b05b 100644 --- a/services/audioflinger/Effects.cpp +++ b/services/audioflinger/Effects.cpp @@ -1284,6 +1284,24 @@ status_t AudioFlinger::EffectHandle::command(uint32_t cmdCode, ALOGVV("command(), cmdCode: %d, mHasControl: %d, mEffect: %p", cmdCode, mHasControl, mEffect.unsafe_get()); + if (cmdCode == EFFECT_CMD_ENABLE) { + if (*replySize < sizeof(int)) { + android_errorWriteLog(0x534e4554, "32095713"); + return BAD_VALUE; + } + *(int *)pReplyData = NO_ERROR; + *replySize = sizeof(int); + return enable(); + } else if (cmdCode == EFFECT_CMD_DISABLE) { + if (*replySize < sizeof(int)) { + android_errorWriteLog(0x534e4554, "32095713"); + return BAD_VALUE; + } + *(int *)pReplyData = NO_ERROR; + *replySize = sizeof(int); + return disable(); + } + AutoMutex _l(mLock); sp effect = mEffect.promote(); if (effect == 0 || mDisconnected) { @@ -1299,11 +1317,17 @@ status_t AudioFlinger::EffectHandle::command(uint32_t cmdCode, // handle commands that are not forwarded transparently to effect engine if (cmdCode == EFFECT_CMD_SET_PARAM_COMMIT) { + if (*replySize < sizeof(int)) { + android_errorWriteLog(0x534e4554, "32095713"); + return BAD_VALUE; + } + *(int *)pReplyData = NO_ERROR; + *replySize = sizeof(int); + // No need to trylock() here as this function is executed in the binder thread serving a // particular client process: no risk to block the whole media server process or mixer // threads if we are stuck here Mutex::Autolock _l(mCblk->lock); - // keep local copy of index in case of client corruption b/32220769 const uint32_t clientIndex = mCblk->clientIndex; const uint32_t serverIndex = mCblk->serverIndex; @@ -1366,12 +1390,6 @@ status_t AudioFlinger::EffectHandle::command(uint32_t cmdCode, mCblk->serverIndex = 0; mCblk->clientIndex = 0; return status; - } else if (cmdCode == EFFECT_CMD_ENABLE) { - *(int *)pReplyData = NO_ERROR; - return enable(); - } else if (cmdCode == EFFECT_CMD_DISABLE) { - *(int *)pReplyData = NO_ERROR; - return disable(); } return effect->command(cmdCode, cmdSize, pCmdData, replySize, pReplyData); -- cgit v1.1