[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20220916135003.GA234913@ubuntu>
Date: Fri, 16 Sep 2022 06:50:03 -0700
From: Hyunwoo Kim <imv4bel@...il.com>
To: laforge@...monks.org
Cc: linux-kernel@...r.kernel.org, imv4bel@...il.com, arnd@...db.de,
gregkh@...uxfoundation.or
Subject: [PATCH v2] char: pcmcia: cm4000_cs: Fix use-after-free in cm4000_fops
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@...il.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
Powered by blists - more mailing lists