All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eli Billauer <eli.billauer@gmail.com>
To: gregkh@linuxfoundation.org
Cc: arnd@arndb.de, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, imv4bel@gmail.com,
	Eli Billauer <eli.billauer@gmail.com>
Subject: [PATCH] char: xillybus: Prevent use-after-free due to race condition
Date: Wed, 26 Oct 2022 11:52:40 +0300	[thread overview]
Message-ID: <2e5cbdfe-f6cd-d24f-9785-55176af6c975@gmail.com> (raw)

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@gmail.com>
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 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


             reply	other threads:[~2022-10-26  8:56 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-26  8:52 Eli Billauer [this message]
2022-10-26 12:07 ` [PATCH] char: xillybus: Prevent use-after-free due to race condition Greg KH
2022-10-27  8:01   ` Eli Billauer
2022-10-26 15:02 ` Alan Stern
2022-10-27  8:05   ` Eli Billauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2e5cbdfe-f6cd-d24f-9785-55176af6c975@gmail.com \
    --to=eli.billauer@gmail.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=imv4bel@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.