From 39d2938341352ba3f5765a26dee8e2ff2a2f24bf Mon Sep 17 00:00:00 2001 From: Yan Han Date: Wed, 10 May 2023 11:57:15 +0200 Subject: [PATCH] Fix system server crash on receiving This crash occurs when enabling absolute volume behavior, when we receive with an out-of-bounds volume level. This is parsed as a negative error code, which causes HdmiControlService to attempt to construct a VolumeInfo with a negative volume. This throws an IllegalArgumentException. To fix this, we ignore messages with out-of-bounds volume levels. To help prevent future issues, we also enforce that the AudioStatus object can only represent volume levels in the [0, 100] range. Bug: 281821462 Test: atest TvToAudioSystemAvcTest PlaybackDeviceToTvAvcTest PlaybackDeviceToAudioSystemAvcTest Change-Id: I31dc0fd58da511262d829a10f7f03a4f0c99c05d --- .../hdmi/AbsoluteVolumeAudioStatusAction.java | 7 ++++++ .../com/android/server/hdmi/AudioStatus.java | 4 +++- .../hdmi/BaseAbsoluteVolumeControlTest.java | 23 +++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/hdmi/AbsoluteVolumeAudioStatusAction.java b/services/core/java/com/android/server/hdmi/AbsoluteVolumeAudioStatusAction.java index d7563e085e614..c56517e0aaa1b 100644 --- a/services/core/java/com/android/server/hdmi/AbsoluteVolumeAudioStatusAction.java +++ b/services/core/java/com/android/server/hdmi/AbsoluteVolumeAudioStatusAction.java @@ -74,6 +74,13 @@ final class AbsoluteVolumeAudioStatusAction extends HdmiCecFeatureAction { boolean mute = HdmiUtils.isAudioStatusMute(cmd); int volume = HdmiUtils.getAudioStatusVolume(cmd); + + // If the volume is out of range, report it as handled and ignore the message. + // According to the spec, such values are either reserved or indicate an unknown volume. + if (volume == Constants.UNKNOWN_VOLUME) { + return true; + } + AudioStatus audioStatus = new AudioStatus(volume, mute); if (mState == STATE_WAIT_FOR_INITIAL_AUDIO_STATUS) { localDevice().getService().enableAbsoluteVolumeControl(audioStatus); diff --git a/services/core/java/com/android/server/hdmi/AudioStatus.java b/services/core/java/com/android/server/hdmi/AudioStatus.java index a884ffb93a6dc..6242c45e82627 100644 --- a/services/core/java/com/android/server/hdmi/AudioStatus.java +++ b/services/core/java/com/android/server/hdmi/AudioStatus.java @@ -23,6 +23,8 @@ import java.util.Objects; /** * Immutable representation of the information in the [Audio Status] operand: * volume status (0 <= N <= 100) and mute status (muted or unmuted). + * The volume level is limited to the range [0, 100] upon construction. + * This object cannot represent an audio status where the volume is unknown, or out of bounds. */ public class AudioStatus { public static final int MAX_VOLUME = 100; @@ -32,7 +34,7 @@ public class AudioStatus { boolean mMute; public AudioStatus(int volume, boolean mute) { - mVolume = volume; + mVolume = Math.max(Math.min(volume, MAX_VOLUME), MIN_VOLUME); mMute = mute; } diff --git a/services/tests/servicestests/src/com/android/server/hdmi/BaseAbsoluteVolumeControlTest.java b/services/tests/servicestests/src/com/android/server/hdmi/BaseAbsoluteVolumeControlTest.java index 9f295b8f42c2f..5440e0d507dfb 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/BaseAbsoluteVolumeControlTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/BaseAbsoluteVolumeControlTest.java @@ -437,6 +437,21 @@ public abstract class BaseAbsoluteVolumeControlTest { verifyAbsoluteVolumeEnabled(); } + @Test + public void giveAudioStatusSent_reportAudioStatusVolumeOutOfBounds_avcNotEnabled() { + mAudioManager.setDeviceVolumeBehavior(getAudioOutputDevice(), + AudioManager.DEVICE_VOLUME_BEHAVIOR_FULL); + setCecVolumeControlSetting(HdmiControlManager.VOLUME_CONTROL_ENABLED); + enableSystemAudioModeIfNeeded(); + receiveSetAudioVolumeLevelSupport(DeviceFeatures.FEATURE_SUPPORTED); + + assertThat(mAudioManager.getDeviceVolumeBehavior(getAudioOutputDevice())).isEqualTo( + AudioManager.DEVICE_VOLUME_BEHAVIOR_FULL); + receiveReportAudioStatus(127, false); + assertThat(mAudioManager.getDeviceVolumeBehavior(getAudioOutputDevice())).isEqualTo( + AudioManager.DEVICE_VOLUME_BEHAVIOR_FULL); + } + @Test public void avcEnabled_cecVolumeDisabled_absoluteVolumeDisabled() { enableAbsoluteVolumeControl(); @@ -512,6 +527,14 @@ public abstract class BaseAbsoluteVolumeControlTest { eq(AudioManager.ADJUST_UNMUTE), anyInt()); clearInvocations(mAudioManager); + // Volume not within range [0, 100]: sets neither volume nor mute + receiveReportAudioStatus(127, true); + verify(mAudioManager, never()).setStreamVolume(eq(AudioManager.STREAM_MUSIC), anyInt(), + anyInt()); + verify(mAudioManager, never()).adjustStreamVolume(eq(AudioManager.STREAM_MUSIC), anyInt(), + anyInt()); + clearInvocations(mAudioManager); + // If AudioService causes us to send , the System Audio device's // volume changes. Afterward, a duplicate of an earlier should // still cause us to call setStreamVolume()