Close inherited listening sockets in forked TCP child processes

When dnsmasq forks a child to handle a TCP connection, the child
inherits copies of all listening sockets.  These are never used but
keep the underlying sockets alive in the kernel.  If a network
interface is removed and re-added while a child is running, the
parent's attempt to re-bind fails with EADDRINUSE because the child
still holds a reference.

Close all listener fds (UDP and TCP) in the child immediately after
fork in both do_tcp_connection() and swap_to_tcp().

[Original patch extended by Simon Kelley to include the TFTP listening
socket, and to extend the existing race-protection scheme for the
netlink socket to the listening sockets. Any bugs are my responsibility.]

src/dnsmasq.c

@@ -2016,6 +2016,7 @@ static void do_tcp_connection(struct listener *listener, time_t now, int slot)
   union mysockaddr tcp_addr;
   socklen_t tcp_len = sizeof(union mysockaddr);
   struct server *s;
+  struct listener *l;
   int flags, auth_dns = 0;
   struct in_addr netmask;
   int pipefd[2];
@@ -2138,24 +2139,18 @@ static void do_tcp_connection(struct listener *listener, time_t now, int slot)
 	  /* fork() done: parent side */
 	  close(pipefd[1]); /* parent needs read pipe end. */
       
-#ifdef HAVE_LINUX_NETWORK
+	  /* The child process inherits various sockets,
-	  /* The child process inherits the netlink socket, 
+	     whose existence can mess up the function of
-	     which it never uses, but when the parent (us) 
+	     the parent. See below for details of which
-	     uses it in the future, the answer may go to the 
+	     sockets, and why they cause problems. The
-	     child, resulting in the parent blocking
+	     child closes the sockets during startup,
-	     forever awaiting the result. To avoid this
+	     but there's a nasty race, since the parent
-	     the child closes the netlink socket, but there's
+	     may use them before the child has done the close.
-	     a nasty race, since the parent may use netlink
-	     before the child has done the close.
 	     
-	     To avoid this, the parent blocks here until a 
+	     To avoid the race, the parent blocks here until a 
 	     single byte comes back up the pipe, which
-	     is sent by the child after it has closed the
+	     is sent by the child has finshed the close. */
-	     netlink socket. */
-
 	  read_write(pipefd[0], &a, 1, RW_READ);
-#endif
-	  
 	  
 	  daemon->tcp_pids[slot] = p;
 	  daemon->tcp_pipes[slot] = pipefd[0];
@@ -2176,16 +2171,39 @@ static void do_tcp_connection(struct listener *listener, time_t now, int slot)
 	  return;
 	}
       
+#ifdef HAVE_LINUX_NETWORK
+      /* The child process inherits the netlink socket, 
+	 which it never uses, but when the parent
+	 uses it in the future, the answer may go to the 
+	 child, resulting in the parent blocking
+	 forever awaiting the result. To avoid this
+	 close the netlink socket in the child. */
+      close(daemon->netlinkfd);
+#endif
+
+      /* Close inherited listening sockets in the child process.
+         These are not needed here and holding them prevents the
+         parent from re-binding if an interface is removed and
+         re-added (the child's copy causes EADDRINUSE). */
+      for (l = daemon->listeners; l; l = l->next)
+        {
+          if (l->fd != -1)
+	    close(l->fd);
+          if (l->tcpfd != -1)
+	    close(l->tcpfd);
+	  if (l->tftpfd != -1)
+	    close(l->tftpfd);
+        }
+
+      /* unblock parent now we're not holding sockets we shouldn't. */
+      read_write(pipefd[1], &a, 1, RW_WRITE);
+		  
       /* Arrange for SIGALRM after CHILD_LIFETIME seconds to
 	 terminate the process. */
-#ifdef HAVE_LINUX_NETWORK
-      /* See comment above re: netlink socket. */
-      close(daemon->netlinkfd);
-      read_write(pipefd[1], &a, 1, RW_WRITE);
-#endif		  
       alarm(CHILD_LIFETIME);
       close(pipefd[0]); /* close read end in child. */
       daemon->pipe_to_parent = pipefd[1];
+
     }
   
   /* The connected socket inherits non-blocking
@@ -2232,6 +2250,7 @@ int swap_to_tcp(struct frec *forward, time_t now, int status, struct dns_header
 		ssize_t *plen, char *name, int class, struct server *server, int *keycount, int *validatecount)
 {
   struct server *s;
+  struct listener *l;
 
   if (!option_bool(OPT_DEBUG))
     {
@@ -2260,22 +2279,18 @@ int swap_to_tcp(struct frec *forward, time_t now, int status, struct dns_header
 	      return STAT_ABANDONED;
 	    }
 
-#ifdef HAVE_LINUX_NETWORK
+	  /* The child process inherits various sockets,
-	  /* The child process inherits the netlink socket, 
+	     whose existence can mess up the function of
-	     which it never uses, but when the parent (us) 
+	     the parent. See below for details of which
-	     uses it in the future, the answer may go to the 
+	     sockets, and why they cause problems. The
-	     child, resulting in the parent blocking
+	     child closes the sockets during startup,
-	     forever awaiting the result. To avoid this
+	     but there's a nasty race, since the parent
-	     the child closes the netlink socket, but there's
+	     may use them before the child has done the close.
-	     a nasty race, since the parent may use netlink
-	     before the child has done the close.
 	     
-	     To avoid this, the parent blocks here until a 
+	     To avoid the race, the parent blocks here until a 
 	     single byte comes back up the pipe, which
-	     is sent by the child after it has closed the
+	     is sent by the child has finshed the close. */
-	     netlink socket. */
 	  read_write(pipefd[0], &a, 1, RW_READ);
-#endif
 	  
 	  /* i holds index of free slot */
 	  daemon->tcp_pids[i] = p;
@@ -2294,10 +2309,32 @@ int swap_to_tcp(struct frec *forward, time_t now, int status, struct dns_header
 	{
 	  /* child starts here. */
 #ifdef HAVE_LINUX_NETWORK
-	  /* See comment above re: netlink socket. */
+	  /* The child process inherits the netlink socket, 
+	     which it never uses, but when the parent
+	     uses it in the future, the answer may go to the 
+	     child, resulting in the parent blocking
+	     forever awaiting the result. To avoid this
+	     close the netlink socket in the child. */
 	  close(daemon->netlinkfd);
-	  read_write(pipefd[1], &a, 1, RW_WRITE);
 #endif
+	  
+	  /* Close inherited listening sockets in the child process.
+	     These are not needed here and holding them prevents the
+	     parent from re-binding if an interface is removed and
+	     re-added (the child's copy causes EADDRINUSE). */
+	  for (l = daemon->listeners; l; l = l->next)
+	    {
+	      if (l->fd != -1)
+		close(l->fd);
+	      if (l->tcpfd != -1)
+		close(l->tcpfd);
+	      if (l->tftpfd != -1)
+		close(l->tftpfd);
+	    }
+	  
+	  /* unblock parent now we're not holding sockets we shouldn't. */
+	  read_write(pipefd[1], &a, 1, RW_WRITE);
+	
 	  alarm(CHILD_LIFETIME);
 	  close(pipefd[0]); /* close read end in child. */
 	  daemon->pipe_to_parent = pipefd[1];