From 8ae8c1095451f0f0e60059962151bbf6ff81e03c Mon Sep 17 00:00:00 2001 From: "chao.an" Date: Sat, 9 Jul 2022 04:43:11 +0800 Subject: [PATCH] net/poll: fix race condition if connect free before poll teardown Net poll teardown is not protected by net lock, if the conn is released before teardown, the assertion failure will be triggered during free dev callback, this patch will add the net lock around net poll teardown to fix race condition nuttx/libs/libc/assert/lib_assert.c:36 nuttx/net/devif/devif_callback.c:85 nuttx/net/tcp/tcp_netpoll.c:405 nuttx/fs/vfs/fs_poll.c:244 nuttx/fs/vfs/fs_poll.c:500 Signed-off-by: chao.an --- net/icmp/icmp_netpoll.c | 33 +++++++++++++++++++++++++------- net/icmpv6/icmpv6_netpoll.c | 31 ++++++++++++++++++++++++------ net/tcp/tcp_netpoll.c | 38 ++++++++++++++++++++++--------------- net/udp/udp_netpoll.c | 38 ++++++++++++++++++++++--------------- 4 files changed, 97 insertions(+), 43 deletions(-) diff --git a/net/icmp/icmp_netpoll.c b/net/icmp/icmp_netpoll.c index a9ee08c43c..3438bc338f 100644 --- a/net/icmp/icmp_netpoll.c +++ b/net/icmp/icmp_netpoll.c @@ -142,17 +142,25 @@ static uint16_t icmp_poll_eventhandler(FAR struct net_driver_s *dev, int icmp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) { - FAR struct icmp_conn_s *conn = psock->s_conn; + FAR struct icmp_conn_s *conn; FAR struct icmp_poll_s *info; FAR struct devif_callback_s *cb; int ret = OK; - DEBUGASSERT(conn != NULL && fds != NULL); - - /* Some of the following must be atomic */ + /* Some of the following must be atomic */ net_lock(); + conn = psock->s_conn; + + /* Sanity check */ + + if (!conn || !fds) + { + ret = -EINVAL; + goto errout_with_lock; + } + /* Find a container to hold the poll information */ info = conn->pollinfo; @@ -250,15 +258,24 @@ int icmp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) FAR struct icmp_conn_s *conn; FAR struct icmp_poll_s *info; - DEBUGASSERT(psock != NULL && psock->s_conn != NULL && - fds != NULL && fds->priv != NULL); + /* Some of the following must be atomic */ + + net_lock(); conn = psock->s_conn; + /* Sanity check */ + + if (!conn || !fds->priv) + { + net_unlock(); + return -EINVAL; + } + /* Recover the socket descriptor poll state info from the poll structure */ info = (FAR struct icmp_poll_s *)fds->priv; - DEBUGASSERT(info != NULL && info->fds != NULL && info->cb != NULL); + DEBUGASSERT(info->fds != NULL && info->cb != NULL); if (info != NULL) { @@ -275,5 +292,7 @@ int icmp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) info->psock = NULL; } + net_unlock(); + return OK; } diff --git a/net/icmpv6/icmpv6_netpoll.c b/net/icmpv6/icmpv6_netpoll.c index 756e4c22e4..aca4c8c92e 100644 --- a/net/icmpv6/icmpv6_netpoll.c +++ b/net/icmpv6/icmpv6_netpoll.c @@ -142,17 +142,25 @@ static uint16_t icmpv6_poll_eventhandler(FAR struct net_driver_s *dev, int icmpv6_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) { - FAR struct icmpv6_conn_s *conn = psock->s_conn; + FAR struct icmpv6_conn_s *conn; FAR struct icmpv6_poll_s *info; FAR struct devif_callback_s *cb; int ret = OK; - DEBUGASSERT(conn != NULL && fds != NULL); - /* Some of the following must be atomic */ net_lock(); + conn = psock->s_conn; + + /* Sanity check */ + + if (!conn || !fds) + { + ret = -EINVAL; + goto errout_with_lock; + } + /* Find a container to hold the poll information */ info = conn->pollinfo; @@ -250,15 +258,24 @@ int icmpv6_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) FAR struct icmpv6_conn_s *conn; FAR struct icmpv6_poll_s *info; - DEBUGASSERT(psock != NULL && psock->s_conn != NULL && - fds != NULL && fds->priv != NULL); + /* Some of the following must be atomic */ + + net_lock(); conn = psock->s_conn; + /* Sanity check */ + + if (!conn || !fds->priv) + { + net_unlock(); + return -EINVAL; + } + /* Recover the socket descriptor poll state info from the poll structure */ info = (FAR struct icmpv6_poll_s *)fds->priv; - DEBUGASSERT(info != NULL && info->fds != NULL && info->cb != NULL); + DEBUGASSERT(info->fds != NULL && info->cb != NULL); if (info != NULL) { @@ -275,5 +292,7 @@ int icmpv6_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) info->psock = NULL; } + net_unlock(); + return OK; } diff --git a/net/tcp/tcp_netpoll.c b/net/tcp/tcp_netpoll.c index da47cc0b4f..e9b6f7c2c6 100644 --- a/net/tcp/tcp_netpoll.c +++ b/net/tcp/tcp_netpoll.c @@ -198,25 +198,26 @@ static uint16_t tcp_poll_eventhandler(FAR struct net_driver_s *dev, int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) { - FAR struct tcp_conn_s *conn = psock->s_conn; + FAR struct tcp_conn_s *conn; FAR struct tcp_poll_s *info; FAR struct devif_callback_s *cb; bool nonblock_conn; int ret = OK; - /* Sanity check */ - -#ifdef CONFIG_DEBUG_FEATURES - if (!conn || !fds) - { - return -EINVAL; - } -#endif - - /* Some of the following must be atomic */ + /* Some of the following must be atomic */ net_lock(); + conn = psock->s_conn; + + /* Sanity check */ + + if (!conn || !fds) + { + ret = -EINVAL; + goto errout_with_lock; + } + /* Non-blocking connection ? */ nonblock_conn = (conn->tcpstateflags == TCP_SYN_SENT && @@ -382,22 +383,27 @@ errout_with_lock: int tcp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) { - FAR struct tcp_conn_s *conn = psock->s_conn; + FAR struct tcp_conn_s *conn; FAR struct tcp_poll_s *info; + /* Some of the following must be atomic */ + + net_lock(); + + conn = psock->s_conn; + /* Sanity check */ -#ifdef CONFIG_DEBUG_FEATURES if (!conn || !fds->priv) { + net_unlock(); return -EINVAL; } -#endif /* Recover the socket descriptor poll state info from the poll structure */ info = (FAR struct tcp_poll_s *)fds->priv; - DEBUGASSERT(info != NULL && info->fds != NULL && info->cb != NULL); + DEBUGASSERT(info->fds != NULL && info->cb != NULL); if (info != NULL) { /* Release the callback */ @@ -413,5 +419,7 @@ int tcp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) info->conn = NULL; } + net_unlock(); + return OK; } diff --git a/net/udp/udp_netpoll.c b/net/udp/udp_netpoll.c index bb26cf7af2..d1b86396d0 100644 --- a/net/udp/udp_netpoll.c +++ b/net/udp/udp_netpoll.c @@ -132,24 +132,25 @@ static uint16_t udp_poll_eventhandler(FAR struct net_driver_s *dev, int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) { - FAR struct udp_conn_s *conn = psock->s_conn; + FAR struct udp_conn_s *conn; FAR struct udp_poll_s *info; FAR struct devif_callback_s *cb; int ret = OK; - /* Sanity check */ - -#ifdef CONFIG_DEBUG_FEATURES - if (conn == NULL || fds == NULL) - { - return -EINVAL; - } -#endif - - /* Some of the following must be atomic */ + /* Some of the following must be atomic */ net_lock(); + conn = psock->s_conn; + + /* Sanity check */ + + if (conn == NULL || fds == NULL) + { + ret = -EINVAL; + goto errout_with_lock; + } + /* Find a container to hold the poll information */ info = conn->pollinfo; @@ -257,22 +258,27 @@ errout_with_lock: int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) { - FAR struct udp_conn_s *conn = psock->s_conn; + FAR struct udp_conn_s *conn; FAR struct udp_poll_s *info; + /* Some of the following must be atomic */ + + net_lock(); + + conn = psock->s_conn; + /* Sanity check */ -#ifdef CONFIG_DEBUG_FEATURES if (!conn || !fds->priv) { + net_unlock(); return -EINVAL; } -#endif /* Recover the socket descriptor poll state info from the poll structure */ info = (FAR struct udp_poll_s *)fds->priv; - DEBUGASSERT(info != NULL && info->fds != NULL && info->cb != NULL); + DEBUGASSERT(info->fds != NULL && info->cb != NULL); if (info != NULL) { /* Release the callback */ @@ -288,5 +294,7 @@ int udp_pollteardown(FAR struct socket *psock, FAR struct pollfd *fds) info->conn = NULL; } + net_unlock(); + return OK; }