| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 18 Sep 2022 21:08:54 -0700
From: Hyunwoo Kim <imv4bel@...il.com>
To: lkundrak@...sk
Cc: linux-kernel@...r.kernel.org, imv4bel@...il.com, arnd@...db.de,
gregkh@...uxfoundation.org, linux@...inikbrodowski.net
Subject: [PATCH v3] char: pcmcia: scr24x_cs: Fix use-after-free in scr24x_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 scr24x_open() function and
the scr24x_remove() function, which may eventually result in UAF.
So, add a mutex to the scr24x_open() and scr24x_remove() functions
to avoid race contidion of krefs.
Signed-off-by: Hyunwoo Kim <imv4bel@...il.com>
---
drivers/char/pcmcia/scr24x_cs.c | 72 +++++++++++++++++++++++----------
1 file changed, 51 insertions(+), 21 deletions(-)
diff --git a/drivers/char/pcmcia/scr24x_cs.c b/drivers/char/pcmcia/scr24x_cs.c
index 1bdce08fae3d..57fe08b3c03a 100644
--- a/drivers/char/pcmcia/scr24x_cs.c
+++ b/drivers/char/pcmcia/scr24x_cs.c
@@ -33,6 +33,7 @@
struct scr24x_dev {
struct device *dev;
+ struct pcmcia_device *p_dev;
struct cdev c_dev;
unsigned char buf[CCID_MAX_LEN];
int devno;
@@ -42,15 +43,31 @@ struct scr24x_dev {
};
#define SCR24X_DEVS 8
-static DECLARE_BITMAP(scr24x_minors, SCR24X_DEVS);
+static struct pcmcia_device *dev_table[SCR24X_DEVS];
+static DEFINE_MUTEX(remove_mutex);
static struct class *scr24x_class;
static dev_t scr24x_devt;
static void scr24x_delete(struct kref *kref)
{
- struct scr24x_dev *dev = container_of(kref, struct scr24x_dev,
- refcnt);
+ struct scr24x_dev *dev = container_of(kref, struct scr24x_dev, refcnt);
+ struct pcmcia_device *link = dev->p_dev;
+ int devno;
+
+ for (devno = 0; devno < SCR24X_DEVS; devno++) {
+ if (dev_table[devno] == link)
+ break;
+ }
+ if (devno == SCR24X_DEVS)
+ return;
+
+ device_destroy(scr24x_class, MKDEV(MAJOR(scr24x_devt), dev->devno));
+ mutex_lock(&dev->lock);
+ pcmcia_disable_device(link);
+ cdev_del(&dev->c_dev);
+ dev->dev = NULL;
+ mutex_unlock(&dev->lock);
kfree(dev);
}
@@ -73,11 +90,23 @@ static int scr24x_wait_ready(struct scr24x_dev *dev)
static int scr24x_open(struct inode *inode, struct file *filp)
{
- struct scr24x_dev *dev = container_of(inode->i_cdev,
- struct scr24x_dev, c_dev);
+ struct scr24x_dev *dev;
+ struct pcmcia_device *link;
+ int minor = iminor(inode);
+
+ if (minor >= SCR24X_DEVS)
+ return -ENODEV;
+
+ mutex_lock(&remove_mutex);
+ link = dev_table[minor];
+ if (link == NULL) {
+ mutex_unlock(&remove_mutex);
+ return -ENODEV;
+ }
kref_get(&dev->refcnt);
filp->private_data = dev;
+ mutex_unlock(&remove_mutex);
return stream_open(inode, filp);
}
@@ -232,24 +261,31 @@ static int scr24x_config_check(struct pcmcia_device *link, void *priv_data)
static int scr24x_probe(struct pcmcia_device *link)
{
struct scr24x_dev *dev;
- int ret;
+ int i, ret;
+
+ for (i = 0; i < SCR24X_DEVS; i++) {
+ if (dev_table[i] == NULL)
+ break;
+ }
+
+ if (i == SCR24X_DEVS)
+ return -ENODEV;
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
- dev->devno = find_first_zero_bit(scr24x_minors, SCR24X_DEVS);
- if (dev->devno >= SCR24X_DEVS) {
- ret = -EBUSY;
- goto err;
- }
+ dev->devno = i;
mutex_init(&dev->lock);
kref_init(&dev->refcnt);
link->priv = dev;
+ dev->p_dev = link;
link->config_flags |= CONF_ENABLE_IRQ | CONF_AUTO_SET_IO;
+ dev_table[i] = link;
+
ret = pcmcia_loop_config(link, scr24x_config_check, NULL);
if (ret < 0)
goto err;
@@ -282,8 +318,8 @@ static int scr24x_probe(struct pcmcia_device *link)
return 0;
err:
- if (dev->devno < SCR24X_DEVS)
- clear_bit(dev->devno, scr24x_minors);
+ dev_table[i] = NULL;
+
kfree (dev);
return ret;
}
@@ -292,15 +328,9 @@ static void scr24x_remove(struct pcmcia_device *link)
{
struct scr24x_dev *dev = (struct scr24x_dev *)link->priv;
- device_destroy(scr24x_class, MKDEV(MAJOR(scr24x_devt), dev->devno));
- mutex_lock(&dev->lock);
- pcmcia_disable_device(link);
- cdev_del(&dev->c_dev);
- clear_bit(dev->devno, scr24x_minors);
- dev->dev = NULL;
- mutex_unlock(&dev->lock);
-
+ mutex_lock(&remove_mutex);
kref_put(&dev->refcnt, scr24x_delete);
+ mutex_unlock(&remove_mutex);
}
static const struct pcmcia_device_id scr24x_ids[] = {
--
2.25.1
Dear,
I fixed the wrong patch referencing "dev" after kref_put() in the previous version of the patch.
Regards,
Hyunwoo Kim.
Powered by blists - more mailing lists