From d0446dc8c411d5032c2b449e6f923a9d7b1f4adf Mon Sep 17 00:00:00 2001 From: "hyomin.oh" Date: Thu, 18 Oct 2018 13:58:27 +0900 Subject: [PATCH] Fixed unsafe lock upon safe media volume Safe media volume uses lock as Integer. But it is not safe since the Integer value is changed. disableSafeMediaVolume() synchronized (mSafeMediaVolumeState) // step. 1 setSafeMediaVolumeEnabled() mSafeMediaVolumeState = SAFE_MEDIA_VOLUME_ACTIVE // step. 2 ... onSetStreamVolume(mPendingVolumeCommand.mStreamType, // step.4 -> mPendingVolumeCommand is set as null by step.3 -> it causes NPE and reboot ------------ setStreamVolume() synchronized (mSafeMediaVolumeState) -> mSafeMediaVolumeState was changed by step.2 -> so that it would go next step mPendingVolumeCommand = null; // step. 3 Test: Build Pass, manual test, change volume Change-Id: I33f473d42ccbf0f9b177c6886622ecc2f8020f8d --- .../android/server/audio/AudioService.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index d05652318ea93..262e389e61bd3 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -819,9 +819,9 @@ public class AudioService extends IAudioService.Stub new String("AudioService ctor"), 0); - mSafeMediaVolumeState = new Integer(Settings.Global.getInt(mContentResolver, - Settings.Global.AUDIO_SAFE_VOLUME_STATE, - SAFE_MEDIA_VOLUME_NOT_CONFIGURED)); + mSafeMediaVolumeState = Settings.Global.getInt(mContentResolver, + Settings.Global.AUDIO_SAFE_VOLUME_STATE, + SAFE_MEDIA_VOLUME_NOT_CONFIGURED); // The default safe volume index read here will be replaced by the actual value when // the mcc is read by onConfigureSafeVolume() mSafeMediaVolumeIndex = mContext.getResources().getInteger( @@ -1658,7 +1658,7 @@ public class AudioService extends IAudioService.Stub } // reset any pending volume command - synchronized (mSafeMediaVolumeState) { + synchronized (mSafeMediaVolumeStateLock) { mPendingVolumeCommand = null; } @@ -2011,7 +2011,7 @@ public class AudioService extends IAudioService.Stub return; } - synchronized (mSafeMediaVolumeState) { + synchronized (mSafeMediaVolumeStateLock) { // reset any pending volume command mPendingVolumeCommand = null; @@ -3238,7 +3238,7 @@ public class AudioService extends IAudioService.Stub checkAllAliasStreamVolumes(); checkMuteAffectedStreams(); - synchronized (mSafeMediaVolumeState) { + synchronized (mSafeMediaVolumeStateLock) { mMusicActiveMs = MathUtils.constrain(Settings.Secure.getIntForUser(mContentResolver, Settings.Secure.UNSAFE_VOLUME_MUSIC_ACTIVE_MS, 0, UserHandle.USER_CURRENT), 0, UNSAFE_VOLUME_MUSIC_ACTIVE_MS_MAX); @@ -4037,7 +4037,7 @@ public class AudioService extends IAudioService.Stub } private void onCheckMusicActive(String caller) { - synchronized (mSafeMediaVolumeState) { + synchronized (mSafeMediaVolumeStateLock) { if (mSafeMediaVolumeState == SAFE_MEDIA_VOLUME_INACTIVE) { int device = getDeviceForStream(AudioSystem.STREAM_MUSIC); @@ -4098,7 +4098,7 @@ public class AudioService extends IAudioService.Stub } private void onConfigureSafeVolume(boolean force, String caller) { - synchronized (mSafeMediaVolumeState) { + synchronized (mSafeMediaVolumeStateLock) { int mcc = mContext.getResources().getConfiguration().mcc; if ((mMcc != mcc) || ((mMcc == 0) && force)) { mSafeMediaVolumeIndex = mContext.getResources().getInteger( @@ -6952,7 +6952,8 @@ public class AudioService extends IAudioService.Stub private static final int SAFE_MEDIA_VOLUME_DISABLED = 1; private static final int SAFE_MEDIA_VOLUME_INACTIVE = 2; // confirmed private static final int SAFE_MEDIA_VOLUME_ACTIVE = 3; // unconfirmed - private Integer mSafeMediaVolumeState; + private int mSafeMediaVolumeState; + private final Object mSafeMediaVolumeStateLock = new Object(); private int mMcc = 0; // mSafeMediaVolumeIndex is the cached value of config_safe_media_volume_index property @@ -6992,7 +6993,7 @@ public class AudioService extends IAudioService.Stub } private void setSafeMediaVolumeEnabled(boolean on, String caller) { - synchronized (mSafeMediaVolumeState) { + synchronized (mSafeMediaVolumeStateLock) { if ((mSafeMediaVolumeState != SAFE_MEDIA_VOLUME_NOT_CONFIGURED) && (mSafeMediaVolumeState != SAFE_MEDIA_VOLUME_DISABLED)) { if (on && (mSafeMediaVolumeState == SAFE_MEDIA_VOLUME_INACTIVE)) { @@ -7040,7 +7041,7 @@ public class AudioService extends IAudioService.Stub } private boolean checkSafeMediaVolume(int streamType, int index, int device) { - synchronized (mSafeMediaVolumeState) { + synchronized (mSafeMediaVolumeStateLock) { if ((mSafeMediaVolumeState == SAFE_MEDIA_VOLUME_ACTIVE) && (mStreamVolumeAlias[streamType] == AudioSystem.STREAM_MUSIC) && ((device & mSafeMediaVolumeDevices) != 0) && @@ -7054,7 +7055,7 @@ public class AudioService extends IAudioService.Stub @Override public void disableSafeMediaVolume(String callingPackage) { enforceVolumeController("disable the safe media volume"); - synchronized (mSafeMediaVolumeState) { + synchronized (mSafeMediaVolumeStateLock) { setSafeMediaVolumeEnabled(false, callingPackage); if (mPendingVolumeCommand != null) { onSetStreamVolume(mPendingVolumeCommand.mStreamType, @@ -7330,7 +7331,7 @@ public class AudioService extends IAudioService.Stub mVolumeLogger.dump(pw); } - private static String safeMediaVolumeStateToString(Integer state) { + private static String safeMediaVolumeStateToString(int state) { switch(state) { case SAFE_MEDIA_VOLUME_NOT_CONFIGURED: return "SAFE_MEDIA_VOLUME_NOT_CONFIGURED"; case SAFE_MEDIA_VOLUME_DISABLED: return "SAFE_MEDIA_VOLUME_DISABLED";