From 263709e7be37c7040aaef385bc5c9389a9b5f514 Mon Sep 17 00:00:00 2001
From: Glenn Kasten <gkasten@google.com>
Date: Fri, 6 Jan 2012 08:40:01 -0800
Subject: [PATCH] Check stream type in AudioFlinger::createTrack

A bad parameter to AudioFlinger::createTrack could cause mediaserver to crash.

Other AudioFlinger stream type cleanup:
 - Simplify range check for audio_stream_type_t
 - Add comment about mStreamTypes array initialization.

Change-Id: Ia33aa1cce0fdd694b08d9288816ffc097a9543d0
---
 services/audioflinger/AudioFlinger.cpp       | 16 ++++++++++------
 services/audioflinger/AudioFlinger.h         |  2 +-
 services/audioflinger/AudioPolicyService.cpp |  6 +++---
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index d96624bebb..cf925b0392 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -398,7 +398,9 @@ sp<IAudioTrack> AudioFlinger::createTrack(
     status_t lStatus;
     int lSessionId;
 
-    if (streamType >= AUDIO_STREAM_CNT) {
+    // client AudioTrack::set already implements AUDIO_STREAM_DEFAULT => AUDIO_STREAM_MUSIC,
+    // but if someone uses binder directly they could bypass that and cause us to crash
+    if (uint32_t(streamType) >= AUDIO_STREAM_CNT) {
         ALOGE("createTrack() invalid stream type %d", streamType);
         lStatus = BAD_VALUE;
         goto Exit;
@@ -663,7 +665,7 @@ status_t AudioFlinger::setStreamVolume(audio_stream_type_t stream, float value,
         return PERMISSION_DENIED;
     }
 
-    if (stream < 0 || uint32_t(stream) >= AUDIO_STREAM_CNT) {
+    if (uint32_t(stream) >= AUDIO_STREAM_CNT) {
         ALOGE("setStreamVolume() invalid stream %d", stream);
         return BAD_VALUE;
     }
@@ -697,7 +699,7 @@ status_t AudioFlinger::setStreamMute(audio_stream_type_t stream, bool muted)
         return PERMISSION_DENIED;
     }
 
-    if (stream < 0 || uint32_t(stream) >= AUDIO_STREAM_CNT ||
+    if (uint32_t(stream) >= AUDIO_STREAM_CNT ||
         uint32_t(stream) == AUDIO_STREAM_ENFORCED_AUDIBLE) {
         ALOGE("setStreamMute() invalid stream %d", stream);
         return BAD_VALUE;
@@ -713,7 +715,7 @@ status_t AudioFlinger::setStreamMute(audio_stream_type_t stream, bool muted)
 
 float AudioFlinger::streamVolume(audio_stream_type_t stream, int output) const
 {
-    if (stream < 0 || uint32_t(stream) >= AUDIO_STREAM_CNT) {
+    if (uint32_t(stream) >= AUDIO_STREAM_CNT) {
         return 0.0f;
     }
 
@@ -734,7 +736,7 @@ float AudioFlinger::streamVolume(audio_stream_type_t stream, int output) const
 
 bool AudioFlinger::streamMute(audio_stream_type_t stream) const
 {
-    if (stream < 0 || stream >= (int)AUDIO_STREAM_CNT) {
+    if (uint32_t(stream) >= AUDIO_STREAM_CNT) {
         return true;
     }
 
@@ -1386,12 +1388,14 @@ AudioFlinger::PlaybackThread::PlaybackThread(const sp<AudioFlinger>& audioFlinge
     mMasterVolume = mAudioFlinger->masterVolume_l();
     mMasterMute = mAudioFlinger->masterMute_l();
 
+    // mStreamTypes[AUDIO_STREAM_CNT] is initialized by stream_type_t default constructor
     // There is no AUDIO_STREAM_MIN, and ++ operator does not compile
     for (audio_stream_type_t stream = (audio_stream_type_t) 0; stream < AUDIO_STREAM_CNT;
             stream = (audio_stream_type_t) (stream + 1)) {
         mStreamTypes[stream].volume = mAudioFlinger->streamVolumeInternal(stream);
         mStreamTypes[stream].mute = mAudioFlinger->streamMute(stream);
-        mStreamTypes[stream].valid = true;
+        // initialized by stream_type_t default constructor
+        // mStreamTypes[stream].valid = true;
     }
 }
 
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index cc7072c859..9d1d8628f3 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -799,7 +799,7 @@ private:
         status_t    dumpTracks(int fd, const Vector<String16>& args);
 
         SortedVector< sp<Track> >       mTracks;
-        // mStreamTypes[] uses 1 additionnal stream type internally for the OutputTrack used by DuplicatingThread
+        // mStreamTypes[] uses 1 additional stream type internally for the OutputTrack used by DuplicatingThread
         stream_type_t                   mStreamTypes[AUDIO_STREAM_CNT + 1];
         AudioStreamOut*                 mOutput;
         float                           mMasterVolume;
diff --git a/services/audioflinger/AudioPolicyService.cpp b/services/audioflinger/AudioPolicyService.cpp
index b06bcb9050..44311d0cd3 100644
--- a/services/audioflinger/AudioPolicyService.cpp
+++ b/services/audioflinger/AudioPolicyService.cpp
@@ -400,7 +400,7 @@ status_t AudioPolicyService::initStreamVolume(audio_stream_type_t stream,
     if (!checkPermission()) {
         return PERMISSION_DENIED;
     }
-    if (stream < 0 || stream >= AUDIO_STREAM_CNT) {
+    if (uint32_t(stream) >= AUDIO_STREAM_CNT) {
         return BAD_VALUE;
     }
     mpAudioPolicy->init_stream_volume(mpAudioPolicy, stream, indexMin, indexMax);
@@ -415,7 +415,7 @@ status_t AudioPolicyService::setStreamVolumeIndex(audio_stream_type_t stream, in
     if (!checkPermission()) {
         return PERMISSION_DENIED;
     }
-    if (stream < 0 || stream >= AUDIO_STREAM_CNT) {
+    if (uint32_t(stream) >= AUDIO_STREAM_CNT) {
         return BAD_VALUE;
     }
 
@@ -427,7 +427,7 @@ status_t AudioPolicyService::getStreamVolumeIndex(audio_stream_type_t stream, in
     if (mpAudioPolicy == NULL) {
         return NO_INIT;
     }
-    if (stream < 0 || stream >= AUDIO_STREAM_CNT) {
+    if (uint32_t(stream) >= AUDIO_STREAM_CNT) {
         return BAD_VALUE;
     }
     return mpAudioPolicy->get_stream_volume_index(mpAudioPolicy, stream, index);
-- 
2.11.0