Correct unitialization of composite USB device. A stale pointer was being reused. From David Sidrane

5 files changed, 67 insertions, 14 deletions

diff --git a/apps/system/composite/composite_main.c b/apps/system/composite/composite_main.c
index b34d38b99..b5a41a2fd 100644
--- a/apps/system/composite/composite_main.c
+++ b/apps/system/composite/composite_main.c
@@ -434,7 +434,6 @@ static int open_serial(void)
   message("open_serial: Successfully opened the serial driver\n");
   return OK;
 }
-#endif
 
 /****************************************************************************
  * Name: echo_serial
@@ -474,8 +473,10 @@ static int echo_serial(void)
       message("echo_serial: ERROR: read size: %d write size: %d\n",
               bytesread, byteswritten);
     }
+
   return OK;
 }
+#endif
 
 /****************************************************************************
  * Public Functions
@@ -577,6 +578,7 @@ int board_mscclassobject(FAR struct usbdevclass_driver_s **classdev)
       message("board_mscclassobject: usbmsc_classobject failed: %d\n", -ret);
       usbmsc_uninitialize(g_composite.mschandle);
     }
+
   check_test_memory_usage("After usbmsc_classobject()");
   return ret;
 }
diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog
index 0465b7d40..e92852bdf 100644
--- a/nuttx/ChangeLog
+++ b/nuttx/ChangeLog
@@ -5966,4 +5966,8 @@
 	  logic when buffers larger than the EP0 packet size are sent.
 	  Also add support for decoded USB trace strings.  From David
 	  Sidrane (2013-11-5).
-
+	* drivers/usbdev/cdcacm.c, composite.c, usbmsc.c:  uninitialization
+	  logic cause re-use of a stale pointer.  Changed to a two pass
+	  uninitialization for the case of the composite driver:  Memory
+	  resources are not freed until the second uninitialization pass.
+	  From David Sidrane (2011-1105).
diff --git a/nuttx/drivers/usbdev/cdcacm.c b/nuttx/drivers/usbdev/cdcacm.c
index 5920a6beb..9c98d199d 100644
--- a/nuttx/drivers/usbdev/cdcacm.c
+++ b/nuttx/drivers/usbdev/cdcacm.c
@@ -2398,6 +2398,22 @@ void cdcacm_uninitialize(FAR void *handle)
   char devname[CDCACM_DEVNAME_SIZE];
   int ret;
 
+#ifdef CONFIG_CDCACM_COMPOSITE
+  /* Check for pass 2 uninitialization.  We did most of the work on the
+   * first pass uninitialization.
+   */
+
+  if (priv->minor == (uint8_t)-1)
+    {
+      /* In this second and final pass, all that remains to be done is to
+       * free the memory resources.
+       */
+
+      kfree(priv);
+      return;
+    }
+#endif
+
   /* Un-register the CDC/ACM TTY device */
 
   sprintf(devname, CDCACM_DEVNAME_FORMAT, priv->minor);
@@ -2420,9 +2436,19 @@ void cdcacm_uninitialize(FAR void *handle)
 
 #ifndef CONFIG_CDCACM_COMPOSITE
   usbdev_unregister(&drvr->drvr);
-#endif
 
   /* And free the driver structure */
 
   kfree(priv);
+
+#else
+  /* For the case of the composite driver, there is a two pass
+   * uninitialization sequence.  We cannot yet free the driver structure.
+   * We will do that on the second pass.  We mark the fact that we have
+   * already unitialized by setting the minor number to -1.  If/when we
+   * are called again, then we will free the memory resources.
+   */
+
+  priv->minor = (uint8_t)-1;
+#endif
 }
diff --git a/nuttx/drivers/usbdev/composite.c b/nuttx/drivers/usbdev/composite.c
index 8a5a983b5..a88d8a7c3 100644
--- a/nuttx/drivers/usbdev/composite.c
+++ b/nuttx/drivers/usbdev/composite.c
@@ -880,20 +880,12 @@ void composite_uninitialize(FAR void *handle)
 
   DEBUGASSERT(alloc != NULL);
 
-  /* Uninitialize each of the member classes */
+  /* First phase uninitialization each of the member classes */
 
   priv = &alloc->dev;
-  if (priv->dev1)
-    {
-      DEV1_UNINITIALIZE(priv->dev1);
-      priv->dev1 = NULL;
-    }
 
-  if (priv->dev2)
-    {
-      DEV2_UNINITIALIZE(priv->dev2);
-      priv->dev2 = NULL;
-    }
+  DEV1_UNINITIALIZE(priv->dev1);
+  DEV2_UNINITIALIZE(priv->dev2);
 
   /* Then unregister and destroy the composite class */
 
@@ -902,6 +894,11 @@ void composite_uninitialize(FAR void *handle)
   /* Free any resources used by the composite driver */
   /* None */
 
+  /* Second phase uninitialization:  Clean up all memory resources */
+
+  DEV1_UNINITIALIZE(priv->dev1);
+  DEV2_UNINITIALIZE(priv->dev2);
+
   /* Then free the composite driver state structure itself */
 
   kfree(priv);
diff --git a/nuttx/drivers/usbdev/usbmsc.c b/nuttx/drivers/usbdev/usbmsc.c
index 710b3ba73..5b46bfac3 100644
--- a/nuttx/drivers/usbdev/usbmsc.c
+++ b/nuttx/drivers/usbdev/usbmsc.c
@@ -1739,6 +1739,22 @@ void usbmsc_uninitialize(FAR void *handle)
 
   priv = &alloc->dev;
 
+#ifdef CONFIG_USBMSC_COMPOSITE
+  /* Check for pass 2 uninitialization.  We did most of the work on the
+   * first pass uninitialization.
+   */
+
+  if (priv->thread == 0)
+    {
+      /* In this second and final pass, all that remains to be done is to
+       * free the memory resources.
+       */
+
+      kfree(priv);
+      return;
+    }
+#endif
+
   /* If the thread hasn't already exitted, tell it to exit now */
 
   if (priv->thstate != USBMSC_STATE_NOTSTARTED)
@@ -1821,5 +1837,13 @@ void usbmsc_uninitialize(FAR void *handle)
   pthread_mutex_destroy(&priv->mutex);
   pthread_cond_destroy(&priv->cond);
 
+#ifndef CONFIG_USBMSC_COMPOSITE
+  /* For the case of the composite driver, there is a two pass
+   * uninitialization sequence.  We cannot yet free the driver structure.
+   * We will do that on the second pass (and we will know that it is the
+   * second pass because of priv->thread == 0)
+   */
+
   kfree(priv);
+#endif
 }