Commit 0346ef1a authored Apr 01, 2015 by Jack Pham Committed by Matt Wagantall May 30, 2015

usb: android: Fix race condition with ADB compositions



There is a possible race condition that can occur when the
android gadget driver is in the middle of handling userspace
setting or switching of compositions.

Consider two processes: one is manipulating the android_usb
sysfs "enable" entry; the other is an FFS client, namely the
ADB daemon getting killed.

Thread 1                                Thread 2
--------                                --------
enable_store(0)                         functionfs_closed_callback()
mutex_lock                              if (config->enabled) { // TRUE
[loop over all enabled functions]           mutex_lock <-- blocks
...
ffs_function_disable()
    config->enabled = false
    if (!config->opened) // FALSE
        android_enable() // does not execute
...
mutex_unlock                                --> unblocks
                                            android_disable()
                                            mutex_unlock
                                        }
                                        config->opened = false

In this scenario, thread 2 sees config->enabled==TRUE and blocks
even though thread 1 is about to set it FALSE. This results in
an extra unbalanced call to android_disable() which will forever
keep the composition from getting activated. A similar condition
could occur if thread 2 executes all the way until just before
setting config->opened to FALSE, while thread 1 races through to
read that value as still TRUE and skips a necessary call to
android_enable().

Fix these races by protecting the reading/writing of
config->opened and config->enabled with the same mutex in both
functionfs_ready_callback() and functionfs_closed_callback().

Change-Id: I847210881f67b9326e87ad6b019c3100a232572d
Signed-off-by: Jack Pham <jackp@codeaurora.org>

parent be1affb9

drivers/usb/gadget/android.c

+12 −6

Original line number	Diff line number	Diff line
		@@ -634,14 +634,17 @@ static int functionfs_ready_callback(struct ffs_data *ffs)
		struct android_dev *dev = ffs_function.android_dev;
		struct functionfs_config *config = ffs_function.config;

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

		config->data = ffs;
		config->opened = true;

		if (config->enabled) {
		mutex_lock(&dev->mutex);
		if (config->enabled && dev)
		android_enable(dev);

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

		return 0;
		}
		@@ -651,15 +654,18 @@ static void functionfs_closed_callback(struct ffs_data *ffs)
		struct android_dev *dev = ffs_function.android_dev;
		struct functionfs_config *config = ffs_function.config;

		if (config->enabled) {
		if (dev)
		mutex_lock(&dev->mutex);

		if (config->enabled && dev)
		android_disable(dev);
		mutex_unlock(&dev->mutex);
		}

		config->opened = false;
		config->data = NULL;

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

		usb_put_function(config->func);
		}