[PATCH] char: xillybus: Prevent use-after-free due to race condition

[Eli Billauer]

xillybus_find_inode() is called by xillybus_open() and xillyusb_open()
to translate the inode's major and minor into a pointer to a relevant
data structure and an index.

But with xillyusb_open(), the data structure can be freed by
xillyusb_disconnect() during an unintentional time gap between the
release of the mutex that is taken by xillybus_find_inode() and the
mutex that is subsequently taken by xillyusb_open().

To fix this, xillybus_find_inode() supplies the pointer to the mutex that
it has locked (when returning success), so xillyusb_open() releases this
mutex only after obtaining the mutex that is specific to a device file.
This ensures that xillyusb_disconnect() won't release anything that is in
use.

This manipulation of mutexes poses no risk for a deadlock, because in
all usage scenarios, @unit_mutex (which is locked by xillybus_find_inode)
is always taken when no other mutex is locked. Hence a consistent locking
order is guaranteed.

xillybus_open() unlocks this mutex immediately, as this driver doesn't
support hot unplugging anyhow.

Reported-by: Hyunwoo Kim <imv4bel@xxxxxxxxx>
Signed-off-by: Eli Billauer <eli.billauer@xxxxxxxxx>
---
drivers/char/xillybus/xillybus_class.c | 8 +++++---
drivers/char/xillybus/xillybus_class.h | 2 ++
drivers/char/xillybus/xillybus_core.c  | 6 +++++-
drivers/char/xillybus/xillyusb.c       | 4 +++-
4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/char/xillybus/xillybus_class.c b/drivers/char/xillybus/xillybus_class.c
index 0f238648dcfe..c846dc3ed225 100644
--- a/drivers/char/xillybus/xillybus_class.c
+++ b/drivers/char/xillybus/xillybus_class.c
@@ -211,6 +211,7 @@ void xillybus_cleanup_chrdev(void *private_data,
EXPORT_SYMBOL(xillybus_cleanup_chrdev);

int xillybus_find_inode(struct inode *inode,
+			struct mutex **to_unlock,
			void **private_data, int *index)
{
	int minor = iminor(inode);
@@ -227,13 +228,14 @@ int xillybus_find_inode(struct inode *inode,
			break;
		}

-	mutex_unlock(&unit_mutex);
-
-	if (!unit)
+	if (!unit) {
+		mutex_unlock(&unit_mutex);
		return -ENODEV;
+	}

	*private_data = unit->private_data;
	*index = minor - unit->lowest_minor;
+	*to_unlock = &unit_mutex;

	return 0;
}
diff --git a/drivers/char/xillybus/xillybus_class.h b/drivers/char/xillybus/xillybus_class.h
index 5dbfdfc95c65..7cf89ac8bb32 100644
--- a/drivers/char/xillybus/xillybus_class.h
+++ b/drivers/char/xillybus/xillybus_class.h
@@ -12,6 +12,7 @@
#include <linux/device.h>
#include <linux/fs.h>
#include <linux/module.h>
+#include <linux/mutex.h>

int xillybus_init_chrdev(struct device *dev,
			 const struct file_operations *fops,
@@ -25,6 +26,7 @@ void xillybus_cleanup_chrdev(void *private_data,
			     struct device *dev);

int xillybus_find_inode(struct inode *inode,
+			struct mutex **to_unlock, /* Mutex to release */
			void **private_data, int *index);

#endif /* __XILLYBUS_CLASS_H */
diff --git a/drivers/char/xillybus/xillybus_core.c b/drivers/char/xillybus/xillybus_core.c
index 11b7c4749274..7fd35f055310 100644
--- a/drivers/char/xillybus/xillybus_core.c
+++ b/drivers/char/xillybus/xillybus_core.c
@@ -1431,11 +1431,15 @@ static int xillybus_open(struct inode *inode, struct file *filp)
	struct xilly_endpoint *endpoint;
	struct xilly_channel *channel;
	int index;
+	struct mutex *to_unlock; /* Mutex locked by xillybus_find_inode() */

-	rc = xillybus_find_inode(inode, (void **)&endpoint, &index);
+	rc = xillybus_find_inode(inode, &to_unlock,
+				 (void **)&endpoint, &index);
	if (rc)
		return rc;

+	mutex_unlock(to_unlock);
+
	if (endpoint->fatal_error)
		return -EIO;

diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index 39bcbfd908b4..63e94d935067 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -1236,8 +1236,9 @@ static int xillyusb_open(struct inode *inode, struct file *filp)
	struct xillyusb_endpoint *out_ep = NULL;
	int rc;
	int index;
+	struct mutex *to_unlock; /* Mutex locked by xillybus_find_inode() */

-	rc = xillybus_find_inode(inode, (void **)&xdev, &index);
+	rc = xillybus_find_inode(inode, &to_unlock, (void **)&xdev, &index);
	if (rc)
		return rc;

@@ -1245,6 +1246,7 @@ static int xillyusb_open(struct inode *inode, struct file *filp)
	filp->private_data = chan;

	mutex_lock(&chan->lock);
+	mutex_unlock(to_unlock);

	rc = -ENODEV;

--
2.17.1