ripd: fix race condition on input processing

In the early days of ripd, we supported running RIP on secondary IP
addresses. To do that, everytime we needed to send a multicast packet,
we would create a new temporary socket for each of the interface's
addresses and call bind() to change the source IP of the outgoing packets.

The problem with these temporary sockets is that they are more specific
than the global RIP socket (bound to INADDR_ANY). Then, even though these
sockets only exist for a short amount of time, they can receive some RIP
packets that were supposed to be received on the global RIP socket. And
since we never read from the temporary sockets, these packets are dropped.

Since we don't support secondary addresses anymore, the simplest way to
fix this problem is to stop using temporary sockets for sending multicast
packets. We are already setting IP_MULTICAST_IF before sending each
multicast packet, and in this case the primary address of the selected
interface is used as the source IP of the outgoing packets, which is
exactly what we want.

If we decide to reintroduce support for secondary addresses in the future,
we should try one of the following:
* Use IP_SENDSRCADDR/IP_PKTINFO to set the source address of the outgoing
  multicast packets;
* Create one permanent UDP socket for each possible interface address,
  and enable reading on all sockets.

Fixes the following IxANVL RIP tests: 7.10 and 14.1.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

diff --git a/ripd/ripd.c b/ripd/ripd.c
index 79de56b78121147030258f27893c53cfe6c952da..a83c50c7ce3a637d84cf2e09a8e96bc9bcd93402 100644 (file)
--- a/ripd/ripd.c
+++ b/ripd/ripd.c
@@ -1339,25 +1339,18 @@ rip_response_process (struct rip_packet *packet, int size,
 
 /* Make socket for RIP protocol. */
 static int 
-rip_create_socket (struct sockaddr_in *from)
+rip_create_socket (void)
 {
   int ret;
   int sock;
   struct sockaddr_in addr;
   
   memset (&addr, 0, sizeof (struct sockaddr_in));
-  
-  if (!from)
-    {
-      addr.sin_family = AF_INET;
-      addr.sin_addr.s_addr = INADDR_ANY;
+  addr.sin_family = AF_INET;
+  addr.sin_addr.s_addr = INADDR_ANY;
 #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
-      addr.sin_len = sizeof (struct sockaddr_in);
+  addr.sin_len = sizeof (struct sockaddr_in);
 #endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */
-    } else {
-      memcpy(&addr, from, sizeof(addr));
-    }
-  
   /* sending port must always be the RIP port */
   addr.sin_port = htons (RIP_PORT_DEFAULT);
   
@@ -1412,7 +1405,7 @@ static int
 rip_send_packet (u_char * buf, int size, struct sockaddr_in *to,
                  struct connected *ifc)
 {
-  int ret, send_sock;
+  int ret;
   struct sockaddr_in sin;
   
   assert (ifc != NULL);
@@ -1468,38 +1461,16 @@ rip_send_packet (u_char * buf, int size, struct sockaddr_in *to,
     {
       sin.sin_port = to->sin_port;
       sin.sin_addr = to->sin_addr;
-      send_sock = rip->sock;
     }
   else
     {
-      struct sockaddr_in from;
-      
       sin.sin_port = htons (RIP_PORT_DEFAULT);
       sin.sin_addr.s_addr = htonl (INADDR_RIP_GROUP);
-      
-      /* multicast send should bind to local interface address */
-      memset (&from, 0, sizeof (from));
-      from.sin_family = AF_INET;
-      from.sin_port = htons (RIP_PORT_DEFAULT);
-      from.sin_addr = ifc->address->u.prefix4;
-#ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
-      from.sin_len = sizeof (struct sockaddr_in);
-#endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */
-      
-      /*
-       * we have to open a new socket for each packet because this
-       * is the most portable way to bind to a different source
-       * ipv4 address for each packet. 
-       */
-      if ( (send_sock = rip_create_socket (&from)) < 0)
-        {
-          zlog_warn("rip_send_packet could not create socket.");
-          return -1;
-        }
-      rip_interface_multicast_set (send_sock, ifc);
+
+      rip_interface_multicast_set (rip->sock, ifc);
     }
 
-  ret = sendto (send_sock, buf, size, 0, (struct sockaddr *)&sin,
+  ret = sendto (rip->sock, buf, size, 0, (struct sockaddr *)&sin,
                sizeof (struct sockaddr_in));
 
   if (IS_RIP_DEBUG_EVENT)
@@ -1509,9 +1480,6 @@ rip_send_packet (u_char * buf, int size, struct sockaddr_in *to,
   if (ret < 0)
     zlog_warn ("can't send packet : %s", safe_strerror (errno));
 
-  if (!to)
-    close(send_sock);
-
   return ret;
 }
 
@@ -2729,7 +2697,7 @@ rip_create (void)
   rip->obuf = stream_new (1500);
 
   /* Make socket. */
-  rip->sock = rip_create_socket (NULL);
+  rip->sock = rip_create_socket ();
   if (rip->sock < 0)
     return rip->sock;