Commit 3b32a7e7 authored by Russell King's avatar Russell King

[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.
parent 755fa12b
...@@ -51,6 +51,8 @@ ...@@ -51,6 +51,8 @@
#include <linux/list.h> #include <linux/list.h>
#include <linux/workqueue.h> #include <linux/workqueue.h>
#include <asm/atomic.h>
#include <pcmcia/version.h> #include <pcmcia/version.h>
#include <pcmcia/cs_types.h> #include <pcmcia/cs_types.h>
#include <pcmcia/cs.h> #include <pcmcia/cs.h>
...@@ -95,10 +97,12 @@ typedef struct user_info_t { ...@@ -95,10 +97,12 @@ typedef struct user_info_t {
int event_head, event_tail; int event_head, event_tail;
event_t event[MAX_EVENTS]; event_t event[MAX_EVENTS];
struct user_info_t *next; struct user_info_t *next;
struct pcmcia_bus_socket *socket;
} user_info_t; } user_info_t;
/* Socket state information */ /* Socket state information */
struct pcmcia_bus_socket { struct pcmcia_bus_socket {
atomic_t refcount;
client_handle_t handle; client_handle_t handle;
int state; int state;
user_info_t *user; user_info_t *user;
...@@ -113,6 +117,7 @@ struct pcmcia_bus_socket { ...@@ -113,6 +117,7 @@ struct pcmcia_bus_socket {
#define SOCKET_PRESENT 0x01 #define SOCKET_PRESENT 0x01
#define SOCKET_BUSY 0x02 #define SOCKET_BUSY 0x02
#define SOCKET_REMOVAL_PENDING 0x10 #define SOCKET_REMOVAL_PENDING 0x10
#define SOCKET_DEAD 0x80
/*====================================================================*/ /*====================================================================*/
...@@ -137,6 +142,24 @@ EXPORT_SYMBOL(cs_error); ...@@ -137,6 +142,24 @@ EXPORT_SYMBOL(cs_error);
static struct pcmcia_driver * get_pcmcia_driver (dev_info_t *dev_info); 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 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 * 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) ...@@ -501,7 +524,7 @@ static int ds_open(struct inode *inode, struct file *file)
DEBUG(0, "ds_open(socket %d)\n", i); DEBUG(0, "ds_open(socket %d)\n", i);
s = get_socket_info_by_nr(i); s = pcmcia_get_bus_socket(i);
if (!s) if (!s)
return -ENODEV; return -ENODEV;
...@@ -517,6 +540,7 @@ static int ds_open(struct inode *inode, struct file *file) ...@@ -517,6 +540,7 @@ static int ds_open(struct inode *inode, struct file *file)
user->event_tail = user->event_head = 0; user->event_tail = user->event_head = 0;
user->next = s->user; user->next = s->user;
user->user_magic = USER_MAGIC; user->user_magic = USER_MAGIC;
user->socket = s;
s->user = user; s->user = user;
file->private_data = user; file->private_data = user;
...@@ -535,14 +559,12 @@ static int ds_release(struct inode *inode, struct file *file) ...@@ -535,14 +559,12 @@ static int ds_release(struct inode *inode, struct file *file)
DEBUG(0, "ds_release(socket %d)\n", i); DEBUG(0, "ds_release(socket %d)\n", i);
s = get_socket_info_by_nr(i);
if (!s)
return 0;
user = file->private_data; user = file->private_data;
if (CHECK_USER(user)) if (CHECK_USER(user))
goto out; goto out;
s = user->socket;
/* Unlink user data structure */ /* Unlink user data structure */
if ((file->f_flags & O_ACCMODE) != O_RDONLY) if ((file->f_flags & O_ACCMODE) != O_RDONLY)
s->state &= ~SOCKET_BUSY; s->state &= ~SOCKET_BUSY;
...@@ -554,6 +576,7 @@ static int ds_release(struct inode *inode, struct file *file) ...@@ -554,6 +576,7 @@ static int ds_release(struct inode *inode, struct file *file)
*link = user->next; *link = user->next;
user->user_magic = 0; user->user_magic = 0;
kfree(user); kfree(user);
pcmcia_put_bus_socket(s);
out: out:
return 0; return 0;
} /* ds_release */ } /* ds_release */
...@@ -572,14 +595,14 @@ static ssize_t ds_read(struct file *file, char *buf, ...@@ -572,14 +595,14 @@ static ssize_t ds_read(struct file *file, char *buf,
if (count < 4) if (count < 4)
return -EINVAL; return -EINVAL;
s = get_socket_info_by_nr(i);
if (!s)
return -ENODEV;
user = file->private_data; user = file->private_data;
if (CHECK_USER(user)) if (CHECK_USER(user))
return -EIO; return -EIO;
s = user->socket;
if (s->state & SOCKET_DEAD)
return -EIO;
if (queue_empty(user)) { if (queue_empty(user)) {
interruptible_sleep_on(&s->queue); interruptible_sleep_on(&s->queue);
if (signal_pending(current)) if (signal_pending(current))
...@@ -605,14 +628,14 @@ static ssize_t ds_write(struct file *file, const char *buf, ...@@ -605,14 +628,14 @@ static ssize_t ds_write(struct file *file, const char *buf,
if ((file->f_flags & O_ACCMODE) == O_RDONLY) if ((file->f_flags & O_ACCMODE) == O_RDONLY)
return -EBADF; return -EBADF;
s = get_socket_info_by_nr(i);
if (!s)
return -ENODEV;
user = file->private_data; user = file->private_data;
if (CHECK_USER(user)) if (CHECK_USER(user))
return -EIO; return -EIO;
s = user->socket;
if (s->state & SOCKET_DEAD)
return -EIO;
if (s->req_pending) { if (s->req_pending) {
s->req_pending--; s->req_pending--;
get_user(s->req_result, (int *)buf); get_user(s->req_result, (int *)buf);
...@@ -635,13 +658,14 @@ static u_int ds_poll(struct file *file, poll_table *wait) ...@@ -635,13 +658,14 @@ static u_int ds_poll(struct file *file, poll_table *wait)
DEBUG(2, "ds_poll(socket %d)\n", i); DEBUG(2, "ds_poll(socket %d)\n", i);
s = get_socket_info_by_nr(i);
if (!s)
return POLLERR;
user = file->private_data; user = file->private_data;
if (CHECK_USER(user)) if (CHECK_USER(user))
return POLLERR; 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); poll_wait(file, &s->queue, wait);
if (!queue_empty(user)) if (!queue_empty(user))
return POLLIN | POLLRDNORM; return POLLIN | POLLRDNORM;
...@@ -658,12 +682,17 @@ static int ds_ioctl(struct inode * inode, struct file * file, ...@@ -658,12 +682,17 @@ static int ds_ioctl(struct inode * inode, struct file * file,
u_int size; u_int size;
int ret, err; int ret, err;
ds_ioctl_arg_t buf; ds_ioctl_arg_t buf;
user_info_t *user;
DEBUG(2, "ds_ioctl(socket %d, %#x, %#lx)\n", i, cmd, arg); DEBUG(2, "ds_ioctl(socket %d, %#x, %#lx)\n", i, cmd, arg);
s = get_socket_info_by_nr(i); user = file->private_data;
if (!s) if (CHECK_USER(user))
return -ENODEV; return -EIO;
s = user->socket;
if (s->state & SOCKET_DEAD)
return -EIO;
size = (cmd & IOCSIZE_MASK) >> IOCSIZE_SHIFT; size = (cmd & IOCSIZE_MASK) >> IOCSIZE_SHIFT;
if (size > sizeof(ds_ioctl_arg_t)) return -EINVAL; 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) ...@@ -833,6 +862,7 @@ static int __devinit pcmcia_bus_add_socket(struct class_device *class_dev)
if(!s) if(!s)
return -ENOMEM; return -ENOMEM;
memset(s, 0, sizeof(struct pcmcia_bus_socket)); 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. * 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) ...@@ -894,7 +924,8 @@ static void pcmcia_bus_remove_socket(struct class_device *class_dev)
pcmcia_deregister_client(socket->pcmcia->handle); pcmcia_deregister_client(socket->pcmcia->handle);
kfree(socket->pcmcia); socket->pcmcia->state |= SOCKET_DEAD;
pcmcia_put_bus_socket(socket->pcmcia);
socket->pcmcia = NULL; socket->pcmcia = NULL;
return; return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment