mirror of
https://github.com/apache/nuttx.git
synced 2026-05-28 11:56:10 +08:00
net/tcp_conn: fix thread_cancel() caused recv/connect/send event used after free
Problem Description: Problem occurrence when Thread1 creat connect/recv socket and Thread2 cancel Thread1. 1. Thread2 cancel when Thread1 connect event, will cause DEBUGASSERT in devif_callback_free.Because cb in g_cbfreelist. 2. Thread2 cancel when Thread1 recvfrom sem-wait, when the FIN packet input and will trigger tcp_recvhandler and will crash.Becuase some thread stack data was freed. Signed-off-by: meijian <meijian@xiaomi.com>
This commit is contained in:
@@ -443,6 +443,14 @@ struct tcp_backlog_s
|
|||||||
};
|
};
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
struct tcp_callback_s
|
||||||
|
{
|
||||||
|
FAR struct tcp_conn_s *tc_conn;
|
||||||
|
FAR struct devif_callback_s *tc_cb;
|
||||||
|
sem_t *tc_sem;
|
||||||
|
bool tc_free;
|
||||||
|
};
|
||||||
|
|
||||||
/****************************************************************************
|
/****************************************************************************
|
||||||
* Public Data
|
* Public Data
|
||||||
****************************************************************************/
|
****************************************************************************/
|
||||||
@@ -1393,6 +1401,19 @@ uint16_t tcp_datahandler(FAR struct net_driver_s *dev,
|
|||||||
*
|
*
|
||||||
****************************************************************************/
|
****************************************************************************/
|
||||||
|
|
||||||
|
/****************************************************************************
|
||||||
|
* Name: tcp_callback_cleanup
|
||||||
|
*
|
||||||
|
* Description:
|
||||||
|
* Cleanup data and cb when thread is canceled.
|
||||||
|
*
|
||||||
|
* Input Parameters:
|
||||||
|
* arg - A pointer with conn and callback struct.
|
||||||
|
*
|
||||||
|
****************************************************************************/
|
||||||
|
|
||||||
|
void tcp_callback_cleanup(FAR void *arg);
|
||||||
|
|
||||||
#ifdef CONFIG_NET_TCPBACKLOG
|
#ifdef CONFIG_NET_TCPBACKLOG
|
||||||
int tcp_backlogcreate(FAR struct tcp_conn_s *conn, int nblg);
|
int tcp_backlogcreate(FAR struct tcp_conn_s *conn, int nblg);
|
||||||
#else
|
#else
|
||||||
|
|||||||
@@ -422,4 +422,35 @@ uint16_t tcp_datahandler(FAR struct net_driver_s *dev,
|
|||||||
return buflen;
|
return buflen;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/****************************************************************************
|
||||||
|
* Name: tcp_callback_cleanup
|
||||||
|
*
|
||||||
|
* Description:
|
||||||
|
* Cleanup data and cb when thread is canceled.
|
||||||
|
*
|
||||||
|
* Input Parameters:
|
||||||
|
* arg - A pointer with conn and callback struct.
|
||||||
|
*
|
||||||
|
****************************************************************************/
|
||||||
|
|
||||||
|
void tcp_callback_cleanup(FAR void *arg)
|
||||||
|
{
|
||||||
|
FAR struct tcp_callback_s *cb = (FAR struct tcp_callback_s *)arg;
|
||||||
|
|
||||||
|
nerr("ERROR: pthread is being canceled, need to cleanup cb\n");
|
||||||
|
|
||||||
|
tcp_callback_free(cb->tc_conn, cb->tc_cb);
|
||||||
|
if (cb->tc_sem)
|
||||||
|
{
|
||||||
|
nxsem_destroy(cb->tc_sem);
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Only connect canceled need to tcp_free */
|
||||||
|
|
||||||
|
if (cb->tc_free)
|
||||||
|
{
|
||||||
|
tcp_free(cb->tc_conn);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#endif /* NET_TCP_HAVE_STACK */
|
#endif /* NET_TCP_HAVE_STACK */
|
||||||
|
|||||||
@@ -37,6 +37,7 @@
|
|||||||
#include <arch/irq.h>
|
#include <arch/irq.h>
|
||||||
|
|
||||||
#include <nuttx/semaphore.h>
|
#include <nuttx/semaphore.h>
|
||||||
|
#include <nuttx/tls.h>
|
||||||
#include <nuttx/net/net.h>
|
#include <nuttx/net/net.h>
|
||||||
#include <nuttx/net/netdev.h>
|
#include <nuttx/net/netdev.h>
|
||||||
#include <nuttx/net/udp.h>
|
#include <nuttx/net/udp.h>
|
||||||
@@ -289,6 +290,7 @@ int psock_tcp_connect(FAR struct socket *psock,
|
|||||||
{
|
{
|
||||||
FAR struct tcp_conn_s *conn;
|
FAR struct tcp_conn_s *conn;
|
||||||
struct tcp_connect_s state;
|
struct tcp_connect_s state;
|
||||||
|
struct tcp_callback_s info;
|
||||||
int ret = OK;
|
int ret = OK;
|
||||||
|
|
||||||
/* Interrupts must be disabled through all of the following because
|
/* Interrupts must be disabled through all of the following because
|
||||||
@@ -306,6 +308,11 @@ int psock_tcp_connect(FAR struct socket *psock,
|
|||||||
{
|
{
|
||||||
ret = -EINVAL;
|
ret = -EINVAL;
|
||||||
}
|
}
|
||||||
|
else if (conn->tcpstateflags == TCP_CLOSED)
|
||||||
|
{
|
||||||
|
nerr("ERROR: tcp conn was released, connect failed \n");
|
||||||
|
ret = -ENOTCONN;
|
||||||
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
/* Perform the TCP connection operation */
|
/* Perform the TCP connection operation */
|
||||||
@@ -361,6 +368,16 @@ int psock_tcp_connect(FAR struct socket *psock,
|
|||||||
ret = psock_setup_callbacks(psock, &state);
|
ret = psock_setup_callbacks(psock, &state);
|
||||||
if (ret >= 0)
|
if (ret >= 0)
|
||||||
{
|
{
|
||||||
|
/* Push a cancellation point onto the stack. This will be
|
||||||
|
* called if the thread is canceled.
|
||||||
|
*/
|
||||||
|
|
||||||
|
info.tc_conn = conn;
|
||||||
|
info.tc_cb = state.tc_cb;
|
||||||
|
info.tc_sem = &state.tc_sem;
|
||||||
|
info.tc_free = true;
|
||||||
|
tls_cleanup_push(tls_get_info(), tcp_callback_cleanup, &info);
|
||||||
|
|
||||||
/* Wait for either the connect to complete or for an
|
/* Wait for either the connect to complete or for an
|
||||||
* error/timeout to occur.
|
* error/timeout to occur.
|
||||||
* NOTES: net_sem_wait will also terminate if a
|
* NOTES: net_sem_wait will also terminate if a
|
||||||
@@ -369,6 +386,8 @@ int psock_tcp_connect(FAR struct socket *psock,
|
|||||||
|
|
||||||
ret = net_sem_wait(&state.tc_sem);
|
ret = net_sem_wait(&state.tc_sem);
|
||||||
|
|
||||||
|
tls_cleanup_pop(tls_get_info(), 0);
|
||||||
|
|
||||||
/* Uninitialize the state structure */
|
/* Uninitialize the state structure */
|
||||||
|
|
||||||
nxsem_destroy(&state.tc_sem);
|
nxsem_destroy(&state.tc_sem);
|
||||||
|
|||||||
@@ -33,6 +33,7 @@
|
|||||||
#include <assert.h>
|
#include <assert.h>
|
||||||
|
|
||||||
#include <nuttx/semaphore.h>
|
#include <nuttx/semaphore.h>
|
||||||
|
#include <nuttx/tls.h>
|
||||||
#include <nuttx/net/net.h>
|
#include <nuttx/net/net.h>
|
||||||
#include <nuttx/mm/iob.h>
|
#include <nuttx/mm/iob.h>
|
||||||
#include <nuttx/net/netdev.h>
|
#include <nuttx/net/netdev.h>
|
||||||
@@ -662,6 +663,7 @@ ssize_t psock_tcp_recvfrom(FAR struct socket *psock, FAR struct msghdr *msg,
|
|||||||
size_t len = msg->msg_iov->iov_len;
|
size_t len = msg->msg_iov->iov_len;
|
||||||
struct tcp_recvfrom_s state;
|
struct tcp_recvfrom_s state;
|
||||||
FAR struct tcp_conn_s *conn;
|
FAR struct tcp_conn_s *conn;
|
||||||
|
struct tcp_callback_s info;
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
net_lock();
|
net_lock();
|
||||||
@@ -769,6 +771,16 @@ ssize_t psock_tcp_recvfrom(FAR struct socket *psock, FAR struct msghdr *msg,
|
|||||||
state.ir_cb->priv = (FAR void *)&state;
|
state.ir_cb->priv = (FAR void *)&state;
|
||||||
state.ir_cb->event = tcp_recvhandler;
|
state.ir_cb->event = tcp_recvhandler;
|
||||||
|
|
||||||
|
/* Push a cancellation point onto the stack. This will be
|
||||||
|
* called if the thread is canceled.
|
||||||
|
*/
|
||||||
|
|
||||||
|
info.tc_conn = conn;
|
||||||
|
info.tc_cb = state.ir_cb;
|
||||||
|
info.tc_sem = &state.ir_sem;
|
||||||
|
info.tc_free = false;
|
||||||
|
tls_cleanup_push(tls_get_info(), tcp_callback_cleanup, &info);
|
||||||
|
|
||||||
/* Wait for either the receive to complete or for an error/timeout
|
/* Wait for either the receive to complete or for an error/timeout
|
||||||
* to occur. net_sem_timedwait will also terminate if a signal is
|
* to occur. net_sem_timedwait will also terminate if a signal is
|
||||||
* received.
|
* received.
|
||||||
@@ -776,6 +788,7 @@ ssize_t psock_tcp_recvfrom(FAR struct socket *psock, FAR struct msghdr *msg,
|
|||||||
|
|
||||||
ret = net_sem_timedwait(&state.ir_sem,
|
ret = net_sem_timedwait(&state.ir_sem,
|
||||||
_SO_TIMEOUT(conn->sconn.s_rcvtimeo));
|
_SO_TIMEOUT(conn->sconn.s_rcvtimeo));
|
||||||
|
tls_cleanup_pop(tls_get_info(), 0);
|
||||||
if (ret == -ETIMEDOUT)
|
if (ret == -ETIMEDOUT)
|
||||||
{
|
{
|
||||||
ret = -EAGAIN;
|
ret = -EAGAIN;
|
||||||
|
|||||||
@@ -49,6 +49,7 @@
|
|||||||
#include <debug.h>
|
#include <debug.h>
|
||||||
|
|
||||||
#include <arch/irq.h>
|
#include <arch/irq.h>
|
||||||
|
#include <nuttx/tls.h>
|
||||||
#include <nuttx/net/net.h>
|
#include <nuttx/net/net.h>
|
||||||
#include <nuttx/mm/iob.h>
|
#include <nuttx/mm/iob.h>
|
||||||
#include <nuttx/net/netdev.h>
|
#include <nuttx/net/netdev.h>
|
||||||
@@ -1431,14 +1432,27 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
|
|||||||
|
|
||||||
while (tcp_wrbuffer_inqueue_size(conn) >= conn->snd_bufs)
|
while (tcp_wrbuffer_inqueue_size(conn) >= conn->snd_bufs)
|
||||||
{
|
{
|
||||||
|
struct tcp_callback_s info;
|
||||||
|
|
||||||
if (nonblock)
|
if (nonblock)
|
||||||
{
|
{
|
||||||
ret = -EAGAIN;
|
ret = -EAGAIN;
|
||||||
goto errout_with_lock;
|
goto errout_with_lock;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Push a cancellation point onto the stack. This will be
|
||||||
|
* called if the thread is canceled.
|
||||||
|
*/
|
||||||
|
|
||||||
|
info.tc_conn = conn;
|
||||||
|
info.tc_cb = conn->sndcb;
|
||||||
|
info.tc_sem = &conn->snd_sem;
|
||||||
|
info.tc_free = false;
|
||||||
|
tls_cleanup_push(tls_get_info(), tcp_callback_cleanup, &info);
|
||||||
|
|
||||||
ret = net_sem_timedwait_uninterruptible(&conn->snd_sem,
|
ret = net_sem_timedwait_uninterruptible(&conn->snd_sem,
|
||||||
tcp_send_gettimeout(start, timeout));
|
tcp_send_gettimeout(start, timeout));
|
||||||
|
tls_cleanup_pop(tls_get_info(), 0);
|
||||||
if (ret < 0)
|
if (ret < 0)
|
||||||
{
|
{
|
||||||
if (ret == -ETIMEDOUT)
|
if (ret == -ETIMEDOUT)
|
||||||
|
|||||||
@@ -42,6 +42,7 @@
|
|||||||
#include <arch/irq.h>
|
#include <arch/irq.h>
|
||||||
|
|
||||||
#include <nuttx/semaphore.h>
|
#include <nuttx/semaphore.h>
|
||||||
|
#include <nuttx/tls.h>
|
||||||
#include <nuttx/net/net.h>
|
#include <nuttx/net/net.h>
|
||||||
#include <nuttx/net/netdev.h>
|
#include <nuttx/net/netdev.h>
|
||||||
#include <nuttx/net/tcp.h>
|
#include <nuttx/net/tcp.h>
|
||||||
@@ -481,6 +482,7 @@ ssize_t psock_tcp_send(FAR struct socket *psock,
|
|||||||
FAR const void *buf, size_t len, int flags)
|
FAR const void *buf, size_t len, int flags)
|
||||||
{
|
{
|
||||||
FAR struct tcp_conn_s *conn;
|
FAR struct tcp_conn_s *conn;
|
||||||
|
struct tcp_callback_s info;
|
||||||
struct send_s state;
|
struct send_s state;
|
||||||
int ret = OK;
|
int ret = OK;
|
||||||
|
|
||||||
@@ -604,8 +606,19 @@ ssize_t psock_tcp_send(FAR struct socket *psock,
|
|||||||
{
|
{
|
||||||
uint32_t acked = state.snd_acked;
|
uint32_t acked = state.snd_acked;
|
||||||
|
|
||||||
|
/* Push a cancellation point onto the stack. This will be
|
||||||
|
* called if the thread is canceled.
|
||||||
|
*/
|
||||||
|
|
||||||
|
info.tc_conn = conn;
|
||||||
|
info.tc_cb = state.snd_cb;
|
||||||
|
info.tc_sem = &state.snd_sem;
|
||||||
|
info.tc_free = false;
|
||||||
|
tls_cleanup_push(tls_get_info(), tcp_callback_cleanup, &info);
|
||||||
|
|
||||||
ret = net_sem_timedwait(&state.snd_sem,
|
ret = net_sem_timedwait(&state.snd_sem,
|
||||||
_SO_TIMEOUT(conn->sconn.s_sndtimeo));
|
_SO_TIMEOUT(conn->sconn.s_sndtimeo));
|
||||||
|
tls_cleanup_pop(tls_get_info(), 0);
|
||||||
if (ret != -ETIMEDOUT || acked == state.snd_acked)
|
if (ret != -ETIMEDOUT || acked == state.snd_acked)
|
||||||
{
|
{
|
||||||
if (ret == -ETIMEDOUT)
|
if (ret == -ETIMEDOUT)
|
||||||
|
|||||||
Reference in New Issue
Block a user