Hi Simon, I'd like to submit a patch to fix an EADDRINUSE issue when dnsmasq is running with --bind-dynamic (OPT_CLEVERBIND) and a network interface is removed and re-added while a TCP child process is still alive. == Bug description == When dnsmasq forks a child process to handle a TCP DNS query (do_tcp_connection) or a DNSSEC UDP-to-TCP fallback (swap_to_tcp), the child inherits all of the parent's listening socket file descriptors. These sockets are never used by the child, but the kernel keeps the underlying socket objects alive as long as any process holds a reference. If a network interface goes away and comes back while the child is still running (waiting for a slow upstream response, for example), the parent calls release_listener() which closes its own fd, then create_bound_listeners() tries to bind() a new socket to the same address:port. This bind() fails with EADDRINUSE because the child's inherited copy of the old listening socket still exists in the kernel. SO_REUSEADDR does not help here — it only permits binding over sockets in TIME_WAIT state, not over a socket that is still in LISTEN state (held open by the child). SO_REUSEPORT would allow the bind() to succeed, but it is not a suitable fix: with SO_REUSEPORT the kernel load-balances incoming connections across all sockets bound to the same address. The child process never calls accept() on the inherited listening socket, so any connection dispatched there is silently black-holed until the child exits. == When does this happen == The race window is open for the entire lifetime of the TCP child process (bounded by CHILD_LIFETIME). It is most likely to trigger when: 1. The upstream DNS server is slow or unresponsive, so the child process lives for a long time waiting for a reply. 2. The network environment is dynamic — interfaces are frequently removed and re-created (e.g. USB NIC unplug/replug, mobile data PDN reconnect, Android tethering toggle). Both conditions together (slow upstream + dynamic interfaces) make this reliably reproducible. == Fix == Close all inherited listener fds in the child immediately after fork, in both do_tcp_connection() and swap_to_tcp(). This mirrors the existing pattern of closing the netlink fd in the child. == How to test == A simple test using /proc/<pid>/fd to verify the fix: 1. Start a fake upstream that accepts connections but never replies: nc -l -k -s 127.0.0.1 -p 15354 & 2. Start dnsmasq with --bind-dynamic pointing to the fake upstream: dnsmasq --keep-in-foreground --no-resolv \ --server=127.0.0.1#15354 \ --listen-address=127.0.0.1 --port=15353 \ --log-queries --log-facility=- 3. Send a TCP query to trigger a fork: dig @127.0.0.1 -p 15353 +tcp example.com 4. Find the child process and inspect its file descriptors: PARENT=$(pgrep -f "dnsmasq.*15353" | head -1) CHILD=$(pgrep -P $PARENT) ls -l /proc/$PARENT/fd | grep socket ls -l /proc/$CHILD/fd | grep socket Without the fix: the child holds copies of all parent listening socket inodes (EADDRINUSE will occur on rebind). With the fix: the child only holds its own connection socket and the pipe to parent — no listening socket inodes are shared. --- src/dnsmasq.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/dnsmasq.c b/src/dnsmasq.c index adaa692..9af3e09 100644 --- a/src/dnsmasq.c +++ b/src/dnsmasq.c @@ -2015,7 +2015,8 @@ static void do_tcp_connection(struct listener *listener, time_t now, int slot) pid_t p; union mysockaddr tcp_addr; socklen_t tcp_len = sizeof(union mysockaddr); - struct server *s; + struct server *s; + struct listener *l; int flags, auth_dns = 0; struct in_addr netmask; int pipefd[2]; @@ -2186,6 +2187,16 @@ static void do_tcp_connection(struct listener *listener, time_t now, int slot) alarm(CHILD_LIFETIME); close(pipefd[0]); /* close read end in child. */ daemon->pipe_to_parent = pipefd[1]; + + /* 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); + } } /* The connected socket inherits non-blocking @@ -2232,6 +2243,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)) { @@ -2304,6 +2316,13 @@ int swap_to_tcp(struct frec *forward, time_t now, int status, struct dns_header daemon->forward_to_tcp = forward; daemon->header_to_tcp = header; daemon->plen_to_tcp = *plen; + + /* Close inherited listening sockets in the child process. */ + for (l = daemon->listeners; l; l = l->next) + { + if (l->fd != -1) close(l->fd); + if (l->tcpfd != -1) close(l->tcpfd); + } } } -- 2.25.1 The patch is attached below. Thanks Zhou Yuefu |
0001-Close-inherited-listening-sockets-in-forked-TCP-chil.patch
Description: Binary data
_______________________________________________ Dnsmasq-discuss mailing list [email protected] https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
