USB: android: Fix a NULL pointer dereference

Fix a possible NULL pointer dereference of dev pointer in
android_disable() called from adb_closed_callback().  It happens
when adb device file is opened/closed and adb is not in the
current composition.

CRs-Fixed: 492464
Change-Id: I80f646658ce56c6883eb79773705e01967f9234e
Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org>
usb: gadget: Fix a race condition in dynamic composition switching

This fix solves a race condition that causes a crash when switching
composition with USB cable disconnected.
The following race condition is fixed by protecting data->opened
with a mutex:

1. enable_store(0) is called.
2. While in enable_store(), adb_closed_callback() is invoked and
data->opened is set to false.
3. The execution is blocked on a mutex in closed_callback().
4. From adb_android_function_disable(), android_enable is invoked,
where we didn't call android_disable() just yet.

CRs-fixed: 482312
Change-Id: I7e37850a594e2e1c6d2cabbdb39760b00827afbd
Signed-off-by: Amit Blay <ablay@codeaurora.org>
USB: android: Fail ffs_ready (i.e. start adbd) if ADB not enabled

F_FS function notifies android composite driver of userspace client's
open and close (start/stop adbd) using android.c's ready and closed
callbacks. Typically userspace starts adbd only in ADB composition as
functionfs_bind from ready_callback happens only if ADB is enabled.
Current design has couple of issues:
1) If adbd is started before enabling ADB then USB composition gets
enabled without functionfs_bind resulting a crash in ffs_func->set_alt.
2) Additionally, even if userspace script for composition switch performs
"stop adbd" before enabling new composition, there is a possibility that
closed_callback runs in parallel with ffs_enable/disable as adb daemon
is stopped asynchronously. Even though these functions use android_dev
mutex but closed callback may not use this mutex if ready_callback
gets called before ADB/FFS is enabled, resulting in different crashes.
This is possible mainly due to above 1st issue or during quick
composition switches from ADB to non-ADB to ADB and by the time adb got
started, userspace enabled non-ADB composition.

To fix both of the above issues only option is to fail ffs_ready
callback (start adbd) if ADB is not enabled. This restriction clearly
avoid 1st issue, and for 2nd: closed callback would always use mutex
to avoid any potential synchronization issues. While we are at this,
also fix config->opened getting cleared twice from disabled_callback.

Change-Id: I9fe80d09b9eefaa87e396ff451a71026b798175b
Signed-off-by: Manu Gautam <mgautam@codeaurora.org>

diff --git a/drivers/usb/gadget/android.c b/drivers/usb/gadget/android.c
index 01accb207c7dfc2d0f52847981a5ea6e6d8c9251..95d460c39de2ee17ef868658f133aab4df53bf02 100644 (file)
--- a/drivers/usb/gadget/android.c
+++ b/drivers/usb/gadget/android.c
@@ -302,6 +302,7 @@ struct functionfs_config {
        bool opened;
        bool enabled;
        struct ffs_data *data;
+       struct android_dev *dev;
 };
 
 static int ffs_function_init(struct android_usb_function *f,
@@ -409,21 +410,30 @@ static int functionfs_ready_callback(struct ffs_data *ffs)
        struct functionfs_config *config = ffs_function.config;
        int ret = 0;
 
-       mutex_lock(&dev->mutex);
-
-       ret = functionfs_bind(ffs, dev->cdev);
-       if (ret)
-               goto err;
+       /* dev is null in case ADB is not in the composition */
+       if (dev) {
+               mutex_lock(&dev->mutex);
+               ret = functionfs_bind(ffs, dev->cdev);
+               if (ret) {
+                       mutex_unlock(&dev->mutex);
+                       return ret;
+               }
+       } else {
+               /* android ffs_func requires daemon to start only after enable*/
+               pr_debug("start adbd only in ADB composition\n");
+               return -ENODEV;
+       }
 
        config->data = ffs;
        config->opened = true;
+       /* Save dev in case the adb function will get disabled */
+       config->dev = dev;
 
        if (config->enabled)
                android_enable(dev);
 
-err:
        mutex_unlock(&dev->mutex);
-       return ret;
+       return 0;
 }
 
 static void functionfs_closed_callback(struct ffs_data *ffs)
@@ -431,17 +441,32 @@ static void functionfs_closed_callback(struct ffs_data *ffs)
        struct android_dev *dev = _android_dev;
        struct functionfs_config *config = ffs_function.config;
 
-       mutex_lock(&dev->mutex);
+       /*
+        * In case new composition is without ADB or ADB got disabled by the
+        * time ffs_daemon was stopped then use saved one
+        */
+       if (!dev)
+               dev = config->dev;
 
-       if (config->enabled)
+       /* fatal-error: It should never happen */
+       if (!dev)
+               pr_err("adb_closed_callback: config->dev is NULL");
+
+       if (dev)
+               mutex_lock(&dev->mutex);
+
+       if (config->enabled && dev)
                android_disable(dev);
 
+       config->dev = NULL;
+
        config->opened = false;
        config->data = NULL;
 
        functionfs_unbind(ffs);
 
-       mutex_unlock(&dev->mutex);
+       if (dev)
+               mutex_unlock(&dev->mutex);
 }
 
 static void *functionfs_acquire_dev_callback(const char *dev_name)