Networking: Fix a race condition. The accept() operation is performed with the network locked. However, the network is unlocked BEFORE the connected state is set. Therefore, a context switch may occur and the socket may no longer be connected when it is marked so. Noted by Pascal Speck.

This commit is contained in:
Gregory Nutt 2017-08-31 07:23:19 -06:00
parent 9d3b1af1cd
commit 7ebef900fb
8 changed files with 28 additions and 13 deletions

View File

@ -313,6 +313,9 @@ static int ieee802154_connect(FAR struct socket *psock,
* Returns 0 (OK) on success. On failure, it returns a negated errno * Returns 0 (OK) on success. On failure, it returns a negated errno
* value. See accept() for a desrciption of the approriate error value. * value. See accept() for a desrciption of the approriate error value.
* *
* Assumptions:
* The network is locked.
*
****************************************************************************/ ****************************************************************************/
static int ieee802154_accept(FAR struct socket *psock, static int ieee802154_accept(FAR struct socket *psock,

View File

@ -741,6 +741,9 @@ static int inet_connect(FAR struct socket *psock,
* Returns 0 (OK) on success. On failure, it returns a negated errno * Returns 0 (OK) on success. On failure, it returns a negated errno
* value. See accept() for a desrciption of the approriate error value. * value. See accept() for a desrciption of the approriate error value.
* *
* Assumptions:
* The network is locked.
*
****************************************************************************/ ****************************************************************************/
static int inet_accept(FAR struct socket *psock, FAR struct sockaddr *addr, static int inet_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
@ -810,9 +813,8 @@ static int inet_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
#ifdef CONFIG_NET_TCP #ifdef CONFIG_NET_TCP
#ifdef NET_TCP_HAVE_STACK #ifdef NET_TCP_HAVE_STACK
/* Perform the local accept operation (with the network locked) */ /* Perform the local accept operation (the network locked must be locked) */
net_lock();
ret = psock_tcp_accept(psock, addr, addrlen, &newsock->s_conn); ret = psock_tcp_accept(psock, addr, addrlen, &newsock->s_conn);
if (ret < 0) if (ret < 0)
{ {
@ -835,7 +837,6 @@ static int inet_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
goto errout_after_accept; goto errout_after_accept;
} }
net_unlock();
return OK; return OK;
errout_after_accept: errout_after_accept:

View File

@ -567,6 +567,9 @@ static int local_connect(FAR struct socket *psock,
* Returns 0 (OK) on success. On failure, it returns a negated errno * Returns 0 (OK) on success. On failure, it returns a negated errno
* value. See accept() for a desrciption of the approriate error value. * value. See accept() for a desrciption of the approriate error value.
* *
* Assumptions:
* The network is locked.
*
****************************************************************************/ ****************************************************************************/
#ifndef CONFIG_NET_LOCAL_STREAM #ifndef CONFIG_NET_LOCAL_STREAM

View File

@ -308,6 +308,9 @@ static int pkt_connect(FAR struct socket *psock,
* Returns 0 (OK) on success. On failure, it returns a negated errno * Returns 0 (OK) on success. On failure, it returns a negated errno
* value. See accept() for a desrciption of the approriate error value. * value. See accept() for a desrciption of the approriate error value.
* *
* Assumptions:
* The network is locked.
*
****************************************************************************/ ****************************************************************************/
static int pkt_accept(FAR struct socket *psock, FAR struct sockaddr *addr, static int pkt_accept(FAR struct socket *psock, FAR struct sockaddr *addr,

View File

@ -153,6 +153,8 @@ int psock_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
/* Let the address family's accept() method handle the operation */ /* Let the address family's accept() method handle the operation */
DEBUGASSERT(psock->s_sockif != NULL && psock->s_sockif->si_accept != NULL); DEBUGASSERT(psock->s_sockif != NULL && psock->s_sockif->si_accept != NULL);
net_lock();
ret = psock->s_sockif->si_accept(psock, addr, addrlen, newsock); ret = psock->s_sockif->si_accept(psock, addr, addrlen, newsock);
if (ret < 0) if (ret < 0)
{ {
@ -165,6 +167,7 @@ int psock_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
newsock->s_flags |= _SF_CONNECTED; newsock->s_flags |= _SF_CONNECTED;
newsock->s_flags &= ~_SF_CLOSED; newsock->s_flags &= ~_SF_CLOSED;
net_unlock();
leave_cancellation_point(); leave_cancellation_point();
return OK; return OK;

View File

@ -1074,7 +1074,7 @@ uint16_t tcp_datahandler(FAR struct tcp_conn_s *conn, FAR uint8_t *buffer,
* the listen arguments. * the listen arguments.
* *
* Assumptions: * Assumptions:
* Called from normal user code. Interrupts may be disabled. * Called from network socket logic. The network may or may not be locked.
* *
****************************************************************************/ ****************************************************************************/
@ -1095,7 +1095,7 @@ int tcp_backlogcreate(FAR struct tcp_conn_s *conn, int nblg);
* is freed that has pending connections. * is freed that has pending connections.
* *
* Assumptions: * Assumptions:
* Called from network stack logic with the network stack locked * Called from network socket logic with the network stack locked
* *
****************************************************************************/ ****************************************************************************/

View File

@ -64,7 +64,7 @@
* the listen arguments. * the listen arguments.
* *
* Assumptions: * Assumptions:
* Called from normal user code. Interrupts may be disabled. * Called from normal task logic. The network may or may not be locked.
* *
****************************************************************************/ ****************************************************************************/
@ -129,7 +129,7 @@ int tcp_backlogcreate(FAR struct tcp_conn_s *conn, int nblg)
/* Now install the backlog tear-off in the connection. NOTE that bls may /* Now install the backlog tear-off in the connection. NOTE that bls may
* actually be NULL if nblg is <= 0; In that case, we are disabling backlog * actually be NULL if nblg is <= 0; In that case, we are disabling backlog
* support. Since interrupts are disabled, destroying the old backlog and * support. Since the network is locked, destroying the old backlog and
* replace it with the new is an atomic operation * replace it with the new is an atomic operation
*/ */
@ -149,8 +149,7 @@ int tcp_backlogcreate(FAR struct tcp_conn_s *conn, int nblg)
* is freed that has pending connections. * is freed that has pending connections.
* *
* Assumptions: * Assumptions:
* The caller has disabled interrupts so that there can be no conflict * Called from network socket logic with the network locked
* with ongoing, interrupt driven activity
* *
****************************************************************************/ ****************************************************************************/
@ -211,7 +210,7 @@ int tcp_backlogdestroy(FAR struct tcp_conn_s *conn)
* function adds the new connection to the backlog. * function adds the new connection to the backlog.
* *
* Assumptions: * Assumptions:
* Called from the interrupt level with interrupts disabled * Called from network socket logic with the network locked
* *
****************************************************************************/ ****************************************************************************/
@ -264,7 +263,7 @@ int tcp_backlogadd(FAR struct tcp_conn_s *conn, FAR struct tcp_conn_s *blconn)
* call this API to see if there are pending connections in the backlog. * call this API to see if there are pending connections in the backlog.
* *
* Assumptions: * Assumptions:
* Called from normal user code, but with interrupts disabled, * Called from network socket logic with the network locked
* *
****************************************************************************/ ****************************************************************************/
@ -283,7 +282,7 @@ bool tcp_backlogavailable(FAR struct tcp_conn_s *conn)
* call this API to see if there are pending connections in the backlog. * call this API to see if there are pending connections in the backlog.
* *
* Assumptions: * Assumptions:
* Called from normal user code, but with interrupts disabled, * Called from network socket logic with the network locked
* *
****************************************************************************/ ****************************************************************************/
@ -333,7 +332,7 @@ FAR struct tcp_conn_s *tcp_backlogremove(FAR struct tcp_conn_s *conn)
* to remove the defunct connection from the list. * to remove the defunct connection from the list.
* *
* Assumptions: * Assumptions:
* Called from the interrupt level with interrupts disabled * Called from network socket logic with the network locked
* *
****************************************************************************/ ****************************************************************************/

View File

@ -297,6 +297,9 @@ int usrsock_sockif_listen(FAR struct socket *psock, int backlog)
* Returns 0 (OK) on success. On failure, it returns a negated errno * Returns 0 (OK) on success. On failure, it returns a negated errno
* value. See accept() for a desrciption of the approriate error value. * value. See accept() for a desrciption of the approriate error value.
* *
* Assumptions:
* The network is locked.
*
****************************************************************************/ ****************************************************************************/
static int usrsock_sockif_accept(FAR struct socket *psock, static int usrsock_sockif_accept(FAR struct socket *psock,