commit 8604a738a79d9caf8b57773a7bcc2ef99a8a624d
author Mikhail Naganov <mnaganov@google.com> Mon Apr 24 09:29:22 2017 -0700
committer Mikhail Naganov <mnaganov@google.com> Thu May 04 18:30:31 2017 +0000
tree 9fcbea247868c429257672179849cf9d1c93f4cd
parent a22bc7450b8a114843a27c36c511910bf4472914

audiohal: Fix incorrect parameter handling in IDevicesFactory::openDevice

Check for possibly invalid values of IDevicesFactory::Device enum.

Bug: 37592306
Change-Id: I4a2300d3be9ab38e6c51ba8a3b5bbbb606a11c2b
Test: VTS
(cherry picked from commit 8c16d85122472ef01a9e023c9568968faac1aff8)

audio/2.0/default/DevicesFactory.cpp

diff --git a/audio/2.0/default/DevicesFactory.cpp b/audio/2.0/default/DevicesFactory.cpp
index 8825107..b913bc7 100644
--- a/audio/2.0/default/DevicesFactory.cpp
+++ b/audio/2.0/default/DevicesFactory.cpp
@@ -39,6 +39,7 @@
         case IDevicesFactory::Device::R_SUBMIX: return AUDIO_HARDWARE_MODULE_ID_REMOTE_SUBMIX;
         case IDevicesFactory::Device::STUB: return AUDIO_HARDWARE_MODULE_ID_STUB;
     }
+    return nullptr;
 }
 
 // static
@@ -75,19 +76,22 @@
 // Methods from ::android::hardware::audio::V2_0::IDevicesFactory follow.
 Return<void> DevicesFactory::openDevice(IDevicesFactory::Device device, openDevice_cb _hidl_cb)  {
     audio_hw_device_t *halDevice;
-    int halStatus = loadAudioInterface(deviceToString(device), &halDevice);
-    Result retval(Result::OK);
+    Result retval(Result::INVALID_ARGUMENTS);
     sp<IDevice> result;
-    if (halStatus == OK) {
-        if (device == IDevicesFactory::Device::PRIMARY) {
-            result = new PrimaryDevice(halDevice);
-        } else {
-            result = new ::android::hardware::audio::V2_0::implementation::Device(halDevice);
+    const char* moduleName = deviceToString(device);
+    if (moduleName != nullptr) {
+        int halStatus = loadAudioInterface(moduleName, &halDevice);
+        if (halStatus == OK) {
+            if (device == IDevicesFactory::Device::PRIMARY) {
+                result = new PrimaryDevice(halDevice);
+            } else {
+                result = new ::android::hardware::audio::V2_0::implementation::
+                    Device(halDevice);
+            }
+            retval = Result::OK;
+        } else if (halStatus == -EINVAL) {
+            retval = Result::NOT_INITIALIZED;
         }
-    } else if (halStatus == -EINVAL) {
-        retval = Result::NOT_INITIALIZED;
-    } else {
-        retval = Result::INVALID_ARGUMENTS;
     }
     _hidl_cb(retval, result);
     return Void();

audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp

diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp
index 711a8d9..074903f 100644
--- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp
+++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp
@@ -155,6 +155,16 @@
     doc::test("test the getService (called in SetUp)");
 }
 
+TEST_F(AudioHidlTest, OpenDeviceInvalidParameter) {
+    doc::test("test passing an invalid parameter to openDevice");
+    IDevicesFactory::Result result;
+    sp<IDevice> device;
+    ASSERT_OK(devicesFactory->openDevice(IDevicesFactory::Device(-1),
+                                         returnIn(result, device)));
+    ASSERT_EQ(IDevicesFactory::Result::INVALID_ARGUMENTS, result);
+    ASSERT_TRUE(device == nullptr);
+}
+
 //////////////////////////////////////////////////////////////////////////////
 /////////////////////////////// openDevice primary ///////////////////////////
 //////////////////////////////////////////////////////////////////////////////