Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 16 Sep 2022 06:50:03 -0700
From:   Hyunwoo Kim <imv4bel@gmail.com>
To:     laforge@gnumonks.org
Cc:     linux-kernel@vger.kernel.org, imv4bel@gmail.com, arnd@arndb.de,
        gregkh@linuxfoundation.or
Subject: [PATCH v2] char: pcmcia: cm4000_cs: Fix use-after-free in cm4000_fops
Message-ID: <20220916135003.GA234913@ubuntu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Precedence: bulk

A race condition may occur if the user physically removes the pcmcia
device while calling open() for this char device node.

This is a race condition between the cmm_open() function and the
cm4000_detach() function, which may eventually result in UAF.

So, add a refcount check to cm4000_detach() to free the "dev" structure
after the char device node is close()d.

Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 drivers/char/pcmcia/cm4000_cs.c | 81 +++++++++++++++++++++++----------
 1 file changed, 58 insertions(+), 23 deletions(-)

diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c
index adaec8fd4b16..9e54ad892056 100644
--- a/drivers/char/pcmcia/cm4000_cs.c
+++ b/drivers/char/pcmcia/cm4000_cs.c
@@ -55,6 +55,7 @@
 	} while (0)
 
 static DEFINE_MUTEX(cmm_mutex);
+static DEFINE_MUTEX(remove_mutex);
 
 #define	T_1SEC		(HZ)
 #define	T_10MSEC	msecs_to_jiffies(10)
@@ -104,6 +105,8 @@ static int major;		/* major number we get from the kernel */
 
 struct cm4000_dev {
 	struct pcmcia_device *p_dev;
+	struct kref		refcnt;
+	int			removed;
 
 	unsigned char atr[MAX_ATR];
 	unsigned char rbuf[512];
@@ -146,6 +149,9 @@ struct cm4000_dev {
 
 #define	ZERO_DEV(dev)	memset(&((dev)->init), 0, sizeof((dev)->init))
 
+static void stop_monitor(struct cm4000_dev *dev);
+static void cm4000_delete(struct kref *kref);
+
 static struct pcmcia_device *dev_table[CM4000_MAX_DEV];
 static struct class *cmm_class;
 
@@ -416,6 +422,30 @@ static struct card_fixup card_fixups[] = {
 	},
 };
 
+
+static void cm4000_delete(struct kref *kref)
+{
+	struct cm4000_dev *dev = container_of(kref, struct cm4000_dev, refcnt);
+	struct pcmcia_device *link = dev->p_dev;
+	int devno;
+
+	/* find device */
+	for (devno = 0; devno < CM4000_MAX_DEV; devno++)
+		if (dev_table[devno] == link)
+			break;
+	if (devno == CM4000_MAX_DEV)
+		return;
+
+	stop_monitor(dev);
+
+	cm4000_release(link);
+
+	dev_table[devno] = NULL;
+	kfree(dev);
+
+	device_destroy(cmm_class, MKDEV(major, devno));
+}
+
 static void set_cardparameter(struct cm4000_dev *dev)
 {
 	int i;
@@ -1629,21 +1659,30 @@ static int cmm_open(struct inode *inode, struct file *filp)
 	if (minor >= CM4000_MAX_DEV)
 		return -ENODEV;
 
+	mutex_lock(&remove_mutex);
 	mutex_lock(&cmm_mutex);
 	link = dev_table[minor];
 	if (link == NULL || !pcmcia_dev_present(link)) {
-		ret = -ENODEV;
-		goto out;
+		mutex_unlock(&cmm_mutex);
+		mutex_unlock(&remove_mutex);
+		return -ENODEV;
 	}
 
 	if (link->open) {
-		ret = -EBUSY;
-		goto out;
+		mutex_unlock(&cmm_mutex);
+		mutex_unlock(&remove_mutex);
+		return -EBUSY;
 	}
 
 	dev = link->priv;
 	filp->private_data = dev;
 
+	if (dev->removed == 1) {
+		mutex_unlock(&cmm_mutex);
+		mutex_unlock(&remove_mutex);
+		return -ENODEV;
+	}
+
 	DEBUGP(2, dev, "-> cmm_open(device=%d.%d process=%s,%d)\n",
 	      imajor(inode), minor, current->comm, current->pid);
 
@@ -1660,8 +1699,9 @@ static int cmm_open(struct inode *inode, struct file *filp)
 	 * inserted)
 	 */
 	if (filp->f_flags & O_NONBLOCK) {
-		ret = -EAGAIN;
-		goto out;
+		mutex_unlock(&cmm_mutex);
+		mutex_unlock(&remove_mutex);
+		return -EAGAIN;
 	}
 
 	dev->mdelay = T_50MSEC;
@@ -1673,8 +1713,12 @@ static int cmm_open(struct inode *inode, struct file *filp)
 
 	DEBUGP(2, dev, "<- cmm_open\n");
 	ret = stream_open(inode, filp);
-out:
+
+	kref_get(&dev->refcnt);
+
 	mutex_unlock(&cmm_mutex);
+	mutex_unlock(&remove_mutex);
+
 	return ret;
 }
 
@@ -1703,6 +1747,8 @@ static int cmm_close(struct inode *inode, struct file *filp)
 	link->open = 0;		/* only one open per device */
 	wake_up(&dev->devq);	/* socket removed? */
 
+	kref_put(&dev->refcnt, cm4000_delete);
+
 	DEBUGP(2, dev, "cmm_close\n");
 	return 0;
 }
@@ -1808,6 +1854,7 @@ static int cm4000_probe(struct pcmcia_device *link)
 	init_waitqueue_head(&dev->ioq);
 	init_waitqueue_head(&dev->atrq);
 	init_waitqueue_head(&dev->readq);
+	kref_init(&dev->refcnt);
 
 	ret = cm4000_config(link, i);
 	if (ret) {
@@ -1824,23 +1871,11 @@ static int cm4000_probe(struct pcmcia_device *link)
 static void cm4000_detach(struct pcmcia_device *link)
 {
 	struct cm4000_dev *dev = link->priv;
-	int devno;
 
-	/* find device */
-	for (devno = 0; devno < CM4000_MAX_DEV; devno++)
-		if (dev_table[devno] == link)
-			break;
-	if (devno == CM4000_MAX_DEV)
-		return;
-
-	stop_monitor(dev);
-
-	cm4000_release(link);
-
-	dev_table[devno] = NULL;
-	kfree(dev);
-
-	device_destroy(cmm_class, MKDEV(major, devno));
+	mutex_lock(&remove_mutex);
+	kref_put(&dev->refcnt, cm4000_delete);
+	dev->removed = 1;
+	mutex_unlock(&remove_mutex);
 
 	return;
 }
-- 
2.25.1