From: Pavankumar Kondeti Date: Tue, 28 May 2013 06:40:24 +0000 (+0530) Subject: USB: android: Fix a NULL pointer dereference X-Git-Url: https://git.stricted.de/?a=commitdiff_plain;h=73f8fab54c41cf5e042ff7073781bcf176b18c19;p=GitHub%2Fmt8127%2Fandroid_kernel_alcatel_ttab.git 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 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 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 --- diff --git a/drivers/usb/gadget/android.c b/drivers/usb/gadget/android.c index 01accb207c7d..95d460c39de2 100644 --- 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)