You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2022/07/09 11:11:46 UTC
[incubator-nuttx] branch master updated: net/poll: fix race condition if connect free before poll teardown
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/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 8ae8c10954 net/poll: fix race condition if connect free before poll teardown
8ae8c10954 is described below
commit 8ae8c1095451f0f0e60059962151bbf6ff81e03c
Author: chao.an <an...@xiaomi.com>
AuthorDate: Sat Jul 9 04:43:11 2022 +0800
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 <an...@xiaomi.com>
---
net/icmp/icmp_netpoll.c | 33 ++++++++++++++++++++++++++-------
net/icmpv6/icmpv6_netpoll.c | 31 +++++++++++++++++++++++++------
net/tcp/tcp_netpoll.c | 32 ++++++++++++++++++++------------
net/udp/udp_netpoll.c | 32 ++++++++++++++++++++------------
4 files changed, 91 insertions(+), 37 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,24 +198,25 @@ 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;
+ /* Some of the following must be atomic */
+
+ net_lock();
+
+ conn = psock->s_conn;
+
/* Sanity check */
-#ifdef CONFIG_DEBUG_FEATURES
if (!conn || !fds)
{
- return -EINVAL;
+ ret = -EINVAL;
+ goto errout_with_lock;
}
-#endif
-
- /* Some of the following must be atomic */
-
- net_lock();
/* Non-blocking connection ? */
@@ -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,23 +132,24 @@ 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;
+ /* Some of the following must be atomic */
+
+ net_lock();
+
+ conn = psock->s_conn;
+
/* Sanity check */
-#ifdef CONFIG_DEBUG_FEATURES
if (conn == NULL || fds == NULL)
{
- return -EINVAL;
+ ret = -EINVAL;
+ goto errout_with_lock;
}
-#endif
-
- /* Some of the following must be atomic */
-
- net_lock();
/* Find a container to hold the poll information */
@@ -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;
}