This is an automated email from the ASF dual-hosted git repository. xiaoxiang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 1d91a1582070e30f791f6af07a34ecc31118e9a5 Author: zhanghongyu <[email protected]> AuthorDate: Thu Jun 19 11:13:05 2025 +0800 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 <[email protected]> --- net/devif/devif_poll.c | 2 ++ net/icmp/icmp.h | 16 ++++++++++++++++ net/icmp/icmp_conn.c | 49 ++++++++++++++++++++++++++++++++++--------------- net/icmp/icmp_input.c | 2 ++ net/icmp/icmp_ioctl.c | 5 +++-- net/icmp/icmp_netpoll.c | 26 ++++++++++++-------------- net/icmp/icmp_recvmsg.c | 12 +++++++----- net/icmp/icmp_sendmsg.c | 10 ++++++++-- net/icmp/icmp_sockif.c | 17 +++++++---------- 9 files changed, 91 insertions(+), 48 deletions(-) diff --git a/net/devif/devif_poll.c b/net/devif/devif_poll.c index 1bb4e46cbef..02b05131de9 100644 --- a/net/devif/devif_poll.c +++ b/net/devif/devif_poll.c @@ -412,6 +412,7 @@ static inline int devif_poll_icmp(FAR struct net_driver_s *dev, * action. */ + icmp_conn_list_lock(); while (!bstop && (conn = icmp_nextconn(conn)) != NULL) { /* Skip ICMP connections that are bound to other polling devices */ @@ -432,6 +433,7 @@ static inline int devif_poll_icmp(FAR struct net_driver_s *dev, } } + icmp_conn_list_unlock(); return bstop; } #endif /* CONFIG_NET_ICMP && CONFIG_NET_ICMP_SOCKET */ diff --git a/net/icmp/icmp.h b/net/icmp/icmp.h index 9e8f6e00809..eaf82f7abfb 100644 --- a/net/icmp/icmp.h +++ b/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 * diff --git a/net/icmp/icmp_conn.c b/net/icmp/icmp_conn.c index 481a2676c68..5457b0268c3 100644 --- a/net/icmp/icmp_conn.c +++ b/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 */ diff --git a/net/icmp/icmp_input.c b/net/icmp/icmp_input.c index 85d9811e2d0..1dd19b4d1e5 100644 --- a/net/icmp/icmp_input.c +++ b/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; } diff --git a/net/icmp/icmp_ioctl.c b/net/icmp/icmp_ioctl.c index c114e7537ba..e37aa4ea571 100644 --- a/net/icmp/icmp_ioctl.c +++ b/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; } diff --git a/net/icmp/icmp_netpoll.c b/net/icmp/icmp_netpoll.c index 27d04e7a119..2966a077563 100644 --- a/net/icmp/icmp_netpoll.c +++ b/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; } diff --git a/net/icmp/icmp_recvmsg.c b/net/icmp/icmp_recvmsg.c index 9fb43a61cfb..f871931dda3 100644 --- a/net/icmp/icmp_recvmsg.c +++ b/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; } diff --git a/net/icmp/icmp_sendmsg.c b/net/icmp/icmp_sendmsg.c index eb83a3301ae..82da6cd8f82 100644 --- a/net/icmp/icmp_sendmsg.c +++ b/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; } diff --git a/net/icmp/icmp_sockif.c b/net/icmp/icmp_sockif.c index 5c3cdcb7b9c..729eca45577 100644 --- a/net/icmp/icmp_sockif.c +++ b/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; }
