net/icmp: replace net_lock with conn_lock and conn_lock_dev

Protect icmp resources through netdev_lock, conn_lock, and icmp_list_lock

Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>

net/icmp/icmp.h

@@ -220,6 +220,22 @@ FAR struct icmp_conn_s *icmp_active(uint16_t id);
 FAR struct icmp_conn_s *icmp_nextconn(FAR struct icmp_conn_s *conn);
 #endif
 
+/****************************************************************************
+ * Name: icmp_conn_list_lock
+ *       icmp_conn_list_unlock
+ *
+ * Description:
+ *   Lock and unlock the ICMP connection list.  This is used to protect the
+ *   list of active connections.
+ *
+ * Assumptions:
+ *   This function is called from driver.
+ *
+ ****************************************************************************/
+
+void icmp_conn_list_lock(void);
+void icmp_conn_list_unlock(void);
+
 /****************************************************************************
  * Name: icmp_findconn
  *

net/icmp/icmp_conn.c

@@ -121,26 +121,18 @@ void icmp_free(FAR struct icmp_conn_s *conn)
 
   NET_BUFPOOL_LOCK(g_icmp_connections);
 
-  /* Is this the last reference on the connection?  It might not be if the
-   * socket was cloned.
-   */
+  /* free any read-ahead data */
 
-  if (conn->crefs > 1)
-    {
-      /* No.. just decrement the reference count */
+  iob_free_queue(&conn->readahead);
 
-      conn->crefs--;
-    }
-  else
-    {
-      /* Remove the connection from the active list */
+  /* Remove the connection from the active list */
 
-      dq_rem(&conn->sconn.node, &g_active_icmp_connections);
+  dq_rem(&conn->sconn.node, &g_active_icmp_connections);
+  nxmutex_destroy(&conn->sconn.s_lock);
 
-      /* Free the connection. */
+  /* Free the connection. */
 
-      NET_BUFPOOL_FREE(g_icmp_connections, conn);
-    }
+  NET_BUFPOOL_FREE(g_icmp_connections, conn);
 
   NET_BUFPOOL_UNLOCK(g_icmp_connections);
 }
@@ -204,6 +196,29 @@ FAR struct icmp_conn_s *icmp_nextconn(FAR struct icmp_conn_s *conn)
     }
 }
 
+/****************************************************************************
+ * Name: icmp_conn_list_lock
+ *       icmp_conn_list_unlock
+ *
+ * Description:
+ *   Lock and unlock the ICMP connection list.  This is used to protect the
+ *   list of active connections.
+ *
+ * Assumptions:
+ *   This function is called from driver.
+ *
+ ****************************************************************************/
+
+void icmp_conn_list_lock(void)
+{
+  NET_BUFPOOL_LOCK(g_icmp_connections);
+}
+
+void icmp_conn_list_unlock(void)
+{
+  NET_BUFPOOL_UNLOCK(g_icmp_connections);
+}
+
 /****************************************************************************
  * Name: icmp_findconn
  *
@@ -221,6 +236,7 @@ FAR struct icmp_conn_s *icmp_findconn(FAR struct net_driver_s *dev,
 {
   FAR struct icmp_conn_s *conn;
 
+  NET_BUFPOOL_LOCK(g_icmp_connections);
   for (conn = icmp_nextconn(NULL); conn != NULL; conn = icmp_nextconn(conn))
     {
       if (conn->id == id && conn->dev == dev)
@@ -229,6 +245,7 @@ FAR struct icmp_conn_s *icmp_findconn(FAR struct net_driver_s *dev,
         }
     }
 
+  NET_BUFPOOL_UNLOCK(g_icmp_connections);
   return conn;
 }
 
@@ -250,6 +267,7 @@ int icmp_foreach(icmp_callback_t callback, FAR void *arg)
   FAR struct icmp_conn_s *conn;
   int ret = 0;
 
+  NET_BUFPOOL_LOCK(g_icmp_connections);
   if (callback != NULL)
     {
       for (conn = icmp_nextconn(NULL); conn != NULL;
@@ -263,6 +281,7 @@ int icmp_foreach(icmp_callback_t callback, FAR void *arg)
         }
     }
 
+  NET_BUFPOOL_UNLOCK(g_icmp_connections);
   return ret;
 }
 #endif /* CONFIG_NET_ICMP */

net/icmp/icmp_input.c

@@ -166,6 +166,7 @@ static uint16_t icmp_datahandler(FAR struct net_driver_s *dev,
    * without waiting).
    */
 
+  conn_lock(&conn->sconn);
   ret = iob_tryadd_queue(iob, &conn->readahead);
   if (ret < 0)
     {
@@ -177,6 +178,7 @@ static uint16_t icmp_datahandler(FAR struct net_driver_s *dev,
       ninfo("Buffered %d bytes\n", buflen);
     }
 
+  conn_unlock(&conn->sconn);
   return buflen;
 }
 

net/icmp/icmp_ioctl.c

@@ -38,6 +38,7 @@
 #include <nuttx/mm/iob.h>
 #include <nuttx/net/net.h>
 
+#include "utils/utils.h"
 #include "icmp/icmp.h"
 
 /****************************************************************************
@@ -62,7 +63,7 @@ int icmp_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
   FAR struct icmp_conn_s *conn = psock->s_conn;
   int ret = OK;
 
-  net_lock();
+  conn_lock(&conn->sconn);
 
   switch (cmd)
     {
@@ -91,7 +92,7 @@ int icmp_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
         break;
     }
 
-  net_unlock();
+  conn_unlock(&conn->sconn);
 
   return ret;
 }

net/icmp/icmp_netpoll.c

@@ -36,6 +36,7 @@
 
 #include "devif/devif.h"
 #include "netdev/netdev.h"
+#include "utils/utils.h"
 #include "icmp/icmp.h"
 
 /****************************************************************************
@@ -144,20 +145,19 @@ int icmp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
   pollevent_t eventset = 0;
   int ret = OK;
 
-  /* Some of the following must be atomic */
-
-  net_lock();
-
   conn = psock->s_conn;
 
   /* Sanity check */
 
   if (!conn || !fds)
     {
-      ret = -EINVAL;
-      goto errout_with_lock;
+      return -EINVAL;
     }
 
+  /* Some of the following must be atomic */
+
+  conn_dev_lock(&conn->sconn, conn->dev);
+
   /* Find a container to hold the poll information */
 
   info = conn->pollinfo;
@@ -226,7 +226,7 @@ int icmp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
   poll_notify(&fds, 1, eventset);
 
 errout_with_lock:
-  net_unlock();
+  conn_dev_unlock(&conn->sconn, conn->dev);
   return ret;
 }
 
@@ -251,17 +251,12 @@ int icmp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds)
   FAR struct icmp_conn_s *conn;
   FAR struct icmp_poll_s *info;
 
-  /* Some of the following must be atomic */
-
-  net_lock();
-
   conn = psock->s_conn;
 
   /* Sanity check */
 
   if (!conn || !fds->priv)
     {
-      net_unlock();
       return -EINVAL;
     }
 
@@ -272,6 +267,10 @@ int icmp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds)
 
   if (info != NULL)
     {
+      /* Some of the following must be atomic */
+
+      conn_dev_lock(&conn->sconn, info->dev);
+
       /* Release the callback */
 
       icmp_callback_free(info->dev, conn, info->cb);
@@ -283,9 +282,8 @@ int icmp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds)
       /* Then free the poll info container */
 
       info->psock = NULL;
+      conn_dev_unlock(&conn->sconn, info->dev);
     }
 
-  net_unlock();
-
   return OK;
 }

net/icmp/icmp_recvmsg.c

@@ -37,6 +37,7 @@
 
 #include "devif/devif.h"
 #include "socket/socket.h"
+#include "utils/utils.h"
 #include "icmp/icmp.h"
 
 #ifdef CONFIG_NET_ICMP_SOCKET
@@ -320,15 +321,14 @@ ssize_t icmp_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
         }
     }
 
-  net_lock();
-
   conn = psock->s_conn;
+  dev = conn->dev;
+
+  conn_dev_lock(&conn->sconn, dev);
   if (psock->s_type != SOCK_RAW)
     {
       /* Get the device that was used to send the ICMP request. */
 
-      dev = conn->dev;
-      DEBUGASSERT(dev != NULL);
       if (dev == NULL)
         {
           ret = -EPROTO;
@@ -378,8 +378,10 @@ ssize_t icmp_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
            * received.
            */
 
+          conn_dev_unlock(&conn->sconn, dev);
           ret = net_sem_timedwait(&state.recv_sem,
                               _SO_TIMEOUT(conn->sconn.s_rcvtimeo));
+          conn_dev_lock(&conn->sconn, dev);
           if (ret < 0)
             {
               state.recv_result = ret;
@@ -426,7 +428,7 @@ errout:
         }
     }
 
-  net_unlock();
+  conn_dev_unlock(&conn->sconn, dev);
 
   return ret;
 }

net/icmp/icmp_sendmsg.c

@@ -350,10 +350,12 @@ ssize_t icmp_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
   if (psock->s_type != SOCK_RAW && (icmp->type != ICMP_ECHO_REQUEST ||
       icmp->id != conn->id || dev != conn->dev))
     {
+      conn_lock(&conn->sconn);
       conn->id  = 0;
       conn->dev = NULL;
 
       iob_free_queue(&conn->readahead);
+      conn_unlock(&conn->sconn);
     }
 
 #ifdef CONFIG_NET_ARP_SEND
@@ -379,7 +381,7 @@ ssize_t icmp_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
   state.snd_buflen = len;                     /* Size of the ICMP header +
                                                * data payload */
 
-  net_lock();
+  conn_dev_lock(&conn->sconn, dev);
 
   /* Set up the callback */
 
@@ -401,6 +403,7 @@ ssize_t icmp_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
 
       /* Notify the device driver of the availability of TX data */
 
+      conn_dev_unlock(&conn->sconn, dev);
       netdev_txnotify_dev(dev);
 
       /* Wait for either the send to complete or for timeout to occur.
@@ -409,6 +412,7 @@ ssize_t icmp_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
 
       ret = net_sem_timedwait(&state.snd_sem,
                           _SO_TIMEOUT(conn->sconn.s_sndtimeo));
+      conn_dev_lock(&conn->sconn, dev);
       if (ret < 0)
         {
           if (ret == -ETIMEDOUT)
@@ -442,7 +446,7 @@ ssize_t icmp_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
 
   nxsem_destroy(&state.snd_sem);
 
-  net_unlock();
+  conn_dev_unlock(&conn->sconn, dev);
 
   /* Return the negated error number in the event of a failure, or the
    * number of bytes sent on success.
@@ -458,10 +462,12 @@ ssize_t icmp_sendmsg(FAR struct socket *psock, FAR struct msghdr *msg,
   return len;
 
 errout:
+  conn_lock(&conn->sconn);
   conn->id  = 0;
   conn->dev = NULL;
 
   iob_free_queue(&conn->readahead);
+  conn_unlock(&conn->sconn);
   return ret;
 }
 

net/icmp/icmp_sockif.c

@@ -41,6 +41,7 @@
 
 #include "icmp/icmp.h"
 #include "inet/inet.h"
+#include "utils/utils.h"
 
 #ifdef CONFIG_NET_ICMP_SOCKET
 
@@ -144,6 +145,8 @@ static int icmp_setup(FAR struct socket *psock)
           conn->filter = UINT32_MAX;
         }
 
+      nxmutex_init(&conn->sconn.s_lock);
+
       /* Save the pre-allocated connection in the socket structure */
 
       psock->s_conn = conn;
@@ -256,7 +259,6 @@ static int icmp_close(FAR struct socket *psock)
 {
   FAR struct icmp_conn_s *conn;
 
-  net_lock();
   conn = psock->s_conn;
 
   /* Is this the last reference to the connection structure (there could be\
@@ -267,10 +269,6 @@ static int icmp_close(FAR struct socket *psock)
 
   if (conn->crefs <= 1)
     {
-      /* Yes... free any read-ahead data */
-
-      iob_free_queue(&conn->readahead);
-
       /* Then free the connection structure */
 
       conn->crefs = 0;           /* No more references on the connection */
@@ -283,7 +281,6 @@ static int icmp_close(FAR struct socket *psock)
       conn->crefs--;
     }
 
-  net_unlock();
   return OK;
 }
 
@@ -324,7 +321,6 @@ static int icmp_getsockopt_internal(FAR struct socket *psock, int option,
       return -ENOPROTOOPT;
     }
 
-  net_lock();
   switch (option)
     {
       case ICMP_FILTER:
@@ -336,7 +332,9 @@ static int icmp_getsockopt_internal(FAR struct socket *psock, int option,
               *value_len = sizeof(uint32_t);
             }
 
+          conn_lock(&conn->sconn);
           memcpy(value, &conn->filter, *value_len);
+          conn_unlock(&conn->sconn);
           ret = OK;
         }
         break;
@@ -347,7 +345,6 @@ static int icmp_getsockopt_internal(FAR struct socket *psock, int option,
         break;
     }
 
-  net_unlock();
   return ret;
 }
 
@@ -441,7 +438,6 @@ static int icmp_setsockopt_internal(FAR struct socket *psock, int option,
       return -ENOPROTOOPT;
     }
 
-  net_lock();
   switch (option)
     {
       case ICMP_FILTER:
@@ -453,7 +449,9 @@ static int icmp_setsockopt_internal(FAR struct socket *psock, int option,
               value_len = sizeof(uint32_t);
             }
 
+          conn_lock(&conn->sconn);
           memcpy(&conn->filter, value, value_len);
+          conn_unlock(&conn->sconn);
           ret = OK;
         }
         break;
@@ -464,7 +462,6 @@ static int icmp_setsockopt_internal(FAR struct socket *psock, int option,
         break;
     }
 
-  net_unlock();
   return ret;
 }