From 44deb053252a3bd2f57a007ab9560f4924f62394 Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Sun, 5 Feb 2012 18:09:08 -0800 Subject: Factor out and speed up permission-checking code Use the caching permission check for dump to save IPC. Cache getpid() to save kernel call for other permission checks. The C runtime library getpid() can't cache due to a fork race condition, but we know that mediaserver doesn't fork. Don't construct String16 on the stack. Change-Id: I6be6161dae5155d39ba6ed6228e7683e67be34ed --- services/audioflinger/Android.mk | 3 +- services/audioflinger/AudioFlinger.cpp | 25 +++---------- services/audioflinger/AudioPolicyService.cpp | 20 ++++------ services/audioflinger/ServiceUtilities.cpp | 55 ++++++++++++++++++++++++++++ services/audioflinger/ServiceUtilities.h | 27 ++++++++++++++ 5 files changed, 97 insertions(+), 33 deletions(-) create mode 100644 services/audioflinger/ServiceUtilities.cpp create mode 100644 services/audioflinger/ServiceUtilities.h diff --git a/services/audioflinger/Android.mk b/services/audioflinger/Android.mk index deeb2de..157405a 100644 --- a/services/audioflinger/Android.mk +++ b/services/audioflinger/Android.mk @@ -6,7 +6,8 @@ LOCAL_SRC_FILES:= \ AudioFlinger.cpp \ AudioMixer.cpp.arm \ AudioResampler.cpp.arm \ - AudioPolicyService.cpp + AudioPolicyService.cpp \ + ServiceUtilities.cpp # AudioResamplerSinc.cpp.arm # AudioResamplerCubic.cpp.arm diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index bbc9196..5c964b2 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -48,6 +48,7 @@ #include "AudioMixer.h" #include "AudioFlinger.h" +#include "ServiceUtilities.h" #include #include @@ -101,20 +102,6 @@ static const uint32_t kMaxThreadSleepTimeShift = 2; // ---------------------------------------------------------------------------- -static bool recordingAllowed() { - if (getpid() == IPCThreadState::self()->getCallingPid()) return true; - bool ok = checkCallingPermission(String16("android.permission.RECORD_AUDIO")); - if (!ok) ALOGE("Request requires android.permission.RECORD_AUDIO"); - return ok; -} - -static bool settingsAllowed() { - if (getpid() == IPCThreadState::self()->getCallingPid()) return true; - bool ok = checkCallingPermission(String16("android.permission.MODIFY_AUDIO_SETTINGS")); - if (!ok) ALOGE("Request requires android.permission.MODIFY_AUDIO_SETTINGS"); - return ok; -} - // To collect the amplifier usage static void addBatteryData(uint32_t params) { sp service = IMediaDeathNotifier::getMediaPlayerService(); @@ -321,7 +308,7 @@ static bool tryLock(Mutex& mutex) status_t AudioFlinger::dump(int fd, const Vector& args) { - if (!checkCallingPermission(String16("android.permission.DUMP"))) { + if (!dumpAllowed()) { dumpPermissionDenial(fd, args); } else { // get state of hardware lock @@ -3436,7 +3423,7 @@ void AudioFlinger::PlaybackThread::Track::dump(char* buffer, size_t size) uint32_t vlr = mCblk->getVolumeLR(); snprintf(buffer, size, " %05d %05d %03u %03u 0x%08x %05u %04u %1d %1d %1d %05u %05u %05u 0x%08x 0x%08x 0x%08x 0x%08x\n", mName - AudioMixer::TRACK0, - (mClient == 0) ? getpid() : mClient->pid(), + (mClient == 0) ? getpid_cached : mClient->pid(), mStreamType, mFormat, mChannelMask, @@ -3757,7 +3744,7 @@ void AudioFlinger::RecordThread::RecordTrack::stop() void AudioFlinger::RecordThread::RecordTrack::dump(char* buffer, size_t size) { snprintf(buffer, size, " %05d %03u 0x%08x %05d %04u %01d %05u %08x %08x\n", - (mClient == 0) ? getpid() : mClient->pid(), + (mClient == 0) ? getpid_cached : mClient->pid(), mFormat, mChannelMask, mSessionId, @@ -5404,7 +5391,7 @@ sp AudioFlinger::createEffect(pid_t pid, // Session AUDIO_SESSION_OUTPUT_STAGE is reserved for output stage effects // that can only be created by audio policy manager (running in same process) - if (sessionId == AUDIO_SESSION_OUTPUT_STAGE && getpid() != pid) { + if (sessionId == AUDIO_SESSION_OUTPUT_STAGE && getpid_cached != pid) { lStatus = PERMISSION_DENIED; goto Exit; } @@ -7022,7 +7009,7 @@ void AudioFlinger::EffectHandle::dump(char* buffer, size_t size) bool locked = mCblk != NULL && tryLock(mCblk->lock); snprintf(buffer, size, "\t\t\t%05d %05d %01u %01u %05u %05u\n", - (mClient == 0) ? getpid() : mClient->pid(), + (mClient == 0) ? getpid_cached : mClient->pid(), mPriority, mHasControl, !locked, diff --git a/services/audioflinger/AudioPolicyService.cpp b/services/audioflinger/AudioPolicyService.cpp index f8b430e..21b5811 100644 --- a/services/audioflinger/AudioPolicyService.cpp +++ b/services/audioflinger/AudioPolicyService.cpp @@ -30,6 +30,7 @@ #include #include #include "AudioPolicyService.h" +#include "ServiceUtilities.h" #include #include #include @@ -49,13 +50,6 @@ static const char kCmdDeadlockedString[] = "AudioPolicyService command thread ma static const int kDumpLockRetries = 50; static const int kDumpLockSleepUs = 20000; -static bool checkPermission() { - if (getpid() == IPCThreadState::self()->getCallingPid()) return true; - bool ok = checkCallingPermission(String16("android.permission.MODIFY_AUDIO_SETTINGS")); - if (!ok) ALOGE("Request requires android.permission.MODIFY_AUDIO_SETTINGS"); - return ok; -} - namespace { extern struct audio_policy_service_ops aps_ops; }; @@ -157,7 +151,7 @@ status_t AudioPolicyService::setDeviceConnectionState(audio_devices_t device, if (mpAudioPolicy == NULL) { return NO_INIT; } - if (!checkPermission()) { + if (!settingsAllowed()) { return PERMISSION_DENIED; } if (!audio_is_output_device(device) && !audio_is_input_device(device)) { @@ -190,7 +184,7 @@ status_t AudioPolicyService::setPhoneState(audio_mode_t state) if (mpAudioPolicy == NULL) { return NO_INIT; } - if (!checkPermission()) { + if (!settingsAllowed()) { return PERMISSION_DENIED; } if (uint32_t(state) >= AUDIO_MODE_CNT) { @@ -213,7 +207,7 @@ status_t AudioPolicyService::setForceUse(audio_policy_force_use_t usage, if (mpAudioPolicy == NULL) { return NO_INIT; } - if (!checkPermission()) { + if (!settingsAllowed()) { return PERMISSION_DENIED; } if (usage < 0 || usage >= AUDIO_POLICY_FORCE_USE_CNT) { @@ -388,7 +382,7 @@ status_t AudioPolicyService::initStreamVolume(audio_stream_type_t stream, if (mpAudioPolicy == NULL) { return NO_INIT; } - if (!checkPermission()) { + if (!settingsAllowed()) { return PERMISSION_DENIED; } if (uint32_t(stream) >= AUDIO_STREAM_CNT) { @@ -405,7 +399,7 @@ status_t AudioPolicyService::setStreamVolumeIndex(audio_stream_type_t stream, if (mpAudioPolicy == NULL) { return NO_INIT; } - if (!checkPermission()) { + if (!settingsAllowed()) { return PERMISSION_DENIED; } if (uint32_t(stream) >= AUDIO_STREAM_CNT) { @@ -578,7 +572,7 @@ status_t AudioPolicyService::dumpInternals(int fd) status_t AudioPolicyService::dump(int fd, const Vector& args) { - if (!checkCallingPermission(String16("android.permission.DUMP"))) { + if (!dumpAllowed()) { dumpPermissionDenial(fd); } else { bool locked = tryLock(mLock); diff --git a/services/audioflinger/ServiceUtilities.cpp b/services/audioflinger/ServiceUtilities.cpp new file mode 100644 index 0000000..6a58852 --- /dev/null +++ b/services/audioflinger/ServiceUtilities.cpp @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2012 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. + */ + +#include +#include +#include +#include "ServiceUtilities.h" + +namespace android { + +// This optimization assumes mediaserver process doesn't fork, which it doesn't +const pid_t getpid_cached = getpid(); + +bool recordingAllowed() { + if (getpid_cached == IPCThreadState::self()->getCallingPid()) return true; + static const String16 sRecordAudio("android.permission.RECORD_AUDIO"); + // don't use PermissionCache; this is not a system permission + bool ok = checkCallingPermission(sRecordAudio); + if (!ok) ALOGE("Request requires android.permission.RECORD_AUDIO"); + return ok; +} + +bool settingsAllowed() { + if (getpid_cached == IPCThreadState::self()->getCallingPid()) return true; + static const String16 sAudioSettings("android.permission.MODIFY_AUDIO_SETTINGS"); + // don't use PermissionCache; this is not a system permission + bool ok = checkCallingPermission(sAudioSettings); + if (!ok) ALOGE("Request requires android.permission.MODIFY_AUDIO_SETTINGS"); + return ok; +} + +bool dumpAllowed() { + // don't optimize for same pid, since mediaserver never dumps itself + static const String16 sDump("android.permission.DUMP"); + // OK to use PermissionCache; this is a system permission + bool ok = PermissionCache::checkCallingPermission(sDump); + // convention is for caller to dump an error message to fd instead of logging here + //if (!ok) ALOGE("Request requires android.permission.DUMP"); + return ok; +} + +} // namespace android diff --git a/services/audioflinger/ServiceUtilities.h b/services/audioflinger/ServiceUtilities.h new file mode 100644 index 0000000..f77ec5b --- /dev/null +++ b/services/audioflinger/ServiceUtilities.h @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2012 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. + */ + +#include + +namespace android { + +extern const pid_t getpid_cached; + +bool recordingAllowed(); +bool settingsAllowed(); +bool dumpAllowed(); + +} -- cgit v1.1