sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

net/udp: Resolve race condition in connection-less UDP sockets with read-ahead buffering.

Browse Source 
In connection-mode UDP sockets, a remote address is retained in the UDP connection structure.  This determines both there send() will send the packets and which packets recv() will accept.

This same mechanism is used for connection-less UDP sendto:  A temporary remote address is written into the connection structure to support the sendto() operation.  That address persists until the next recvfrom() when it is reset to accept any address.

When UDP read-ahead buffering is enabled, however, that means that the old, invalid remote address can be left in the connection structure for some time.  This can cause read-ahead buffer to fail, dropping UDP packets.

Shortening the time between when he remote address is reset (i.e., immediately after the sendto() completes) is not a solution, that does not eliminate the race condition; in only makes it smaller.

With this change, a flag was added to the connection structure to indicate if the UDP socket is in connection mode or if it is connection-less.  This change effects only UDP receive operations:  The remote address in the UDP connection is always ignored if the UDP socket is not in connection-mode.

No for connection-mode sockets, that remote address behaves as before.  But for connection-less sockets, it is only used by sendto().
This commit is contained in:
Gregory Nutt 2018-05-13 09:57:34 -06:00
parent aec56484ab
commit fb8cf9373c
7 changed files with 189 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
net/inet/inet_recvfrom.c
View File

@ -1,7 +1,7 @@
/****************************************************************************
* net/inet/inet_recvfrom.c
*
* Copyright (C) 2007-2009, 2011-2017 Gregory Nutt. All rights reserved.
* Copyright (C) 2007-2009, 2011-2018 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -1206,27 +1206,9 @@ static ssize_t inet_udp_recvfrom(FAR struct socket *psock, FAR void *buf, size_t
net_lock();
inet_recvfrom_initialize(psock, buf, len, from, fromlen, &state);
/* If the UDP socket was previously assigned a remote peer address via
* connect(), then it is not necessary to use recvfrom() to obtain the
* address of the remote peer: UDP messages will be received only from
* the connected peer. Normally recv() would be used with such connected
* UDP sockets.
*/
if (!_SS_ISCONNECTED(psock->s_flags))
{
/* Setup the UDP remote connection to accept packets from any remote
* address.
*/
ret = udp_connect(conn, NULL);
if (ret < 0)
{
goto errout_with_state;
}
}
#ifdef CONFIG_NET_UDP_READAHEAD
/* Copy the read-ahead data from the packet */
inet_udp_readahead(&state);
/* The default return value is the number of bytes that we just copied
@ -1246,6 +1228,8 @@ static ssize_t inet_udp_recvfrom(FAR struct socket *psock, FAR void *buf, size_t
#endif
#ifdef CONFIG_NET_UDP_READAHEAD
/* Handle non-blocking UDP sockets */
if (_SS_ISNONBLOCK(psock->s_flags))
{
/* Return the number of bytes read from the read-ahead buffer if

8
net/inet/inet_sockif.c
View File

@ -1,7 +1,7 @@
/****************************************************************************
* net/inet/inet_sockif.c
*
* Copyright (C) 2017 Gregory Nutt. All rights reserved.
* Copyright (C) 2017-2018 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -695,6 +695,7 @@ static int inet_connect(FAR struct socket *psock,
#if defined(CONFIG_NET_UDP) && defined(NET_UDP_HAVE_STACK)
case SOCK_DGRAM:
{
FAR struct udp_conn_s *conn;
int ret;
/* We will accept connecting to a addr == NULL for disconnection.
@ -709,18 +710,21 @@ static int inet_connect(FAR struct socket *psock,
/* Perform the connect/disconnect operation */
ret = udp_connect(psock->s_conn, addr);
conn = (FAR struct udp_conn_s *)psock->s_conn;
ret = udp_connect(conn, addr);
if (ret < 0 || addr == NULL)
{
/* Failed to connect or explicitly disconnected */
psock->s_flags &= ~_SF_CONNECTED;
conn->flags &= ~_UDP_FLAG_CONNECTMODE;
}
else
{
/* Successfully connected */
psock->s_flags |= _SF_CONNECTED;
conn->flags |= _UDP_FLAG_CONNECTMODE;
}
return ret;

7
net/udp/udp.h
View File

@ -86,6 +86,12 @@
#define udp_callback_free(dev,conn,cb) \
devif_conn_callback_free((dev), (cb), &(conn)->list)
/* Definitions for the UDP connection struct flag field */
#define _UDP_FLAG_CONNECTMODE (1 << 0) /* Bit 0: UDP connection-mode */
#define _UDP_ISCONNECTMODE(f) (((f) & _UDP_FLAG_CONNECTMODE) != 0)
/****************************************************************************
* Public Type Definitions
****************************************************************************/
@ -101,6 +107,7 @@ struct udp_conn_s
union ip_binding_u u; /* IP address binding */
uint16_t lport; /* Bound local port number (network byte order) */
uint16_t rport; /* Remote port number (network byte order) */
uint8_t flags; /* See _UDP_FLAG_* definitions */
uint8_t domain; /* IP domain: PF_INET or PF_INET6 */
uint8_t ttl; /* Default time-to-live */
uint8_t crefs; /* Reference counts on this instance */

238
net/udp/udp_conn.c
View File

@ -272,35 +272,56 @@ static inline FAR struct udp_conn_s *
/* If the local UDP port is non-zero, the connection is considered
* to be used. If so, then the following checks are performed:
*
* - The local port number is checked against the destination port
* number in the received packet.
* - The remote port number is checked if the connection is bound
* to a remote port.
* - If multiple network interfaces are supported, then the local
* IP address is available and we will insist that the
* destination IP matches the bound address (or the destination
* IP address is a broadcast address). If a socket is bound to
* INADDRY_ANY (laddr), then it should receive all packets
* directed to the port.
* - Finally, if the connection is bound to a remote IP address,
* the source IP address of the packet is checked. Broadcast
* addresses are also accepted.
* 1. The destination address is verified against the bound address
* of the connection.
*
* - The local port number is checked against the destination port
* number in the received packet.
* - If multiple network interfaces are supported, then the local
* IP address is available and we will insist that the
* destination IP matches the bound address (or the destination
* IP address is a broadcast address). If a socket is bound to
* INADDRY_ANY (laddr), then it should receive all packets
* directed to the port.
*
* 2. If this is a connection mode UDP socket, then the source address
* is verified against the connected remote address.
*
* - The remote port number is checked if the connection is bound
* to a remote port.
* - Finally, if the connection is bound to a remote IP address,
* the source IP address of the packet is checked. Broadcast
* addresses are also accepted.
*
* If all of the above are true then the newly received UDP packet
* is destined for this UDP connection.
*
* To send and receive broadcast packets, the application should:
* To send and receive multicast packets, the application should:
*
* - Bind socket to INADDR_ANY
* - setsockopt to SO_BROADCAST
* - call sendto with sendaddr.sin_addr.s_addr = <broadcast-address>
* - call recvfrom.
* - Bind socket to INADDR6_ANY (for the all-nodes multicast address)
* or to a specific <multicast-address>
* - setsockopt to SO_BROADCAST (for all-nodes address)
*
* For connection-less UDP sockets:
*
* - call sendto with sendaddr.sin_addr.s_addr = <multicast-address>
* - call recvfrom.
*
* For connection-mode UDP sockets:
*
* - call connect() to connect the UDP socket to a specific remote
* address, then
* - Call send() with no address address information
* - call recv() (from address information should not be needed)
*
* REVIST: SO_BROADCAST flag is currently ignored.
*/
/* Check that there is a local port number and this is matches
* the port number in the destination address.
*/
if (conn->lport != 0 && udp->destport == conn->lport &&
(conn->rport == 0 || udp->srcport == conn->rport) &&
/* Local port accepts any address on this port or there
* is an exact match in destipaddr and the bound local
@ -309,22 +330,53 @@ static inline FAR struct udp_conn_s *
*/
(net_ipv4addr_cmp(conn->u.ipv4.laddr, INADDR_ANY) ||
net_ipv4addr_hdrcmp(ip->destipaddr, &conn->u.ipv4.laddr)) &&
/* If not connected to a remote address, or a broadcast address
* destipaddr was received, or there is an exact match between the
* srcipaddr and the bound IP address, then accept the packet.
net_ipv4addr_hdrcmp(ip->destipaddr, &conn->u.ipv4.laddr)))
{
/* Check if the socket is connection mode. In this case, only
* packets with source addresses from the connected remote peer
* will be accepted.
*/
(net_ipv4addr_cmp(conn->u.ipv4.raddr, INADDR_ANY) ||
#ifdef CONFIG_NET_BROADCAST
net_ipv4addr_hdrcmp(ip->destipaddr, &bcast) ||
#endif
net_ipv4addr_hdrcmp(ip->srcipaddr, &conn->u.ipv4.raddr)))
{
/* Matching connection found.. return a reference to it */
if (_UDP_ISCONNECTMODE(conn->flags))
{
/* Check if the UDP connection is either (1) accepting packets
* from any port or (2) the packet srcport matches the local
* bound port number.
*/
break;
if ((conn->rport == 0 || udp->srcport == conn->rport) &&
/* If (1) not connected to a remote address, or (2) a all-
* nodes multicast destipaddr was received, or (3) there is an
* exact match between the srcipaddr and the bound remote IP
* address, then accept the packet.
*/
(net_ipv4addr_cmp(conn->u.ipv4.raddr, INADDR_ANY) ||
#ifdef CONFIG_NET_BROADCAST
net_ipv4addr_hdrcmp(ip->destipaddr, &bcast) ||
#endif
net_ipv4addr_hdrcmp(ip->srcipaddr, &conn->u.ipv4.raddr)))
{
/* Matching connection found.. Break out of the loop and
* return this reference to it.
*/
break;
}
}
else
{
/* This UDP socket is not connected. We need to match only
* the destination address with the bound socket address.
* Break out out of the loop and return this reference to
* the matching connection structure.
*/
break;
}
}
/* Look at the next active connection */
@ -356,64 +408,112 @@ static inline FAR struct udp_conn_s *
FAR struct udp_conn_s *conn;
conn = (FAR struct udp_conn_s *)g_active_udp_connections.head;
while (conn)
while (conn != NULL)
{
/* If the local UDP port is non-zero, the connection is considered
* to be used. If so, then the following checks are performed:
*
* - The local port number is checked against the destination port
* number in the received packet.
* - The remote port number is checked if the connection is bound
* to a remote port.
* - If multiple network interfaces are supported, then the local
* IP address is available and we will insist that the
* destination IP matches the bound address. If a socket is bound to
* INADDR6_ANY (laddr), then it should receive all packets directed
* to the port. REVISIT: Should also depend on SO_BROADCAST.
* - Finally, if the connection is bound to a remote IP address,
* the source IP address of the packet is checked.
* 1. The destination address is verified against the bound address
* of the connection.
*
* - The local port number is checked against the destination port
* number in the received packet.
* - If multiple network interfaces are supported, then the local
* IP address is available and we will insist that the
* destination IP matches the bound address. If a socket is bound
* to INADDR6_ANY (laddr), then it should receive all packets
* directed to the port. REVISIT: Should also depend on SO_BROADCAST.
*
* 2. If this is a connection mode UDP socket, then the source address
* is verified against the connected remote address.
*
* - The remote port number is checked if the connection is bound
* to a remote port.
* - Finally, if the connection is bound to a remote IP address,
* the source IP address of the packet is checked.
*
* If all of the above are true then the newly received UDP packet
* is destined for this UDP connection.
*
* To send and receive multicast packets, the application should:
*
* - Bind socket to INADDR6_ANY (for the all-nodes multicast address)
* or to a specific <multicast-address>
* - setsockopt to SO_BROADCAST (for all-nodes address)
* - call sendto with sendaddr.sin_addr.s_addr = <multicast-address>
* - call recvfrom.
* - Bind socket to INADDR6_ANY (for the all-nodes multicast address)
* or to a specific <multicast-address>
* - setsockopt to SO_BROADCAST (for all-nodes address)
*
* For connection-less UDP sockets:
*
* - call sendto with sendaddr.sin_addr.s_addr = <multicast-address>
* - call recvfrom.
*
* For connection-mode UDP sockets:
*
* - call connect() to connect the UDP socket to a specific remote
* address, then
* - Call send() with no address address information
* - call recv() (from address information should not be needed)
*
* REVIST: SO_BROADCAST flag is currently ignored.
*/
if (conn->lport != 0 && udp->destport == conn->lport &&
(conn->rport == 0 || udp->srcport == conn->rport) &&
/* Check that there is a local port number and this is matches
* the port number in the destination address.
*/
/* Local port accepts any address on this port or there
* is an exact match in destipaddr and the bound local
* address. This catches the cast of the all nodes multicast
* when the socket is bound to INADDR6_ANY.
if ((conn->lport != 0 && udp->destport == conn->lport &&
/* Check if the local port accepts any address on this port or
* that there is an exact match between the destipaddr and the
* bound local address. This catches the case of the all nodes
* multicast when the socket is bound to INADDR6_ANY.
*/
(net_ipv6addr_cmp(conn->u.ipv6.laddr, g_ipv6_allzeroaddr) ||
net_ipv6addr_hdrcmp(ip->destipaddr, conn->u.ipv6.laddr)) &&
/* If not connected to a remote address, or a all-nodes multicast
* destipaddr was received, or there is an exact match between the
* srcipaddr and the bound remote IP address, then accept the
* packet.
net_ipv6addr_hdrcmp(ip->destipaddr, conn->u.ipv6.laddr))))
{
/* Check if the socket is connection mode. In this case, only
* packets with source addresses from the connected remote peer
* will be accepted.
*/
(net_ipv6addr_cmp(conn->u.ipv6.raddr, g_ipv6_allzeroaddr) ||
#ifdef CONFIG_NET_BROADCAST
net_ipv6addr_hdrcmp(ip->destipaddr, g_ipv6_allnodes) ||
#endif
net_ipv6addr_hdrcmp(ip->srcipaddr, conn->u.ipv6.raddr)))
{
/* Matching connection found.. return a reference to it */
if (_UDP_ISCONNECTMODE(conn->flags))
{
/* Check if the UDP connection is either (1) accepting packets
* from any port or (2) the packet srcport matches the local
* bound port number.
*/
break;
if ((conn->rport == 0 || udp->srcport == conn->rport) &&
/* If (1) not connected to a remote address, or (2) a all-
* nodes multicast destipaddr was received, or (3) there is an
* exact match between the srcipaddr and the bound remote IP
* address, then accept the packet.
*/
(net_ipv6addr_cmp(conn->u.ipv6.raddr, g_ipv6_allzeroaddr) ||
#ifdef CONFIG_NET_BROADCAST
net_ipv6addr_hdrcmp(ip->destipaddr, g_ipv6_allnodes) ||
#endif
net_ipv6addr_hdrcmp(ip->srcipaddr, conn->u.ipv6.raddr)))
{
/* Matching connection found.. Break out of the loop and
* return this reference to it.
*/
break;
}
}
else
{
/* This UDP socket is not connected. We need to match only
* the destination address with the bound socket address.
* Break out out of the loop and return this reference to
* the matching connection structure.
*/
break;
}
}
/* Look at the next active connection */

2
net/udp/udp_finddev.c
View File

@ -181,7 +181,7 @@ FAR struct net_driver_s *udp_find_laddr_device(FAR struct udp_conn_s *conn)
* remote IP address.
*
* Input Parameters:
* conn - UDP connection structure (not currently used).
* conn - UDP connection structure.
*
* Returned Value:
* A pointer to the network driver to use.

2
net/udp/udp_input.c
View File

@ -2,7 +2,7 @@
* net/udp/udp_input.c
* Handling incoming UDP input
*
* Copyright (C) 2007-2009, 2011 Gregory Nutt. All rights reserved.
* Copyright (C) 2007-2009, 2011, 2018 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Adapted for NuttX from logic in uIP which also has a BSD-like license:

18
net/udp/udp_netpoll.c
View File

@ -198,24 +198,6 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
info->dev = udp_find_laddr_device(conn);
/* Check if the UDP socket was previously assigned a remote peer address
* via connect(). In that case, we will only be interested in poll
* related events associated with that address.
*/
if (!_SS_ISCONNECTED(psock->s_flags))
{
/* Setup the UDP remote connection to accept packets from any remote
* address.
*/
ret = udp_connect(conn, NULL);
if (ret)
{
goto errout_with_lock;
}
}
/* Allocate a UDP callback structure */
cb = udp_callback_alloc(info->dev, conn);