lkml.org 
[lkml]   [2022]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH v4] 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@gmail.com>
Reported-by: kernel test robot <lkp@intel.com>
---
drivers/char/pcmcia/scr24x_cs.c | 73 +++++++++++++++++++++++----------
1 file changed, 52 insertions(+), 21 deletions(-)

diff --git a/drivers/char/pcmcia/scr24x_cs.c b/drivers/char/pcmcia/scr24x_cs.c
index 1bdce08fae3d..039d44ee0ebe 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,24 @@ 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;
+ }

+ dev = link->priv;
kref_get(&dev->refcnt);
filp->private_data = dev;
+ mutex_unlock(&remove_mutex);

return stream_open(inode, filp);
}
@@ -232,24 +262,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 +319,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 +329,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
\
 
 \ /
  Last update: 2022-09-19 11:38    [W:0.038 / U:0.604 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site