commit c694ff449bef4ca1e45a6506f34e66c613757311
author Pavlin Radoslavov <pavlin@google.com> Mon Jan 09 23:27:29 2017 -0800
committer Pavlin Radoslavov <pavlin@google.com> Wed Jan 11 16:51:37 2017 +0000
tree 36f66657e3389dbb6b30b7e122ff415f9ce5f40c
parent 20adbc00032d6df980a3300f1ae974610f16f81f

Add missing nullptr checks for data.readCString() strings

Protect the Audio Policy Manager by adding extra checks
for data.readCString() strings when parsing / decoding
a Binder transaction.

Also:
 * Moved audio_is_output_device() / audio_is_input_device() checks
   inside handleDeviceConfigChange()
   from the AudioPolicyInterface to the AudioPolicyManager
 * Removed similar redundant checks inside
    AudioPolicyService::setDeviceConnectionState()

Test: code compilation
Change-Id: Ib32a28ba2669b73aaf32b31bb18f41c8dd7d2605

media/libaudioclient/IAudioPolicyService.cpp

diff --git a/media/libaudioclient/IAudioPolicyService.cpp b/media/libaudioclient/IAudioPolicyService.cpp
index 222189a..0ac5726 100644
--- a/media/libaudioclient/IAudioPolicyService.cpp
+++ b/media/libaudioclient/IAudioPolicyService.cpp
@@ -835,10 +835,15 @@
                     static_cast <audio_policy_dev_state_t>(data.readInt32());
             const char *device_address = data.readCString();
             const char *device_name = data.readCString();
-            reply->writeInt32(static_cast<uint32_t> (setDeviceConnectionState(device,
-                                                                              state,
-                                                                              device_address,
-                                                                              device_name)));
+            if (device_address == nullptr || device_name == nullptr) {
+                ALOGE("Bad Binder transaction: SET_DEVICE_CONNECTION_STATE for device %u", device);
+                reply->writeInt32(static_cast<int32_t> (BAD_VALUE));
+            } else {
+                reply->writeInt32(static_cast<uint32_t> (setDeviceConnectionState(device,
+                                                                                  state,
+                                                                                  device_address,
+                                                                                  device_name)));
+            }
             return NO_ERROR;
         } break;
 
@@ -847,8 +852,13 @@
             audio_devices_t device =
                     static_cast<audio_devices_t> (data.readInt32());
             const char *device_address = data.readCString();
-            reply->writeInt32(static_cast<uint32_t> (getDeviceConnectionState(device,
-                                                                              device_address)));
+            if (device_address == nullptr) {
+                ALOGE("Bad Binder transaction: GET_DEVICE_CONNECTION_STATE for device %u", device);
+                reply->writeInt32(static_cast<int32_t> (AUDIO_POLICY_DEVICE_STATE_UNAVAILABLE));
+            } else {
+                reply->writeInt32(static_cast<uint32_t> (getDeviceConnectionState(device,
+                                                                                  device_address)));
+            }
             return NO_ERROR;
         } break;
 
@@ -858,9 +868,14 @@
                     static_cast <audio_devices_t>(data.readInt32());
             const char *device_address = data.readCString();
             const char *device_name = data.readCString();
-            reply->writeInt32(static_cast<uint32_t> (handleDeviceConfigChange(device,
-                                                                              device_address,
-                                                                              device_name)));
+            if (device_address == nullptr || device_name == nullptr) {
+                ALOGE("Bad Binder transaction: HANDLE_DEVICE_CONFIG_CHANGE for device %u", device);
+                reply->writeInt32(static_cast<int32_t> (BAD_VALUE));
+            } else {
+                reply->writeInt32(static_cast<uint32_t> (handleDeviceConfigChange(device,
+                                                                                  device_address,
+                                                                                  device_name)));
+            }
             return NO_ERROR;
         } break;
 

services/audiopolicy/managerdefault/AudioPolicyManager.cpp

diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
index 8744de8..f64c4f9 100644
--- a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
+++ b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
@@ -342,6 +342,9 @@
     ALOGV("handleDeviceConfigChange(() device: 0x%X, address %s name %s",
           device, device_address, device_name);
 
+    // connect/disconnect only 1 device at a time
+    if (!audio_is_output_device(device) && !audio_is_input_device(device)) return BAD_VALUE;
+
     // Check if the device is currently connected
     sp<DeviceDescriptor> devDesc =
             mHwModules.getDeviceDescriptor(device, device_address, device_name);

services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp

diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
index b4470c0..c5323d1 100644
--- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
+++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
@@ -37,9 +37,6 @@
     if (!settingsAllowed()) {
         return PERMISSION_DENIED;
     }
-    if (!audio_is_output_device(device) && !audio_is_input_device(device)) {
-        return BAD_VALUE;
-    }
     if (state != AUDIO_POLICY_DEVICE_STATE_AVAILABLE &&
             state != AUDIO_POLICY_DEVICE_STATE_UNAVAILABLE) {
         return BAD_VALUE;
@@ -72,9 +69,6 @@
     if (!settingsAllowed()) {
         return PERMISSION_DENIED;
     }
-    if (!audio_is_output_device(device) && !audio_is_input_device(device)) {
-        return BAD_VALUE;
-    }
 
     ALOGV("handleDeviceConfigChange()");
     Mutex::Autolock _l(mLock);