[PCMCIA] Add refcounting to struct pcmcia_bus_socket

If you perform the following commands in order:

 # cardctl eject
 # rmmod yenta_socket
 # insmod drivers/pcmcia/yenta_socket.ko
 # killall cardmgr

the rmmod ends up freeing the pcmcia_bus_socket while the wait
queue is still active.  The killall cardmgr cases the the select()
to complete, and users to be removed from the "queue" - which ends
up writing to freed memory.

The following patch adds refcounting to pcmcia_bus_socket so we
won't free it until all users have gone.  We also add "SOCKET_DEAD"
to mark the condition where the socket is no longer present in the
system.

Note that we don't wake up cardmgr when we remove sockets -
unfortunately cardmgr doesn't like receiving errors from read().
Really, cardmgr should treat EIO from read() as a fatal error
for that socket, and stop listening for events from it.

diff --git a/drivers/pcmcia/ds.c b/drivers/pcmcia/ds.c
index 0e99f7dba28ee3c88db55baedcb9fd2be872ebca..82a4d766acae94e1f342ba4c7625373f344270bb 100644 (file)
--- a/drivers/pcmcia/ds.c
+++ b/drivers/pcmcia/ds.c
@@ -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 @@ typedef struct user_info_t {
     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 @@ struct pcmcia_bus_socket {
 #define SOCKET_PRESENT         0x01
 #define SOCKET_BUSY            0x02
 #define SOCKET_REMOVAL_PENDING 0x10
+#define SOCKET_DEAD            0x80
 
 /*====================================================================*/
 
@@ -137,6 +142,24 @@ EXPORT_SYMBOL(cs_error);
 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 @@ static int ds_open(struct inode *inode, struct file *file)
 
     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 @@ static int ds_open(struct inode *inode, struct file *file)
     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;
     
@@ -535,14 +559,12 @@ static int ds_release(struct inode *inode, struct file *file)
 
     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 +576,7 @@ static int ds_release(struct inode *inode, struct file *file)
     *link = user->next;
     user->user_magic = 0;
     kfree(user);
+    pcmcia_put_bus_socket(s);
 out:
     return 0;
 } /* ds_release */
@@ -572,14 +595,14 @@ static ssize_t ds_read(struct file *file, char *buf,
     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))
@@ -605,14 +628,14 @@ static ssize_t ds_write(struct file *file, const char *buf,
     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);
@@ -635,13 +658,14 @@ static u_int ds_poll(struct file *file, poll_table *wait)
 
     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;
@@ -658,12 +682,17 @@ static int ds_ioctl(struct inode * inode, struct file * file,
     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 +862,7 @@ static int __devinit pcmcia_bus_add_socket(struct class_device *class_dev)
        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 +924,8 @@ static void pcmcia_bus_remove_socket(struct class_device *class_dev)
 
        pcmcia_deregister_client(socket->pcmcia->handle);
 
-       kfree(socket->pcmcia);
+       socket->pcmcia->state |= SOCKET_DEAD;
+       pcmcia_put_bus_socket(socket->pcmcia);
        socket->pcmcia = NULL;
 
        return;