[PATCH 2.6] fix use-after-free in ds.c

From: Daniel Ritz (daniel.ritz_at_gmx.ch)
Date: 12/28/03

  • Next message: Daniel Ritz: "[PATCH 2.6] fix yenta memleak"
    To: Russell King <rmk@arm.linux.org.uk>, "linux-pcmcia" <linux-pcmcia@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>
    Date:	Sun, 28 Dec 2003 18:52:01 +0100
    
    

    fixes a use-after-free in ds.c seen when doing:

      # cardctl eject
      # rmmod yenta_socket
      # killall cardmgr

    this is russell king's patch from october plus removes some unused vars.
    (original see: http://lists.infradead.org/pipermail/linux-pcmcia/2003-October/000303.html )

    against 2.6.0, please enqueue.

    --- 1.39/drivers/pcmcia/ds.c Sat Sep 27 11:11:05 2003
    +++ edited/drivers/pcmcia/ds.c Sun Dec 28 17:30:54 2003
    @@ -51,6 +51,8 @@
     #include <linux/list.h>
     #include <linux/workqueue.h>
     
    +#include <asm/atomic.h>
    +
     #include <pcmcia/version.h>
     #include <pcmcia/cs_types.h>
     #include <pcmcia/cs.h>
    @@ -95,10 +97,12 @@
         int event_head, event_tail;
         event_t event[MAX_EVENTS];
         struct user_info_t *next;
    + struct pcmcia_bus_socket *socket;
     } user_info_t;
     
     /* Socket state information */
     struct pcmcia_bus_socket {
    + atomic_t refcount;
             client_handle_t handle;
             int state;
             user_info_t *user;
    @@ -113,6 +117,7 @@
     #define SOCKET_PRESENT 0x01
     #define SOCKET_BUSY 0x02
     #define SOCKET_REMOVAL_PENDING 0x10
    +#define SOCKET_DEAD 0x80
     
     /*====================================================================*/
     
    @@ -137,6 +142,24 @@
     static struct pcmcia_driver * get_pcmcia_driver (dev_info_t *dev_info);
     static struct pcmcia_bus_socket * get_socket_info_by_nr(unsigned int nr);
     
    +static void pcmcia_put_bus_socket(struct pcmcia_bus_socket *s)
    +{
    + if (atomic_dec_and_test(&s->refcount))
    + kfree(s);
    +}
    +
    +static struct pcmcia_bus_socket *pcmcia_get_bus_socket(int nr)
    +{
    + struct pcmcia_bus_socket *s;
    +
    + s = get_socket_info_by_nr(nr);
    + if (s) {
    + WARN_ON(atomic_read(&s->refcount) == 0);
    + atomic_inc(&s->refcount);
    + }
    + return s;
    +}
    +
     /**
      * pcmcia_register_driver - register a PCMCIA driver with the bus core
      *
    @@ -501,7 +524,7 @@
     
         DEBUG(0, "ds_open(socket %d)\n", i);
     
    - s = get_socket_info_by_nr(i);
    + s = pcmcia_get_bus_socket(i);
         if (!s)
                 return -ENODEV;
     
    @@ -517,6 +540,7 @@
         user->event_tail = user->event_head = 0;
         user->next = s->user;
         user->user_magic = USER_MAGIC;
    + user->socket = s;
         s->user = user;
         file->private_data = user;
         
    @@ -529,20 +553,17 @@
     
     static int ds_release(struct inode *inode, struct file *file)
     {
    - socket_t i = iminor(inode);
         struct pcmcia_bus_socket *s;
         user_info_t *user, **link;
     
         DEBUG(0, "ds_release(socket %d)\n", i);
     
    - s = get_socket_info_by_nr(i);
    - if (!s)
    - return 0;
    -
         user = file->private_data;
         if (CHECK_USER(user))
             goto out;
     
    + s = user->socket;
    +
         /* Unlink user data structure */
         if ((file->f_flags & O_ACCMODE) != O_RDONLY)
             s->state &= ~SOCKET_BUSY;
    @@ -554,6 +575,7 @@
         *link = user->next;
         user->user_magic = 0;
         kfree(user);
    + pcmcia_put_bus_socket(s);
     out:
         return 0;
     } /* ds_release */
    @@ -563,7 +585,6 @@
     static ssize_t ds_read(struct file *file, char *buf,
                            size_t count, loff_t *ppos)
     {
    - socket_t i = iminor(file->f_dentry->d_inode);
         struct pcmcia_bus_socket *s;
         user_info_t *user;
     
    @@ -572,14 +593,14 @@
         if (count < 4)
             return -EINVAL;
     
    - s = get_socket_info_by_nr(i);
    - if (!s)
    - return -ENODEV;
    -
         user = file->private_data;
         if (CHECK_USER(user))
             return -EIO;
         
    + s = user->socket;
    + if (s->state & SOCKET_DEAD)
    + return -EIO;
    +
         if (queue_empty(user)) {
             interruptible_sleep_on(&s->queue);
             if (signal_pending(current))
    @@ -594,7 +615,6 @@
     static ssize_t ds_write(struct file *file, const char *buf,
                             size_t count, loff_t *ppos)
     {
    - socket_t i = iminor(file->f_dentry->d_inode);
         struct pcmcia_bus_socket *s;
         user_info_t *user;
     
    @@ -605,14 +625,14 @@
         if ((file->f_flags & O_ACCMODE) == O_RDONLY)
             return -EBADF;
     
    - s = get_socket_info_by_nr(i);
    - if (!s)
    - return -ENODEV;
    -
         user = file->private_data;
         if (CHECK_USER(user))
             return -EIO;
     
    + s = user->socket;
    + if (s->state & SOCKET_DEAD)
    + return -EIO;
    +
         if (s->req_pending) {
             s->req_pending--;
             get_user(s->req_result, (int *)buf);
    @@ -629,19 +649,19 @@
     /* No kernel lock - fine */
     static u_int ds_poll(struct file *file, poll_table *wait)
     {
    - socket_t i = iminor(file->f_dentry->d_inode);
         struct pcmcia_bus_socket *s;
         user_info_t *user;
     
         DEBUG(2, "ds_poll(socket %d)\n", i);
         
    - s = get_socket_info_by_nr(i);
    - if (!s)
    - return POLLERR;
    -
         user = file->private_data;
         if (CHECK_USER(user))
             return POLLERR;
    + s = user->socket;
    + /*
    + * We don't check for a dead socket here since that
    + * will send cardmgr into an endless spin.
    + */
         poll_wait(file, &s->queue, wait);
         if (!queue_empty(user))
             return POLLIN | POLLRDNORM;
    @@ -653,17 +673,21 @@
     static int ds_ioctl(struct inode * inode, struct file * file,
                         u_int cmd, u_long arg)
     {
    - socket_t i = iminor(inode);
         struct pcmcia_bus_socket *s;
         u_int size;
         int ret, err;
         ds_ioctl_arg_t buf;
    + user_info_t *user;
     
         DEBUG(2, "ds_ioctl(socket %d, %#x, %#lx)\n", i, cmd, arg);
         
    - s = get_socket_info_by_nr(i);
    - if (!s)
    - return -ENODEV;
    + user = file->private_data;
    + if (CHECK_USER(user))
    + return -EIO;
    +
    + s = user->socket;
    + if (s->state & SOCKET_DEAD)
    + return -EIO;
         
         size = (cmd & IOCSIZE_MASK) >> IOCSIZE_SHIFT;
         if (size > sizeof(ds_ioctl_arg_t)) return -EINVAL;
    @@ -833,6 +857,7 @@
             if(!s)
                     return -ENOMEM;
             memset(s, 0, sizeof(struct pcmcia_bus_socket));
    + atomic_set(&s->refcount, 1);
         
             /*
              * Ugly. But we want to wait for the socket threads to have started up.
    @@ -894,7 +919,8 @@
     
             pcmcia_deregister_client(socket->pcmcia->handle);
     
    - kfree(socket->pcmcia);
    + socket->pcmcia->state |= SOCKET_DEAD;
    + pcmcia_put_bus_socket(socket->pcmcia);
             socket->pcmcia = NULL;
     
             return;

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/


  • Next message: Daniel Ritz: "[PATCH 2.6] fix yenta memleak"

    Relevant Pages

    • [PATCH 1/3, RFC] misc char dev BKL pushdown
      ... return ret; ... static int coreb_open(struct inode *inode, struct file *file) ...
      (Linux-Kernel)
    • [2.6.6-mm3] sysfs-leaves-bin.patch
      ... opening the sysfs file in openand a few changes in variable names. ... static int open(struct inode * inode, ... static int release(struct inode * inode, struct file * file) ...
      (Linux-Kernel)
    • [patch 3/4] Refactor do_syslog interface
      ... This patch breaks out the read operations in do_sysloginto their ... static int kmsg_open(struct inode * inode, ... static int kmsg_release(struct inode * inode, struct file * file) ...
      (Linux-Kernel)
    • [PATCH 2/3, RFC] watchdog dev BKL pushdown
      ... static int acq_open(struct inode *inode, struct file *file) ... if (nowayout) ...
      (Linux-Kernel)
    • [RFC 5/6] sysfs backing store ver 0.5
      ... opening the sysfs file in openand a few changes in variable names. ... static int open(struct inode * inode, ... static int release(struct inode * inode, struct file * file) ...
      (Linux-Kernel)