[PCMCIA] Fix race condition causing cards to be incorrectly recognised

This patch fixes a race condition between the pcmcia socket initial
insert processing, ds.c and cardmgr.  This allowed cardmgr to believe
that a card was inserted while the card is still in the process of
resetting itself, and it therefore tried to read the CIS while it
was unavailable.

We change the meaning of SOCKET_PRESENT slightly - it now means that
a card is present _and we have completed its initialisation_.  We
introduce SOCKET_INUSE to indicate that we have a reference count
against the module.

We also take the skt_sem to prevent clients from registering while
we're handling an insert/remove/suspend/resume.

diff --git a/drivers/pcmcia/cs.c b/drivers/pcmcia/cs.c
index a012fad69666131a73e3a8d3bc06440fcccef965..2a5d0fc528c698bd8c02d45928499d58ac925537 100644 (file)
--- a/drivers/pcmcia/cs.c
+++ b/drivers/pcmcia/cs.c
@@ -474,7 +474,7 @@ static void shutdown_socket(struct pcmcia_socket *s)
     DEBUG(1, "cs: shutdown_socket(%p)\n", s);
 
     /* Blank out the socket state */
-    s->state &= SOCKET_PRESENT|SOCKET_SETUP_PENDING;
+    s->state &= SOCKET_PRESENT|SOCKET_INUSE;
     s->socket = dead_socket;
     s->ops->init(s);
     s->irq.AssignedIRQ = s->irq.Config = 0;
@@ -657,7 +657,6 @@ static int socket_setup(struct pcmcia_socket *skt, int initial_delay)
                pcmcia_error(skt, "unsupported voltage key.\n");
                return CS_BAD_TYPE;
        }
-       skt->state |= SOCKET_PRESENT;
        skt->socket.flags = SS_DEBOUNCED;
        skt->ops->set_socket(skt, &skt->socket);
 
@@ -678,11 +677,12 @@ static int socket_insert(struct pcmcia_socket *skt)
 {
        int ret;
 
-       if (!try_module_get(skt->owner))
+       if (!cs_socket_get(skt))
                return CS_NO_CARD;
 
        ret = socket_setup(skt, setup_delay);
        if (ret == CS_SUCCESS) {
+               skt->state |= SOCKET_PRESENT;
 #ifdef CONFIG_CARDBUS
                if (skt->state & SOCKET_CARDBUS) {
                        cb_alloc(skt);
@@ -693,7 +693,7 @@ static int socket_insert(struct pcmcia_socket *skt)
                skt->socket.flags &= ~SS_DEBOUNCED;
        } else {
                socket_shutdown(skt);
-               module_put(skt->owner);
+               cs_socket_put(skt);
        }
 
        return ret;
@@ -741,10 +741,8 @@ static int socket_resume(struct pcmcia_socket *skt)
                }
                skt->socket.flags &= ~SS_DEBOUNCED;
        } else {
-               unsigned int old_state = skt->state;
                socket_shutdown(skt);
-               if (old_state & SOCKET_PRESENT)
-                       module_put(skt->owner);
+               cs_socket_put(skt);
        }
 
        skt->state &= ~SOCKET_SUSPEND;
@@ -755,7 +753,7 @@ static int socket_resume(struct pcmcia_socket *skt)
 static void socket_remove(struct pcmcia_socket *skt)
 {
        socket_shutdown(skt);
-       module_put(skt->owner);
+       cs_socket_put(skt);
 }
 
 /*
@@ -1063,7 +1061,7 @@ int pcmcia_bind_mtd(mtd_bind_t *req)
 
 /*====================================================================*/
 
-int pcmcia_deregister_client(client_handle_t handle)
+int pcmcia_deregister_clientR(client_handle_t handle)
 {
     client_t **client;
     struct pcmcia_socket *s;
@@ -1346,8 +1344,6 @@ int pcmcia_get_status(client_handle_t handle, cs_status_t *status)
        status->CardState |= CS_EVENT_PM_SUSPEND;
     if (!(s->state & SOCKET_PRESENT))
        return CS_NO_CARD;
-    if (s->state & SOCKET_SETUP_PENDING)
-       status->CardState |= CS_EVENT_CARD_INSERTION;
     
     /* Get info from the PRR, if necessary */
     if (handle->Function == BIND_FN_ALL) {
@@ -1524,6 +1520,10 @@ int pcmcia_register_client(client_handle_t *handle, client_reg_t *req)
     if (client == NULL)
        return CS_OUT_OF_RESOURCE;
 
+    /*
+     * Prevent this racing with a card insertion.
+     */
+    down(&s->skt_sem);
     *handle = client;
     client->state &= ~CLIENT_UNBOUND;
     client->Socket = s;
@@ -1547,7 +1547,7 @@ int pcmcia_register_client(client_handle_t *handle, client_reg_t *req)
        s->config = kmalloc(sizeof(config_t) * s->functions,
                            GFP_KERNEL);
        if (!s->config)
-               return CS_OUT_OF_RESOURCE;
+               goto out_no_resource;
        memset(s->config, 0, sizeof(config_t) * s->functions);
     }
     
@@ -1555,14 +1555,20 @@ int pcmcia_register_client(client_handle_t *handle, client_reg_t *req)
          client, client->Socket, client->dev_info);
     if (client->EventMask & CS_EVENT_REGISTRATION_COMPLETE)
        EVENT(client, CS_EVENT_REGISTRATION_COMPLETE, CS_EVENT_PRI_LOW);
-    if ((s->state & SOCKET_PRESENT) &&
-       !(s->state & SOCKET_SETUP_PENDING)) {
+
+    if ((s->state & (SOCKET_PRESENT|SOCKET_CARDBUS)) == SOCKET_PRESENT) {
        if (client->EventMask & CS_EVENT_CARD_INSERTION)
            EVENT(client, CS_EVENT_CARD_INSERTION, CS_EVENT_PRI_LOW);
        else
            client->PendingEvents |= CS_EVENT_CARD_INSERTION;
     }
+
+    up(&s->skt_sem);
     return CS_SUCCESS;
+
+ out_no_resource:
+    up(&s->skt_sem);
+    return CS_OUT_OF_RESOURCE;
 } /* register_client */
 
 /*====================================================================*/

diff --git a/drivers/pcmcia/cs_internal.h b/drivers/pcmcia/cs_internal.h
index ae857307a979237ffc11bcd06aa215410262b20a..3bc779147092fb0038c49050f32ffbc43875abc0 100644 (file)
--- a/drivers/pcmcia/cs_internal.h
+++ b/drivers/pcmcia/cs_internal.h
@@ -99,7 +99,7 @@ struct cis_cache_entry {
 
 /* Flags in socket state */
 #define SOCKET_PRESENT         0x0008
-#define SOCKET_SETUP_PENDING   0x0010
+#define SOCKET_INUSE           0x0010
 #define SOCKET_SHUTDOWN_PENDING        0x0020
 #define SOCKET_RESET_PENDING   0x0040
 #define SOCKET_SUSPEND         0x0080
@@ -109,6 +109,26 @@ struct cis_cache_entry {
 #define SOCKET_CARDBUS         0x8000
 #define SOCKET_CARDBUS_CONFIG  0x10000
 
+static inline int cs_socket_get(struct pcmcia_socket *skt)
+{
+       int ret;
+
+       WARN_ON(skt->state & SOCKET_INUSE);
+
+       ret = try_module_get(skt->owner);
+       if (ret)
+               skt->state |= SOCKET_INUSE;
+       return ret;
+}
+
+static inline void cs_socket_put(struct pcmcia_socket *skt)
+{
+       if (skt->state & SOCKET_INUSE) {
+               skt->state &= ~SOCKET_INUSE;
+               module_put(skt->owner);
+       }
+}
+
 #define CHECK_HANDLE(h) \
     (((h) == NULL) || ((h)->client_magic != CLIENT_MAGIC))