There was a reference counting problem in the TPC logic of net_clone(). net_clone() which is the common logic underlying dup() and dup2() for sockets. When net_clone() calls net_start_monitor() and net_start_monitor() returns a failure (because the underlying TCP connection) then net_clone() must back out the reference count on the structure. Problem noted by Pascal Speck and this implementation of the solution is based on his suggestion.

This commit is contained in:
Gregory Nutt 2017-10-19 11:09:23 -06:00
parent 5ef548677a
commit bc40403516
4 changed files with 54 additions and 18 deletions

View File

@ -585,7 +585,7 @@ FAR struct mtd_dev_s *blockmtd_initialize(FAR const char *path, size_t offset,
* Teardown a previously created blockmtd device. * Teardown a previously created blockmtd device.
* *
* Input Parameters: * Input Parameters:
* mtd - Pointer to the mtd. * dev - Pointer to the mtd driver instance.
* *
****************************************************************************/ ****************************************************************************/
@ -655,7 +655,7 @@ FAR struct mtd_dev_s *filemtd_initialize(FAR const char *path, size_t offset,
* Teardown a previously created filemtd device. * Teardown a previously created filemtd device.
* *
* Input Parameters: * Input Parameters:
* mtd - Pointer to the mtd. * dev - Pointer to the mtd driver instance.
* *
****************************************************************************/ ****************************************************************************/

View File

@ -564,7 +564,10 @@ FAR struct mtd_dev_s *n25qxxx_initialize(FAR struct qspi_dev_s *qspi,
* Name: blockmtd_initialize * Name: blockmtd_initialize
* *
* Description: * Description:
* Create a block device backed a MTD device. * Create and initialize a BLOCK MTD device instance.
*
* Input Parameters:
* path - Path name of the block device backing the MTD device
* *
****************************************************************************/ ****************************************************************************/
@ -576,7 +579,10 @@ FAR struct mtd_dev_s *blockmtd_initialize(FAR const char *path, size_t offset,
* Name: blockmtd_teardown * Name: blockmtd_teardown
* *
* Description: * Description:
* Tear down a blockmtd device. * Teardown a previously created blockmtd device.
*
* Input Parameters:
* dev - Pointer to the mtd driver instance.
* *
****************************************************************************/ ****************************************************************************/
@ -586,7 +592,10 @@ void blockmtd_teardown(FAR struct mtd_dev_s *dev);
* Name: filemtd_initialize * Name: filemtd_initialize
* *
* Description: * Description:
* Create a file backed MTD device. * Create and initialize a FILE MTD device instance.
*
* Input Parameters:
* path - Path name of the file backing the MTD device
* *
****************************************************************************/ ****************************************************************************/
@ -597,17 +606,23 @@ FAR struct mtd_dev_s *filemtd_initialize(FAR const char *path, size_t offset,
* Name: filemtd_teardown * Name: filemtd_teardown
* *
* Description: * Description:
* Tear down a filemtd device. * Teardown a previously created filemtd device.
*
* Input Parameters:
* dev - Pointer to the mtd driver instance.
* *
****************************************************************************/ ****************************************************************************/
void filemtd_teardown(FAR struct mtd_dev_s* mtd); void filemtd_teardown(FAR struct mtd_dev_s* dev);
/**************************************************************************** /****************************************************************************
* Name: filemtd_isfilemtd * Name: filemtd_isfilemtd
* *
* Description: * Description:
* Test if MTD is a filemtd device. * Tests if the provided mtd is a filemtd or blockmtd device.
*
* Input Parameters:
* mtd - Pointer to the mtd.
* *
****************************************************************************/ ****************************************************************************/

View File

@ -40,6 +40,7 @@
#include <nuttx/config.h> #include <nuttx/config.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <string.h>
#include <errno.h> #include <errno.h>
#include <debug.h> #include <debug.h>
@ -47,6 +48,7 @@
#include <nuttx/net/net.h> #include <nuttx/net/net.h>
#include <nuttx/net/udp.h> #include <nuttx/net/udp.h>
#include "inet/inet.h"
#include "tcp/tcp.h" #include "tcp/tcp.h"
#include "socket/socket.h" #include "socket/socket.h"
@ -81,7 +83,9 @@ int net_clone(FAR struct socket *psock1, FAR struct socket *psock2)
net_lock(); net_lock();
/* Duplicate the socket state */ /* Duplicate the relevant socket state (zeroing everything else) */
memset(psock2, 0, sizeof(struct socket));
psock2->s_domain = psock1->s_domain; /* IP domain: PF_INET, PF_INET6, or PF_PACKET */ psock2->s_domain = psock1->s_domain; /* IP domain: PF_INET, PF_INET6, or PF_PACKET */
psock2->s_type = psock1->s_type; /* Protocol type: Only SOCK_STREAM or SOCK_DGRAM */ psock2->s_type = psock1->s_type; /* Protocol type: Only SOCK_STREAM or SOCK_DGRAM */
@ -96,10 +100,6 @@ int net_clone(FAR struct socket *psock1, FAR struct socket *psock2)
#endif #endif
#endif #endif
psock2->s_conn = psock1->s_conn; /* UDP or TCP connection structure */ psock2->s_conn = psock1->s_conn; /* UDP or TCP connection structure */
#ifdef CONFIG_NET_TCP_WRITE_BUFFERS
psock2->s_sndcb = NULL; /* Force allocation of new callback
* instance for TCP send */
#endif
/* Increment the reference count on the socket */ /* Increment the reference count on the socket */
@ -113,14 +113,35 @@ int net_clone(FAR struct socket *psock1, FAR struct socket *psock2)
psock2->s_sockif->si_addref(psock2); psock2->s_sockif->si_addref(psock2);
#ifdef NET_TCP_HAVE_STACK #ifdef NET_TCP_HAVE_STACK
/* For connected socket types, it is necessary to also start the network monitor so /* For connected socket types, it is necessary to also start the network
* that the newly cloned socket can receive a notification if the network connection * monitor so that the newly cloned socket can receive a notification if
* is lost. * the network connection is lost.
*/ */
if (psock2->s_type == SOCK_STREAM) if (psock2->s_type == SOCK_STREAM)
{ {
ret = tcp_start_monitor(psock2); ret = tcp_start_monitor(psock2);
/* On failure, back out the reference count on the TCP connection
* structure. tcp_start_monitor() will fail only in the race condition
* where the TCP connection has been lost.
*/
if (ret < 0)
{
/* There should be at least two reference counts on the connection
* structure: At least one from the original socket and the one
* from above where we incremented the reference count.
* inet_close() will handle all cases.
*
* NOTE: As a side-effect, inet_close()will also call
* tcp_stop_monitor() which could inform the loss of connection to
* all open sockets on the connection structure if the reference
* count decrements to zero.
*/
(void)inet_close(psock2);
}
} }
#endif #endif

View File

@ -89,10 +89,10 @@ int dup2(int sockfd1, int sockfd2)
psock2 = sockfd_socket(sockfd2); psock2 = sockfd_socket(sockfd2);
/* Verify that the sockfd1 and sockfd2 both refer to valid socket /* Verify that the sockfd1 and sockfd2 both refer to valid socket
* descriptors and that sockfd2 corresponds to allocated socket * descriptors and that sockfd2 corresponds to an allocated socket
*/ */
if (!psock1 || !psock2 || psock1->s_crefs <= 0) if (psock1 == NULL || psock2 == NULL || psock1->s_crefs <= 0)
{ {
ret = -EBADF; ret = -EBADF;
goto errout; goto errout;