Commit be12787d authored Jul 17, 2014 by Manu Gautam

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>

parent e0a1ca6a

drivers/usb/gadget/android.c

+11 −6

Original line number	Diff line number	Diff line
		@@ -639,6 +639,10 @@ static int functionfs_ready_callback(struct ffs_data *ffs)
		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;
		@@ -646,10 +650,9 @@ static int functionfs_ready_callback(struct ffs_data *ffs)
		/* Save dev in case the adb function will get disabled */
		config->dev = dev;

		if (config->enabled && dev)
		if (config->enabled)
		android_enable(dev);

		if (dev)
		mutex_unlock(&dev->mutex);

		return 0;
		@@ -660,18 +663,20 @@ static void functionfs_closed_callback(struct ffs_data *ffs)
		struct android_dev *dev = ffs_function.android_dev;
		struct functionfs_config *config = ffs_function.config;

		/* In case new composition is without ADB, use saved one */
		/*
		* 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;

		/* fatal-error: It should never happen */
		if (!dev)
		pr_err("adb_closed_callback: config->dev is NULL");

		if (dev)
		mutex_lock(&dev->mutex);

		config->opened = false;

		if (config->enabled && dev)
		android_disable(dev);