Fix misc race conditions and use of stale pointer

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@3234 42af7a65-404d-4744-a932-0658087f49c3

2 files changed, 363 insertions, 142 deletions

diff --git a/nuttx/arch/arm/src/lpc17xx/lpc17_usbhost.c b/nuttx/arch/arm/src/lpc17xx/lpc17_usbhost.c
index 49719673c..b9882c34c 100755
--- a/nuttx/arch/arm/src/lpc17xx/lpc17_usbhost.c
+++ b/nuttx/arch/arm/src/lpc17xx/lpc17_usbhost.c
@@ -158,8 +158,10 @@ struct lpc17_usbhost_s
 
   volatile uint8_t tdstatus;  /* TD control status bits from last Writeback Done Head event */
   volatile bool    connected; /* Connected to device */
-  sem_t            rhssem;    /* Semaphore  to wait Root Hub Status change */
-  sem_t            wdhsem;    /* Semaphore  used to wait for Writeback Done Head event */
+  volatile bool    rhswait;   /* TRUE: Thread is waiting for Root Hub Status change */
+  volatile bool    wdhwait;   /* TRUE: Thread is waiting for WDH interrupt */
+  sem_t            rhssem;    /* Semaphore to wait Writeback Done Head event */
+  sem_t            wdhsem;    /* Semaphore used to wait for Writeback Done Head event */
 };
 
 /* The following are used to manage lists of free EDs and TD buffers*/
@@ -603,6 +605,38 @@ static void lpc17_enqueuetd(volatile struct ohci_ed_s *ed, uint32_t dirpid,
 }
 
 /*******************************************************************************
+ * Name: lpc17_wdhwait
+ *
+ * Description:
+ *   Set the request for the Writeback Done Head event well BEFORE enabling the
+ *   transfer (as soon as we are absolutely committed to the to avoid transfer).
+ *   We do this to minimize race conditions.  This logic would have to be expanded
+ *   if we want to have more than one packet in flight at a time!
+ *
+ *******************************************************************************/
+
+static int lpc17_wdhwait(struct lpc17_usbhost_s *priv)
+{
+  irqstate_t flags = irqsave();
+  int        ret   = -ENODEV;
+
+  /* Is the device still connected? */
+
+  if (priv->connected)
+    {
+      /* Yes.. then set wdhwait to indicate that we expect to be informed when
+       * either (1) the device is disconnected, or (2) the transfer completed.
+       */
+
+      priv->wdhwait = true;
+      ret           = OK;
+    }
+
+  irqrestore(flags);
+  return ret;
+}
+
+/*******************************************************************************
  * Name: lpc17_ctrltd
  *
  * Description:
@@ -621,6 +655,20 @@ static int lpc17_ctrltd(struct lpc17_usbhost_s *priv, uint32_t dirpid,
 {
   uint32_t toggle;
   uint32_t regval;
+  int ret;
+
+  /* Set the request for the Writeback Done Head event well BEFORE enabling the
+   * transfer.
+   */
+
+  ret = lpc17_wdhwait(priv);
+  if (ret != OK)
+    {
+      udbg("ERROR: Device disconnected\n");
+      return ret;
+    }
+
+  /* Configure the toggle field in the TD */
 
   if (dirpid == GTD_STATUS_DP_SETUP)
     {
@@ -633,7 +681,7 @@ static int lpc17_ctrltd(struct lpc17_usbhost_s *priv, uint32_t dirpid,
 
   /* Then enqueue the transfer */
 
-  priv->tdstatus = 0;
+  priv->tdstatus = TD_CC_NOERROR;
   lpc17_enqueuetd(EDCTRL, dirpid, toggle, buffer, buflen);
 
   /* Set the head of the control list to the EP0 EDCTRL (this would have to
@@ -666,7 +714,7 @@ static int lpc17_ctrltd(struct lpc17_usbhost_s *priv, uint32_t dirpid,
 
   /* Check the TDHEAD completion status bits */
 
-  if (priv->tdstatus == 0)
+  if (priv->tdstatus == TD_CC_NOERROR)
     {
       return OK;
     }
@@ -733,12 +781,16 @@ static int lpc17_usbinterrupt(int irq, FAR void *context)
                           /* Yes.. connected. */
 
                           ullvdbg("Connected\n");
-                          priv->tdstatus = 0;
+                          priv->tdstatus  = TD_CC_NOERROR;
+                          priv->connected = true;
 
                           /* Notify any waiters */
 
-                          priv->connected = true;
-                          lpc17_givesem(&priv->rhssem);
+                          if (priv->rhswait)
+                            {
+                              lpc17_givesem(&priv->rhssem);
+                              priv->rhswait = false;
+                            }
                         }
                       else
                         {
@@ -762,11 +814,29 @@ static int lpc17_usbinterrupt(int irq, FAR void *context)
                           /* Yes.. Disconnect the class */
 
                           CLASS_DISCONNECTED(priv->class);
+                          priv->class = NULL;
                         }
 
-                      /* Notify any waiters */
+                      /* Notify any waiters for the Root Hub Status change event */
 
-                      lpc17_givesem(&priv->rhssem);
+                      if (priv->rhswait)
+                        {
+                          lpc17_givesem(&priv->rhssem);
+                          priv->rhswait = false;
+                        }
+
+                      /* Wake up the thread waiting for the WDH event. In my
+                       * experience, the WHD event will occur later after the
+                       * disconnection, but it is safer to make sure that
+                       * there are no waiters.
+                       */
+
+                      if (priv->wdhwait)
+                        {
+                          priv->tdstatus = TD_CC_DEVNOTRESPONDING;
+                          lpc17_givesem(&priv->wdhsem);
+                          priv->wdhwait  = false;
+                        }
                     }
                   else
                     {
@@ -802,7 +872,7 @@ static int lpc17_usbinterrupt(int irq, FAR void *context)
 
           /* Since there is only one TD, we can disable further TD processing. */
 
-          regval = lpc17_getreg(LPC17_USBHOST_CTRL);
+          regval  = lpc17_getreg(LPC17_USBHOST_CTRL);
           regval &= ~(OHCI_CTRL_PLE|OHCI_CTRL_IE|OHCI_CTRL_CLE|OHCI_CTRL_BLE);
           lpc17_putreg(regval, LPC17_USBHOST_CTRL);
 
@@ -811,7 +881,7 @@ static int lpc17_usbinterrupt(int irq, FAR void *context)
           priv->tdstatus = (TDHEAD->ctrl & GTD_STATUS_CC_MASK) >> GTD_STATUS_CC_SHIFT;
 
 #ifdef CONFIG_DEBUG_USB
-          if (priv->tdstatus != 0)
+          if (priv->tdstatus != TD_CC_NOERROR)
             {
               /* The transfer failed for some reason... dump some diagnostic info. */
 
@@ -823,8 +893,11 @@ static int lpc17_usbinterrupt(int irq, FAR void *context)
 
           /* And wake up the thread waiting for the WDH event */
 
-          DEBUGASSERT(priv->wdhsem.semcount <= 0);
-          lpc17_givesem(&priv->wdhsem);
+          if (priv->wdhwait)
+            {
+              lpc17_givesem(&priv->wdhsem);
+              priv->wdhwait = false;
+            }
         }
 
 #ifdef CONFIG_DEBUG_USB
@@ -873,15 +946,19 @@ static int lpc17_usbinterrupt(int irq, FAR void *context)
 static int lpc17_wait(FAR struct usbhost_driver_s *drvr, bool connected)
 {
   struct lpc17_usbhost_s *priv = (struct lpc17_usbhost_s *)drvr;
+  irqstate_t flags;
 
   /* Are we already connected? */
 
+  flags = irqsave();
   while (priv->connected == connected)
     {
       /* No... wait for the connection/disconnection */
 
+      priv->rhswait = true;
       lpc17_takesem(&priv->rhssem);
     }
+  irqrestore(flags);
 
   udbg("Connected:%s\n", priv->connected ? "YES" : "NO");
   return OK;
@@ -1202,7 +1279,7 @@ static int lpc17_transfer(FAR struct usbhost_driver_s *drvr,
 #ifdef CONFIG_UBHOST_AHBIOBUFFERS
   uint8_t *origbuf = NULL;
 #endif
-  int ret = -ENOMEM;
+  int ret;
 
   DEBUGASSERT(drvr && ep && buffer && buflen > 0);
   uvdbg("EP%d %s maxpacket:%d buflen:%d\n",
@@ -1220,6 +1297,7 @@ static int lpc17_transfer(FAR struct usbhost_driver_s *drvr,
         {
           uvdbg("buflen (%d) > IO buffer size (%d)\n",
                  buflen, CONFIG_USBHOST_IOBUFSIZE);
+          ret = -ENOMEM;
           goto errout;
         }
 
@@ -1230,6 +1308,7 @@ static int lpc17_transfer(FAR struct usbhost_driver_s *drvr,
       if (!buffer)
         {
           uvdbg("IO buffer allocation failed\n");
+          ret = -ENOMEM;
           goto errout;
         }
 
@@ -1246,12 +1325,24 @@ static int lpc17_transfer(FAR struct usbhost_driver_s *drvr,
     }
 #endif
 
+  /* Set the request for the Writeback Done Head event well BEFORE enabling the
+   * transfer.
+   */
+
+  ret = lpc17_wdhwait(priv);
+  if (ret != OK)
+    {
+      udbg("ERROR: Device disconnected\n");
+      goto errout;
+    }
+
   /* Allocate an ED */
 
   ed = lpc17_edalloc(priv);
   if (!ed)
     {
       udbg("ED allocation failed\n");
+      ret = -ENOMEM;
       goto errout;
     }
 
@@ -1277,7 +1368,7 @@ static int lpc17_transfer(FAR struct usbhost_driver_s *drvr,
 
   /* Then enqueue the transfer */
 
-  priv->tdstatus = 0;
+  priv->tdstatus = TD_CC_NOERROR;
   lpc17_enqueuetd(ed, dirpid, GTD_STATUS_T_TOGGLE, buffer, buflen);
 
   /* Set the head of the bulk list to the EP descriptor (this would have to
@@ -1310,7 +1401,7 @@ static int lpc17_transfer(FAR struct usbhost_driver_s *drvr,
 
   /* Check the TDHEAD completion status bits */
 
-  if (priv->tdstatus == 0)
+  if (priv->tdstatus == TD_CC_NOERROR)
     {
       ret = OK;
     }
@@ -1332,7 +1423,7 @@ errout:
        * way around this copy.
        */
 
-      if (ep->in != 0)
+      if (ep->in != 0 && ret == OK)
         {
           memcpy(origbuf, buffer, buflen);
         }
diff --git a/nuttx/drivers/usbhost/usbhost_storage.c b/nuttx/drivers/usbhost/usbhost_storage.c
index 680e21569..7aee9d2ea 100644
--- a/nuttx/drivers/usbhost/usbhost_storage.c
+++ b/nuttx/drivers/usbhost/usbhost_storage.c
@@ -120,16 +120,17 @@ struct usbhost_state_s
 
   /* The remainder of the fields are provide o the mass storage class */
   
-  char                    sdchar;     /* Character identifying the /dev/sd[n] device */
-  int16_t                 crefs;      /* Reference count on the driver instance */
-  uint16_t                blocksize;  /* Block size of USB mass storage device */
-  uint32_t                nblocks;    /* Number of blocks on the USB mass storage device */
-  sem_t                   exclsem;    /* Used to maintain mutual exclusive access */
-  struct work_s           work;       /* For interacting with the worker thread */
-  FAR uint8_t            *tdbuffer;   /* The allocated transfer descriptor buffer */
-  size_t                  tdbuflen;   /* Size of the allocated transfer buffer */
-  struct usbhost_epdesc_s bulkin;     /* Bulk IN endpoint */
-  struct usbhost_epdesc_s bulkout;    /* Bulk OUT endpoint */
+  char                    sdchar;       /* Character identifying the /dev/sd[n] device */
+  volatile bool           disconnected; /* TRUE: Device has been disconnected */
+  int16_t                 crefs;        /* Reference count on the driver instance */
+  uint16_t                blocksize;    /* Block size of USB mass storage device */
+  uint32_t                nblocks;      /* Number of blocks on the USB mass storage device */
+  sem_t                   exclsem;      /* Used to maintain mutual exclusive access */
+  struct work_s           work;         /* For interacting with the worker thread */
+  FAR uint8_t            *tdbuffer;     /* The allocated transfer descriptor buffer */
+  size_t                  tdbuflen;     /* Size of the allocated transfer buffer */
+  struct usbhost_epdesc_s bulkin;       /* Bulk IN endpoint */
+  struct usbhost_epdesc_s bulkout;      /* Bulk OUT endpoint */
 };
 
 /****************************************************************************
@@ -143,8 +144,8 @@ static void usbhost_takesem(sem_t *sem);
 
 /* Memory allocation services */
 
-static inline struct usbhost_state_s *usbhost_allocclass(void);
-static inline void usbhost_freeclass(struct usbhost_state_s *class);
+static inline FAR struct usbhost_state_s *usbhost_allocclass(void);
+static inline void usbhost_freeclass(FAR struct usbhost_state_s *class);
 
 /* Device name management */
 
@@ -342,7 +343,7 @@ static void usbhost_takesem(sem_t *sem)
  ****************************************************************************/
 
 #if CONFIG_USBHOST_NPREALLOC > 0
-static inline struct usbhost_state_s *usbhost_allocclass(void)
+static inline FAR struct usbhost_state_s *usbhost_allocclass(void)
 {
   struct usbhost_state_s *priv;
   irqstate_t flags;
@@ -359,17 +360,22 @@ static inline struct usbhost_state_s *usbhost_allocclass(void)
       priv->class.flink = NULL;
     }
   irqrestore(flags);
+  ullvdbg("Allocated: %p\n", priv);;
   return priv;
 }
 #else
-static inline struct usbhost_state_s *usbhost_allocclass(void)
+static inline FAR struct usbhost_state_s *usbhost_allocclass(void)
 {
+  FAR struct usbhost_state_s *priv;
+
   /* We are not executing from an interrupt handler so we can just call
    * malloc() to get memory for the class instance.
    */
 
   DEBUGASSERT(!up_interrupt_context());
-  return (struct usbhost_state_s *)malloc(sizeof(struct usbhost_state_s));
+  priv = (FAR struct usbhost_state_s *)malloc(sizeof(struct usbhost_state_s));
+  uvdbg("Allocated: %p\n", priv);;
+  return priv;
 }
 #endif
 
@@ -388,11 +394,13 @@ static inline struct usbhost_state_s *usbhost_allocclass(void)
  ****************************************************************************/
 
 #if CONFIG_USBHOST_NPREALLOC > 0
-static inline void usbhost_freeclass(struct usbhost_state_s *class)
+static inline void usbhost_freeclass(FAR struct usbhost_state_s *class)
 {
   irqstate_t flags;
   DEBUGASSERT(class != NULL);
 
+  ullvdbg("Freeing: %p\n", class);;
+
   /* Just put the pre-allocated class structure back on the freelist */
 
   flags = irqsave();
@@ -401,7 +409,7 @@ static inline void usbhost_freeclass(struct usbhost_state_s *class)
   irqrestore(flags);  
 }
 #else
-static inline void usbhost_freeclass(struct usbhost_state_s *class)
+static inline void usbhost_freeclass(FAR struct usbhost_state_s *class)
 {
   DEBUGASSERT(class != NULL);
 
@@ -409,6 +417,7 @@ static inline void usbhost_freeclass(struct usbhost_state_s *class)
    * from an interrupt handler.
    */
 
+  uvdbg("Freeing: %p\n", class);;
   free(class);
 }
 #endif
@@ -651,44 +660,63 @@ static inline int usbhost_maxlunreq(FAR struct usbhost_state_s *priv)
   FAR struct usb_ctrlreq_s *req = (FAR struct usb_ctrlreq_s *)priv->tdbuffer;
   DEBUGASSERT(priv && priv->tdbuffer);
 
-  /* Request maximum logical unit number.  NOTE: On an IN transaction, The
-   * req and buffer pointers passed to DRVR_CTRLIN may refer to the same
-   * allocated memory.
-   */
+  /* Make sure that the device is still connected. */
 
-  uvdbg("Request maximum logical unit number\n");
-  memset(req, 0, sizeof(struct usb_ctrlreq_s));
-  req->type    = USB_DIR_IN|USB_REQ_TYPE_CLASS|USB_REQ_RECIPIENT_INTERFACE;
-  req->req     = USBSTRG_REQ_GETMAXLUN;
-  usbhost_putle16(req->len, 1);
-  return DRVR_CTRLIN(priv->drvr, req, priv->tdbuffer);
+  if (!priv->disconnected)
+    {
+      /* Request maximum logical unit number.  NOTE: On an IN transaction, The
+       * req and buffer pointers passed to DRVR_CTRLIN may refer to the same
+       * allocated memory.
+       */
+
+      uvdbg("Request maximum logical unit number\n");
+      memset(req, 0, sizeof(struct usb_ctrlreq_s));
+      req->type    = USB_DIR_IN|USB_REQ_TYPE_CLASS|USB_REQ_RECIPIENT_INTERFACE;
+      req->req     = USBSTRG_REQ_GETMAXLUN;
+      usbhost_putle16(req->len, 1);
+      return DRVR_CTRLIN(priv->drvr, req, priv->tdbuffer);
+    }
+
+  udbg("ERROR: No longer connected\n");
+  return -ENODEV;
 }
 
 static inline int usbhost_testunitready(FAR struct usbhost_state_s *priv)
 {
   FAR struct usbstrg_cbw_s *cbw;
-  int result = -ENOMEM;
+  int result;
+
+  /* Make sure that the device is still connected */
+
+  if (priv->disconnected)
+    {
+      udbg("ERROR: No longer connected\n");
+      return -ENODEV;
+    }
 
   /* Initialize a CBW (re-using the allocated transfer buffer) */
  
   cbw = usbhost_cbwalloc(priv);
-  if (cbw)
+  if (!cbw)
     {
-      /* Construct and send the CBW */
+      udbg("ERROR: Failed to create CBW\n");
+      return -ENOMEM;
+    }
+  
+  /* Construct and send the CBW */
  
-      usbhost_testunitreadycbw(cbw);
-      result = DRVR_TRANSFER(priv->drvr, &priv->bulkout,
-                            (uint8_t*)cbw, USBSTRG_CBW_SIZEOF);
+  usbhost_testunitreadycbw(cbw);
+  result = DRVR_TRANSFER(priv->drvr, &priv->bulkout,
+                        (uint8_t*)cbw, USBSTRG_CBW_SIZEOF);
+  if (result == OK)
+    {
+      /* Receive the CSW */
+
+      result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
+                             priv->tdbuffer, USBSTRG_CSW_SIZEOF);
       if (result == OK)
         {
-          /* Receive the CSW */
-
-          result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
-                                 priv->tdbuffer, USBSTRG_CSW_SIZEOF);
-          if (result == OK)
-            {
-              usbhost_dumpcsw((FAR struct usbstrg_csw_s *)priv->tdbuffer);
-            }
+          usbhost_dumpcsw((FAR struct usbstrg_csw_s *)priv->tdbuffer);
         }
     }
   return result;
@@ -697,37 +725,49 @@ static inline int usbhost_testunitready(FAR struct usbhost_state_s *priv)
 static inline int usbhost_requestsense(FAR struct usbhost_state_s *priv)
 {
   FAR struct usbstrg_cbw_s *cbw;
-  int result = -ENOMEM;
+  int result;
+
+  /* Make sure that the device is still connected */
+
+  if (priv->disconnected)
+    {
+      udbg("ERROR: No longer connected\n");
+      return -ENODEV;
+    }
 
   /* Initialize a CBW (re-using the allocated transfer buffer) */
  
   cbw = usbhost_cbwalloc(priv);
-  if (cbw)
+  if (!cbw)
     {
-      /* Construct and send the CBW */
+      udbg("ERROR: Failed to create CBW\n");
+      return -ENOMEM;
+    }
+
+  /* Construct and send the CBW */
  
-      usbhost_requestsensecbw(cbw);
-      result = DRVR_TRANSFER(priv->drvr, &priv->bulkout,
-                            (uint8_t*)cbw, USBSTRG_CBW_SIZEOF);
+  usbhost_requestsensecbw(cbw);
+  result = DRVR_TRANSFER(priv->drvr, &priv->bulkout,
+                        (uint8_t*)cbw, USBSTRG_CBW_SIZEOF);
+  if (result == OK)
+    {
+      /* Receive the sense data response */
+
+      result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
+                             priv->tdbuffer, SCSIRESP_FIXEDSENSEDATA_SIZEOF);
       if (result == OK)
         {
-          /* Receive the sense data response */
+          /* Receive the CSW */
 
           result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
-                                 priv->tdbuffer, SCSIRESP_FIXEDSENSEDATA_SIZEOF);
+                                 priv->tdbuffer, USBSTRG_CSW_SIZEOF);
           if (result == OK)
             {
-              /* Receive the CSW */
-
-              result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
-                                     priv->tdbuffer, USBSTRG_CSW_SIZEOF);
-              if (result == OK)
-                {
-                  usbhost_dumpcsw((FAR struct usbstrg_csw_s *)priv->tdbuffer);
-                }
+              usbhost_dumpcsw((FAR struct usbstrg_csw_s *)priv->tdbuffer);
             }
         }
     }
+
   return result;
 }
 
@@ -735,43 +775,55 @@ static inline int usbhost_readcapacity(FAR struct usbhost_state_s *priv)
 {
   FAR struct usbstrg_cbw_s *cbw;
   FAR struct scsiresp_readcapacity10_s *resp;
-  int result = -ENOMEM;
+  int result;
+
+  /* Make sure that the device is still connected */
+
+  if (priv->disconnected)
+    {
+      udbg("ERROR: No longer connected\n");
+      return -ENODEV;
+    }
 
   /* Initialize a CBW (re-using the allocated transfer buffer) */
  
   cbw = usbhost_cbwalloc(priv);
-  if (cbw)
+  if (!cbw)
     {
-      /* Construct and send the CBW */
+      udbg("ERROR: Failed to create CBW\n");
+      return -ENOMEM;
+    }
+
+  /* Construct and send the CBW */
  
-      usbhost_readcapacitycbw(cbw);
-      result = DRVR_TRANSFER(priv->drvr, &priv->bulkout,
-                             (uint8_t*)cbw, USBSTRG_CBW_SIZEOF);
+  usbhost_readcapacitycbw(cbw);
+  result = DRVR_TRANSFER(priv->drvr, &priv->bulkout,
+                        (uint8_t*)cbw, USBSTRG_CBW_SIZEOF);
+  if (result == OK)
+    {
+      /* Receive the read capacity CBW IN response */
+
+      result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
+                             priv->tdbuffer, SCSIRESP_READCAPACITY10_SIZEOF);
       if (result == OK)
         {
-          /* Receive the read capacity CBW IN response */
+          /* Save the capacity information */
+
+          resp            = (FAR struct scsiresp_readcapacity10_s *)priv->tdbuffer;
+          priv->nblocks   = usbhost_getbe32(resp->lba);
+          priv->blocksize = usbhost_getbe32(resp->blklen);
+
+          /* Receive the CSW */
 
           result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
-                                 priv->tdbuffer, SCSIRESP_READCAPACITY10_SIZEOF);
+                                 priv->tdbuffer, USBSTRG_CSW_SIZEOF);
           if (result == OK)
             {
-              /* Save the capacity information */
-
-              resp = (FAR struct scsiresp_readcapacity10_s *)priv->tdbuffer;
-              priv->nblocks = usbhost_getbe32(resp->lba);
-              priv->blocksize = usbhost_getbe32(resp->blklen);
-
-              /* Receive the CSW */
-
-              result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
-                                     priv->tdbuffer, USBSTRG_CSW_SIZEOF);
-              if (result == OK)
-                {
-                  usbhost_dumpcsw((FAR struct usbstrg_csw_s *)priv->tdbuffer);
-                }
+              usbhost_dumpcsw((FAR struct usbstrg_csw_s *)priv->tdbuffer);
             }
         }
     }
+
   return result;
 }
 
@@ -779,41 +831,53 @@ static inline int usbhost_inquiry(FAR struct usbhost_state_s *priv)
 {
   FAR struct usbstrg_cbw_s *cbw;
   FAR struct scsiresp_inquiry_s *resp;
-  int result = -ENOMEM;
+  int result;
+
+  /* Make sure that the device is still connected */
+
+  if (priv->disconnected)
+    {
+      udbg("ERROR: No longer connected\n");
+      return -ENODEV;
+    }
 
   /* Initialize a CBW (re-using the allocated transfer buffer) */
  
   cbw = usbhost_cbwalloc(priv);
-  if (cbw)
+  if (!cbw)
     {
-      /* Construct and send the CBW */
+      udbg("ERROR: Failed to create CBW\n");
+      return -ENOMEM;
+    }
+
+  /* Construct and send the CBW */
  
-      usbhost_inquirycbw(cbw);
-      result = DRVR_TRANSFER(priv->drvr, &priv->bulkout,
-                             (uint8_t*)cbw, USBSTRG_CBW_SIZEOF);
+  usbhost_inquirycbw(cbw);
+  result = DRVR_TRANSFER(priv->drvr, &priv->bulkout,
+                         (uint8_t*)cbw, USBSTRG_CBW_SIZEOF);
+  if (result == OK)
+    {
+      /* Receive the CBW IN response */
+
+      result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
+                             priv->tdbuffer, SCSIRESP_INQUIRY_SIZEOF);
       if (result == OK)
         {
-          /* Receive the CBW IN response */
+          /* TODO: If USB debug is enabled, dump the response data here */
+
+          resp = (FAR struct scsiresp_inquiry_s *)priv->tdbuffer;
+
+          /* Receive the CSW */
 
           result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
-                                 priv->tdbuffer, SCSIRESP_INQUIRY_SIZEOF);
+                                 priv->tdbuffer, USBSTRG_CSW_SIZEOF);
           if (result == OK)
             {
-              /* TODO: If USB debug is enabled, dump the response data here */
-
-              resp = (FAR struct scsiresp_inquiry_s *)priv->tdbuffer;
-
-              /* Receive the CSW */
-
-              result = DRVR_TRANSFER(priv->drvr, &priv->bulkin,
-                                     priv->tdbuffer, USBSTRG_CSW_SIZEOF);
-              if (result == OK)
-                {
-                  usbhost_dumpcsw((FAR struct usbstrg_csw_s *)priv->tdbuffer);
-                }
+              usbhost_dumpcsw((FAR struct usbstrg_csw_s *)priv->tdbuffer);
             }
         }
     }
+
   return result;
 }
 
@@ -839,6 +903,7 @@ static void usbhost_destroy(FAR void *arg)
   char devname[DEV_NAMELEN];
 
   DEBUGASSERT(priv != NULL);
+  uvdbg("crefs: %d\n", priv->crefs);
  
   /* Unregister the block driver */
 
@@ -857,8 +922,14 @@ static void usbhost_destroy(FAR void *arg)
 
   sem_destroy(&priv->exclsem);
 
+  /* Disconnect the USB host device */
+
+  DRVR_DISCONNECT(priv->drvr);
+
   /* And free the class instance.  Hmmm.. this may execute on the worker
-   * thread and the work structure is part of what is getting freed.
+   * thread and the work structure is part of what is getting freed.  That
+   * should be okay because once the work contained is removed from the
+   * queue, it should not longer be accessed by the worker thread.
    */
 
   usbhost_freeclass(priv);
@@ -898,10 +969,17 @@ static void usbhost_initvolume(FAR void *arg)
   result = usbhost_tdalloc(priv);
   if (result != OK)
     {
-      udbg("Failed to allocate transfer buffer\n");
+      udbg("ERROR: Failed to allocate transfer buffer\n");
       return;
     }
 
+  /* Increment the reference count.  This will prevent usbhost_destroy() from
+   * being called asynchronously if the device is removed.
+   */
+
+  priv->crefs++;
+  DEBUGASSERT(priv->crefs == 2);
+
   /* Request the maximum logical unit number */
 
   uvdbg("Get max LUN\n");
@@ -959,7 +1037,7 @@ static void usbhost_initvolume(FAR void *arg)
           csw = (FAR struct usbstrg_csw_s *)priv->tdbuffer;
           if (csw->status != 0)
             {
-              udbg("CSW status error: %d\n", csw->status);
+              udbg("ERROR: CSW status error: %d\n", csw->status);
               result = -ENODEV;
             }
         }
@@ -980,7 +1058,7 @@ static void usbhost_initvolume(FAR void *arg)
           csw = (FAR struct usbstrg_csw_s *)priv->tdbuffer;
           if (csw->status != 0)
             {
-              udbg("CSW status error: %d\n", csw->status);
+              udbg("ERROR: CSW status error: %d\n", csw->status);
               result = -ENODEV;
             }
         }
@@ -997,18 +1075,45 @@ static void usbhost_initvolume(FAR void *arg)
       result = register_blockdriver(devname, &g_bops, 0, priv);
     }
 
-  /* Check if we successfully initialized */
+  /* Check if we successfully initialized. We now have to be concerned
+   * about asynchronous modification of crefs because the block
+   * driver has been registerd.
+   */
 
   if (result == OK)
     {
-      /* Ready for normal operation as a block device driver */
+      usbhost_takesem(&priv->exclsem);
+      DEBUGASSERT(priv->crefs >= 2);
+
+      /* Handle a corner case where (1) open() has been called so the
+       * reference count is > 2, but the device has been disconnected.
+       * In this case, the class instance needs to persist until close()
+       * is called.
+       */
 
-      uvdbg("Successfully initialized\n");
+      if (priv->crefs <= 2 && priv->disconnected)
+        {
+          /* We don't have to give the semaphore because it will be
+           * destroyed when usb_destroy is called.
+           */
+  
+          result = -ENODEV;
+        }
+      else
+        {
+          /* Ready for normal operation as a block device driver */
+
+          uvdbg("Successfully initialized\n");
+          priv->crefs--;
+          usbhost_givesem(&priv->exclsem);
+        }
     }
-  else
+
+  /* Disconnect on any errors detected during volume initialization */
+
+  if (result != OK)
     {
       udbg("ERROR! Aborting: %d\n", result);
-      DRVR_DISCONNECT(priv->drvr);
       usbhost_destroy(priv);
     }
 }
@@ -1040,6 +1145,8 @@ static void usbhost_work(FAR struct usbhost_state_s *priv, worker_t worker)
        * prevent us from over-running the work structure.
        */
 
+      uvdbg("Queuing work: worker %p->%p\n", priv->work.worker, worker);
+      DEBUGASSERT(priv->work.worker == NULL);
       (void)work_queue(&priv->work, worker, priv, 0);
     }
   else
@@ -1258,12 +1365,16 @@ static inline int usbhost_tdalloc(FAR struct usbhost_state_s *priv)
 
 static inline int usbhost_tdfree(FAR struct usbhost_state_s *priv)
 {
-  int result;
-  DEBUGASSERT(priv && priv->tdbuffer != NULL);
+  int result = OK;
+  DEBUGASSERT(priv);
 
-  result         = DRVR_FREE(priv->drvr, priv->tdbuffer);
-  priv->tdbuffer = NULL;
-  priv->tdbuflen = 0;
+  if (priv->tdbuffer)
+    {
+      DEBUGASSERT(priv->drvr);
+      result         = DRVR_FREE(priv->drvr, priv->tdbuffer);
+      priv->tdbuffer = NULL;
+      priv->tdbuflen = 0;
+    }
   return result;
 }
 
@@ -1594,8 +1705,8 @@ static int usbhost_disconnected(struct usbhost_class_s *class)
    * of the mass storage device that the device is no longer available.
    */
 
-  flags = irqsave();
-  priv->drvr = NULL;
+  flags              = irqsave();
+  priv->disconnected = true;
 
   /* Now check the number of references on the class instance.  If it is one,
    * then we can free the class instance now.  Otherwise, we will have to
@@ -1603,6 +1714,7 @@ static int usbhost_disconnected(struct usbhost_class_s *class)
    * block driver.
    */
 
+  ullvdbg("crefs: %d\n", priv->crefs);
   if (priv->crefs == 1)
     {
       /* Destroy the class instance */
@@ -1626,15 +1738,25 @@ static int usbhost_disconnected(struct usbhost_class_s *class)
 static int usbhost_open(FAR struct inode *inode)
 {
   FAR struct usbhost_state_s *priv;
+  irqstate_t flags;
   int ret;
 
   uvdbg("Entry\n");
   DEBUGASSERT(inode && inode->i_private);
   priv = (FAR struct usbhost_state_s *)inode->i_private;
 
-  /* Check if the mass storage device is still connected */
+  /* Make sure that we have exclusive access to the private data structure */
+
+  DEBUGASSERT(priv->crefs > 0 && priv->crefs < USBHOST_MAX_CREFS);
+  usbhost_takesem(&priv->exclsem);
+
+  /* Check if the mass storage device is still connected.  We need to disable
+   * interrupts momentarily to assure that there are no asynchronous disconnect
+   * events.
+   */
 
-  if (!priv->drvr)
+  flags = irqsave();
+  if (priv->disconnected)
     {
       /* No... the block driver is no longer bound to the class.  That means that
        * the USB storage device is no longer connected.  Refuse any further
@@ -1647,13 +1769,12 @@ static int usbhost_open(FAR struct inode *inode)
     {
       /* Otherwise, just increment the reference count on the driver */
 
-      DEBUGASSERT(priv->crefs > 0 && priv->crefs < USBHOST_MAX_CREFS);
-      usbhost_takesem(&priv->exclsem);
       priv->crefs++;
-      usbhost_givesem(&priv->exclsem);
       ret = OK;
     }
+  irqrestore(flags);
 
+  usbhost_givesem(&priv->exclsem);
   return ret;
 }
 
@@ -1667,6 +1788,7 @@ static int usbhost_open(FAR struct inode *inode)
 static int usbhost_close(FAR struct inode *inode)
 {
   FAR struct usbhost_state_s *priv;
+  irqstate_t flags;
 
   uvdbg("Entry\n");
   DEBUGASSERT(inode && inode->i_private);
@@ -1685,19 +1807,27 @@ static int usbhost_close(FAR struct inode *inode)
 
   usbhost_givesem(&priv->exclsem);
 
+  /* We need to disable interrupts momentarily to assure that there are
+   * no asynchronous disconnect events.
+   */
+
+  flags = irqsave();
+
   /* Check if the USB mass storage device is still connected.  If the
    * storage device is not connected and the reference count just
    * decremented to one, then unregister the block driver and free
    * the class instance.
    */
 
-  if (priv->crefs <= 1 && priv->drvr == NULL)
+  if (priv->crefs <= 1 && priv->disconnected)
     {
       /* Destroy the class instance */
 
       DEBUGASSERT(priv->crefs == 1);
       usbhost_destroy(priv);
     }
+
+  irqrestore(flags);
   return OK;
 }
 
@@ -1724,7 +1854,7 @@ static ssize_t usbhost_read(FAR struct inode *inode, unsigned char *buffer,
 
   /* Check if the mass storage device is still connected */
 
-  if (!priv->drvr)
+  if (priv->disconnected)
     {
       /* No... the block driver is no longer bound to the class.  That means that
        * the USB storage device is no longer connected.  Refuse any attempt to read
@@ -1816,7 +1946,7 @@ static ssize_t usbhost_write(FAR struct inode *inode, const unsigned char *buffe
 
   /* Check if the mass storage device is still connected */
 
-  if (!priv->drvr)
+  if (priv->disconnected)
     {
       /* No... the block driver is no longer bound to the class.  That means that
        * the USB storage device is no longer connected.  Refuse any attempt to
@@ -1904,7 +2034,7 @@ static int usbhost_geometry(FAR struct inode *inode, struct geometry *geometry)
   /* Check if the mass storage device is still connected */
 
   priv = (FAR struct usbhost_state_s *)inode->i_private;
-  if (!priv->drvr)
+  if (priv->disconnected)
     {
       /* No... the block driver is no longer bound to the class.  That means that
        * the USB storage device is no longer connected.  Refuse to return any
@@ -1957,7 +2087,7 @@ static int usbhost_ioctl(FAR struct inode *inode, int cmd, unsigned long arg)
 
   /* Check if the mass storage device is still connected */
 
-  if (!priv->drvr)
+  if (priv->disconnected)
     {
       /* No... the block driver is no longer bound to the class.  That means that
        * the USB storage device is no longer connected.  Refuse to process any