Messages in this thread | | | Date | Tue, 21 Feb 2023 07:51:46 +0100 | Subject | Re: [PATCH v3] char: pcmcia: cm4000_cs: Fix use-after-free in cm4000_fops | From | Jiri Slaby <> |
| |
Ping -- what's the status of these?
Should we mark cm4000_cs, cm4040_cs, and scr24x_cs as BROKEN instead?
Thanks.
On 19. 09. 22, 6:07, Hyunwoo Kim wrote: > 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 | 58 +++++++++++++++++++++++---------- > 1 file changed, 41 insertions(+), 17 deletions(-) > > diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c > index adaec8fd4b16..7103812b4019 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) > @@ -103,7 +104,8 @@ static int major; /* major number we get from the kernel */ > #define REG_STOPBITS(x) (x + 7) > > struct cm4000_dev { > - struct pcmcia_device *p_dev; > + struct pcmcia_device *p_dev; > + struct kref refcnt; > > unsigned char atr[MAX_ATR]; > unsigned char rbuf[512]; > @@ -146,6 +148,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 +421,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,6 +1658,7 @@ 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)) { > @@ -1673,8 +1703,12 @@ static int cmm_open(struct inode *inode, struct file *filp) > > DEBUGP(2, dev, "<- cmm_open\n"); > ret = stream_open(inode, filp); > + > + kref_get(&dev->refcnt); > out: > mutex_unlock(&cmm_mutex); > + mutex_unlock(&remove_mutex); > + > return ret; > } > > @@ -1703,6 +1737,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 +1844,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 +1861,10 @@ 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); > + mutex_unlock(&remove_mutex); > > return; > }
-- js suse labs
| |