Messages in this thread Patch in this message | | | Date | Sun, 18 Sep 2022 21:04:57 -0700 | From | Hyunwoo Kim <> | Subject | [PATCH v3] char: pcmcia: cm4040_cs: Fix use-after-free in reader_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 cm4040_open() function and the reader_detach() function, which may eventually result in UAF.
So, add a refcount check to reader_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/cm4040_cs.c | 50 +++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 15 deletions(-)
diff --git a/drivers/char/pcmcia/cm4040_cs.c b/drivers/char/pcmcia/cm4040_cs.c index 827711911da4..bb9116d81890 100644 --- a/drivers/char/pcmcia/cm4040_cs.c +++ b/drivers/char/pcmcia/cm4040_cs.c @@ -59,6 +59,7 @@ static DEFINE_MUTEX(cm4040_mutex); /* how often to poll for fifo status change */ #define POLL_PERIOD msecs_to_jiffies(10) +static void cm4040_delete(struct kref *kref); static void reader_release(struct pcmcia_device *link); static int major; @@ -73,6 +74,7 @@ struct reader_dev { wait_queue_head_t poll_wait; wait_queue_head_t read_wait; wait_queue_head_t write_wait; + struct kref refcnt; unsigned long buffer_status; unsigned long timeout; unsigned char s_buf[READ_WRITE_BUFFER_SIZE]; @@ -102,6 +104,28 @@ static inline unsigned char xinb(unsigned short port) } #endif +static void cm4040_delete(struct kref *kref) +{ + struct reader_dev *dev = container_of(kref, struct reader_dev, refcnt); + struct pcmcia_device *link = dev->p_dev; + int devno; + + /* find device */ + for (devno = 0; devno < CM_MAX_DEV; devno++) { + if (dev_table[devno] == link) + break; + } + if (devno == CM_MAX_DEV) + return; + + reader_release(link); + + dev_table[devno] = NULL; + kfree(dev); + + device_destroy(cmx_class, MKDEV(major, devno)); +} + /* poll the device fifo status register. not to be confused with * the poll syscall. */ static void cm4040_do_poll(struct timer_list *t) @@ -442,6 +466,7 @@ static int cm4040_open(struct inode *inode, struct file *filp) return -ENODEV; mutex_lock(&cm4040_mutex); + link = dev_table[minor]; if (link == NULL || !pcmcia_dev_present(link)) { ret = -ENODEV; @@ -468,8 +493,11 @@ static int cm4040_open(struct inode *inode, struct file *filp) DEBUGP(2, dev, "<- cm4040_open (successfully)\n"); ret = nonseekable_open(inode, filp); + + kref_get(&dev->refcnt); out: mutex_unlock(&cm4040_mutex); + return ret; } @@ -495,6 +523,9 @@ static int cm4040_close(struct inode *inode, struct file *filp) wake_up(&dev->devq); DEBUGP(2, dev, "<- cm4040_close\n"); + + kref_put(&dev->refcnt, cm4040_delete); + return 0; } @@ -584,6 +615,7 @@ static int reader_probe(struct pcmcia_device *link) init_waitqueue_head(&dev->read_wait); init_waitqueue_head(&dev->write_wait); timer_setup(&dev->poll_timer, cm4040_do_poll, 0); + kref_init(&dev->refcnt); ret = reader_config(link, i); if (ret) { @@ -600,22 +632,10 @@ static int reader_probe(struct pcmcia_device *link) static void reader_detach(struct pcmcia_device *link) { struct reader_dev *dev = link->priv; - int devno; - - /* find device */ - for (devno = 0; devno < CM_MAX_DEV; devno++) { - if (dev_table[devno] == link) - break; - } - if (devno == CM_MAX_DEV) - return; - - reader_release(link); - - dev_table[devno] = NULL; - kfree(dev); - device_destroy(cmx_class, MKDEV(major, devno)); + mutex_lock(&cm4040_mutex); + kref_put(&dev->refcnt, cm4040_delete); + mutex_unlock(&cm4040_mutex); return; } -- 2.25.1 Dear,
I fixed the wrong patch referencing "dev" after kref_put() in the previous version of the patch.
Regards, Hyunwoo Kim.
| |