Networking: Modify how callback structures are stored to avoid another potential use of a stal pointer.

net/arp/arp.h

@@ -87,10 +87,8 @@
 
 /* Allocate a new ARP data callback */
 
-#define arp_callback_alloc(dev) \
+#define arp_callback_alloc(dev)   devif_callback_alloc(dev, &(dev)->d_conncb)
-  devif_callback_alloc(dev, &(dev)->d_conncb)
+#define arp_callback_free(dev,cb) devif_dev_callback_free(dev, cb)
-#define arp_callback_free(dev,cb) \
-  devif_callback_free(dev, cb, &(dev)->d_conncb)
 
 /****************************************************************************
  * Public Types

net/devif/devif.h

@@ -303,8 +303,13 @@ void devif_callback_init(void);
  * Description:
  *   Allocate a callback container from the free list.
  *
+ *   If dev is non-NULL, then this function verifies that the device
+ *   reference is still  valid and that the device is still UP status.  If
+ *   those conditions are not true, this function will fail to allocate the
+ *   callback.
+ *
  * Assumptions:
- *   This function is called with interrupts disabled.
+ *   This function is called with the network locked.
  *
  ****************************************************************************/
 
@@ -313,19 +318,49 @@ FAR struct devif_callback_s *
                        FAR struct devif_callback_s **list);
 
 /****************************************************************************
- * Function: devif_callback_free
+ * Function: devif_conn_callback_free
  *
  * Description:
- *   Return a callback container to the free list.
+ *   Return a connection/port callback container to the free list.
+ *
+ *   This function is just a front-end for devif_callback_free().  If the
+ *   dev argument is non-NULL, it will verify that the device reference is
+ *   still valid before attempting to free the callback structure.  A
+ *   non-NULL list pointer is assumed to be valid in any case.
+ *
+ *   The callback structure will be freed in any event.
  *
  * Assumptions:
- *   This function is called with interrupts disabled.
+ *   This function is called with the network locked.
  *
  ****************************************************************************/
 
-void devif_callback_free(FAR struct net_driver_s *dev,
+void devif_conn_callback_free(FAR struct net_driver_s *dev,
-                         FAR struct devif_callback_s *cb,
+                              FAR struct devif_callback_s *cb,
-                         FAR struct devif_callback_s **list);
+                              FAR struct devif_callback_s **list);
+
+/****************************************************************************
+ * Function: devif_dev_callback_free
+ *
+ * Description:
+ *   Return a device callback container to the free list.
+ *
+ *   This function is just a front-end for devif_callback_free().  If the
+ *   de argument is non-NULL, it will verify that the device reference is
+ *   still valid before attempting to free the callback structure.  It
+ *   differs from devif_conn_callback_free in that connection/port-related
+ *   connections are also associated with the device and, hence, also will
+ *   not be valid if the device pointer is not valid.
+ *
+ *   The callback structure will be freed in any event.
+ *
+ * Assumptions:
+ *   This function is called with the network locked.
+ *
+ ****************************************************************************/
+
+void devif_dev_callback_free(FAR struct net_driver_s *dev,
+                             FAR struct devif_callback_s *cb);
 
 /****************************************************************************
  * Function: devif_conn_event

net/devif/devif_callback.c

@@ -63,6 +63,106 @@ static FAR struct devif_callback_s *g_cbfreelist = NULL;
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Function: devif_callback_free
+ *
+ * Description:
+ *   Return a callback container to the free list.
+ *
+ * Assumptions:
+ *   This function is called with the network locked.
+ *
+ ****************************************************************************/
+
+static void devif_callback_free(FAR struct net_driver_s *dev,
+                                FAR struct devif_callback_s *cb,
+                                FAR struct devif_callback_s **list)
+{
+  FAR struct devif_callback_s *prev;
+  FAR struct devif_callback_s *curr;
+  net_lock_t save;
+
+  if (cb)
+    {
+      save = net_lock();
+
+#ifdef CONFIG_DEBUG
+      /* Check for double freed callbacks */
+
+      curr = g_cbfreelist;
+
+      while (curr != NULL)
+        {
+          DEBUGASSERT(cb != curr);
+          curr = curr->nxtconn;
+        }
+#endif
+
+      /* Remove the callback structure from the device notification list if
+       * it is supposed to be in the device notification list.
+       */
+
+      if (dev)
+        {
+          /* Find the callback structure in the device event list */
+
+          for (prev = NULL, curr = dev->d_devcb;
+               curr && curr != cb;
+               prev = curr, curr = curr->nxtdev);
+
+          /* Remove the structure from the device event list */
+
+          DEBUGASSERT(curr);
+          if (curr)
+            {
+              if (prev)
+                {
+                  prev->nxtdev = cb->nxtdev;
+                }
+              else
+                {
+                  dev->d_devcb = cb->nxtdev;
+                }
+            }
+        }
+
+      /* Remove the callback structure from the data notification list if
+       * it is supposed to be in the data notification list.
+       */
+
+      if (list)
+        {
+          /* Find the callback structure in the connection event list */
+
+          for (prev = NULL, curr = *list;
+               curr && curr != cb;
+               prev = curr, curr = curr->nxtconn);
+
+          /* Remove the structure from the connection event list */
+
+          DEBUGASSERT(curr);
+          if (curr)
+            {
+              if (prev)
+                {
+                  prev->nxtconn = cb->nxtconn;
+                }
+              else
+                {
+                  *list = cb->nxtconn;
+                }
+            }
+        }
+
+      /* Put the structure into the free list */
+
+      cb->nxtconn  = g_cbfreelist;
+      cb->nxtdev   = NULL;
+      g_cbfreelist = cb;
+      net_unlock(save);
+    }
+}
+
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
@@ -95,8 +195,13 @@ void devif_callback_init(void)
  * Description:
  *   Allocate a callback container from the free list.
  *
+ *   If dev is non-NULL, then this function verifies that the device
+ *   reference is still  valid and that the device is still UP status.  If
+ *   those conditions are not true, this function will fail to allocate the
+ *   callback.
+ *
  * Assumptions:
- *   This function is called with interrupts disabled.
+ *   This function is called with the network locked.
  *
  ****************************************************************************/
 
@@ -164,104 +269,93 @@ FAR struct devif_callback_s *
 }
 
 /****************************************************************************
- * Function: devif_callback_free
+ * Function: devif_conn_callback_free
  *
  * Description:
- *   Return a callback container to the free list.
+ *   Return a connection/port callback container to the free list.
+ *
+ *   This function is just a front-end for devif_callback_free().  If the
+ *   dev argument is non-NULL, it will verify that the device reference is
+ *   still valid before attempting to free the callback structure.  A
+ *   non-NULL list pointer is assumed to be valid in any case.
+ *
+ *   The callback structure will be freed in any event.
  *
  * Assumptions:
- *   This function is called with interrupts disabled.
+ *   This function is called with the network locked.
  *
  ****************************************************************************/
 
-void devif_callback_free(FAR struct net_driver_s *dev,
+void devif_conn_callback_free(FAR struct net_driver_s *dev,
-                         FAR struct devif_callback_s *cb,
+                              FAR struct devif_callback_s *cb,
-                         FAR struct devif_callback_s **list)
+                              FAR struct devif_callback_s **list)
 {
-  FAR struct devif_callback_s *prev;
+  /* Check if the device pointer is still valid.  It could be invalid if, for
-  FAR struct devif_callback_s *curr;
+   * example, the device were unregistered between the time when the callback
-  net_lock_t save;
+   * was allocated and the time when the callback was freed.
+   */
 
-  if (cb)
+  if (dev && !netdev_verify(dev))
     {
-      save = net_lock();
+      /* The device reference is longer valid */
 
-#ifdef CONFIG_DEBUG
+      dev = NULL;
-      /* Check for double freed callbacks */
-
-      curr = g_cbfreelist;
-
-      while (curr != NULL)
-        {
-          DEBUGASSERT(cb != curr);
-          curr = curr->nxtconn;
-        }
-#endif
-
-      /* Remove the callback structure from the device notification list if
-       * it is supposed to be in the device notification list AND if the
-       * device pointer is still valid.
-       */
-
-      if (dev && netdev_verify(dev))
-        {
-          /* Find the callback structure in the device event list */
-
-          for (prev = NULL, curr = dev->d_devcb;
-               curr && curr != cb;
-               prev = curr, curr = curr->nxtdev);
-
-          /* Remove the structure from the device event list */
-
-          DEBUGASSERT(curr);
-          if (curr)
-            {
-              if (prev)
-                {
-                  prev->nxtdev = cb->nxtdev;
-                }
-              else
-                {
-                  dev->d_devcb = cb->nxtdev;
-                }
-            }
-        }
-
-      /* Remove the callback structure from the data notification list if
-       * it is supposed to be in the data notification list.
-       */
-
-      if (list)
-        {
-          /* Find the callback structure in the connection event list */
-
-          for (prev = NULL, curr = *list;
-               curr && curr != cb;
-               prev = curr, curr = curr->nxtconn);
-
-          /* Remove the structure from the connection event list */
-
-          DEBUGASSERT(curr);
-          if (curr)
-            {
-              if (prev)
-                {
-                  prev->nxtconn = cb->nxtconn;
-                }
-              else
-                {
-                  *list = cb->nxtconn;
-                }
-            }
-        }
-
-      /* Put the structure into the free list */
-
-      cb->nxtconn  = g_cbfreelist;
-      cb->nxtdev   = NULL;
-      g_cbfreelist = cb;
-      net_unlock(save);
     }
+
+  /* Then free the callback */
+
+  devif_callback_free(dev, cb, list);
+}
+
+/****************************************************************************
+ * Function: devif_dev_callback_free
+ *
+ * Description:
+ *   Return a device callback container to the free list.
+ *
+ *   This function is just a front-end for devif_callback_free().  If the
+ *   de argument is non-NULL, it will verify that the device reference is
+ *   still valid before attempting to free the callback structure.  It
+ *   differs from devif_conn_callback_free in that connection/port-related
+ *   connections are also associated with the device and, hence, also will
+ *   not be valid if the device pointer is not valid.
+ *
+ *   The callback structure will be freed in any event.
+ *
+ * Assumptions:
+ *   This function is called with the network locked.
+ *
+ ****************************************************************************/
+
+void devif_dev_callback_free(FAR struct net_driver_s *dev,
+                             FAR struct devif_callback_s *cb)
+{
+  FAR struct devif_callback_s **list;
+
+  /* Check if the device pointer is still valid.  It could be invalid if, for
+   * example, the device were unregistered between the time when the callback
+   * was allocated and the time when the callback was freed.
+   */
+
+  if (dev && netdev_verify(dev))
+    {
+      /* The device reference is valid.. the use the list pointer in the
+       * device structure as well.
+       */
+
+      list = &dev->d_conncb;
+    }
+  else
+    {
+      /* The device reference is longer valid */
+
+      dev  = NULL;
+      list = NULL;
+    }
+
+  /* Then free the callback */
+
+  devif_callback_free(dev, cb, list);
 }
 
 /****************************************************************************

net/icmp/icmp_ping.c

@@ -71,10 +71,8 @@
 
 /* Allocate a new ICMP data callback */
 
-#define icmp_callback_alloc(dev) \
+#define icmp_callback_alloc(dev)   devif_callback_alloc(dev, &(dev)->d_conncb)
-  devif_callback_alloc(dev, &(dev)->d_conncb)
+#define icmp_callback_free(dev,cb) devif_dev_callback_free(dev, cb)
-#define icmp_callback_free(dev,cb) \
-  devif_callback_free(dev, cb, &(dev)->d_conncb)
 
 /****************************************************************************
  * Private Types

net/icmpv6/icmpv6.h

@@ -55,10 +55,8 @@
 
 /* Allocate a new ICMPv6 data callback */
 
-#define icmpv6_callback_alloc(dev) \
+#define icmpv6_callback_alloc(dev)   devif_callback_alloc(dev, &(dev)->d_conncb)
-  devif_callback_alloc(dev, &(dev)->d_conncb)
+#define icmpv6_callback_free(dev,cb) devif_dev_callback_free(dev, cb)
-#define icmpv6_callback_free(dev,cb) \
-  devif_callback_free(dev, cb, &(dev)->d_conncb)
 
 /****************************************************************************
  * Public Type Definitions

net/pkt/pkt.h

@@ -52,8 +52,10 @@
 
 /* Allocate a new packet socket data callback */
 
-#define pkt_callback_alloc(dev,conn)   devif_callback_alloc(dev, &conn->list)
+#define pkt_callback_alloc(dev,conn) \
-#define pkt_callback_free(dev,conn,cb) devif_callback_free(dev, cb, &conn->list)
+  devif_callback_alloc(dev, &conn->list)
+#define pkt_callback_free(dev,conn,cb) \
+  devif_conn_callback_free(dev, cb, &conn->list)
 
 /****************************************************************************
  * Public Type Definitions

net/tcp/tcp.h

@@ -70,7 +70,7 @@
 #  define tcp_callback_alloc(conn) \
     devif_callback_alloc(conn->dev, &conn->list)
 #  define tcp_callback_free(conn,cb) \
-    devif_callback_free(conn->dev, cb, &conn->list)
+    devif_conn_callback_free(conn->dev, cb, &conn->list)
 
 /* These macros allocate and free callback structures used for receiving
  * notifications of device-related events.
@@ -79,7 +79,7 @@
 #  define tcp_monitor_callback_alloc(conn) \
     devif_callback_alloc(conn->dev, NULL)
 #  define tcp_monitor_callback_free(conn,cb) \
-    devif_callback_free(conn->dev, cb, NULL)
+    devif_conn_callback_free(conn->dev, cb, NULL)
 
 #else
 /* These macros allocate and free callback structures used for receiving
@@ -89,7 +89,7 @@
 #  define tcp_callback_alloc(conn) \
     devif_callback_alloc(g_netdevices, &conn->list)
 #  define tcp_callback_free(conn,cb) \
-    devif_callback_free(g_netdevices, cb, &conn->list)
+    devif_conn_callback_free(g_netdevices, cb, &conn->list)
 
 /* These macros allocate and free callback structures used for receiving
  * notifications of device-related events.
@@ -98,7 +98,7 @@
 #  define tcp_monitor_callback_alloc(conn) \
     devif_callback_alloc(g_netdevices, NULL)
 #  define tcp_monitor_callback_free(conn,cb) \
-    devif_callback_free(g_netdevices, cb, NULL)
+    devif_conn_callback_free(g_netdevices, cb, NULL)
 #endif
 
 /* Get the current maximum segment size that can be sent on the current

net/udp/udp.h

@@ -68,7 +68,7 @@
 #define udp_callback_alloc(dev, conn) \
   devif_callback_alloc(dev, &conn->list)
 #define udp_callback_free(dev, conn,cb) \
-  devif_callback_free(dev, cb, &conn->list)
+  devif_conn_callback_free(dev, cb, &conn->list)
 
 /****************************************************************************
  * Public Type Definitions