Subject: [PATCH] proxy DHCP: busy loop when a proxy dhcp-range has a tag

Hello Simon,

dnsmasq can be put into a busy loop by two PXE DHCPDISCOVERs from any client
on a network with a tagged proxy range. dnsmasq then stops answering DHCP,
TFTP and DNS until it is restarted. Reproduced on 2.91 (Debian 13,
2.91-1+deb13u2) and on 2.93 built from the release tarball; a one-line patch
is below.

Configuration (the relevant part):

    dhcp-range=set:prov,192.0.2.0,proxy,255.255.255.0
    pxe-service=tag:prov,x86PC,"boot",pxelinux
    log-dhcp

After the second PXE request dnsmasq sits at 100% CPU, makes no system
calls, and its sockets fill up. gdb on the stuck process:

    #0  __strcmp_sse42 ()
    #1  log_tags (netid=..., xid=...) at dhcp-common.c:276
    #2  dhcp_reply (...) at rfc2131.c:1028      (proxy branch)
    #3  dhcp_packet (...) at dhcp.c:340
    #4  main (...) at dnsmasq.c:1299

The tag list that log_tags() walks is circular. Walking it in gdb on two
occasions:

    prov -> eth1 -> eth1 -> eth1 -> ...           (iface tag points at itself)
    prov -> known -> bios -> eth1 -> known -> ... (with dhcp-host and dhcp-match)

Cause, as far as I can see: in the proxy branch of dhcp_reply(),

    tagif_netid = option_filter(netid, tmp->netid.net ? &tmp->netid : NULL, ...);

option_filter() walks the context tags to their end and appends the request's
tags:

    for (last_tag = context_tags; last_tag->next; last_tag = last_tag->next);
    last_tag->next = tags;

tmp->netid is part of the (static) dhcp_context, so after the first request
tmp->netid.next still points at the previous request's iface_id, a stack
variable of dhcp_reply(). The next request has its iface_id at the same
address with next == NULL, so the walk stops on it and sets
iface_id.next = &iface_id. The non-proxy path does not have this problem
because it assigns context->netid.next before use.

It is not only log_tags(): without log-dhcp, any walk that looks for a tag
that is not in the list never ends. With

    dhcp-option=tag:nosuch,option:ntp-server,192.0.2.9

added (and log-dhcp removed) 2.93 loops the same way.

The attached repro.sh shows it in a few seconds (Linux, root): it creates two
network namespaces joined by a veth pair, runs the given dnsmasq binary with
the configuration above in one and sends two PXE DISCOVERs from the other:

    stock 2.93:            dnsmasq CPU over 5 s: 100%   RESULT: BUSY LOOP
    2.93 + patch:          dnsmasq CPU over 5 s: 0%     RESULT: ok
    stock 2.93, no log-dhcp, dhcp-option for an absent tag:  69%  BUSY LOOP
    2.93 + patch, same:    0%  ok

We found it on a PXE provisioning system whose rack controllers run dnsmasq
in proxy mode next to a site's own DHCP server; the clients were ordinary
iPXE ROMs. Our generated configurations now leave the tag off proxy ranges.

The patch resets the context tag before option_filter() appends to it, which
is what the non-proxy path effectively does:

--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -977,7 +977,12 @@
 		  int redirect4011 = 0;
 		  struct dhcp_opt *option;

-		  /* OK only dhcp-option-pxe options. */
+		  /* OK only dhcp-option-pxe options.
+		     option_filter() appends netid to the end of the context's tag list.
+		     tmp->netid is static and still links to the previous request's
+		     (stack) tags here; start it afresh, as the non-proxy path does,
+		     or the list can become circular and the tag walks never end. */
+		  tmp->netid.next = NULL;
 		  tagif_netid = option_filter(netid, tmp->netid.net ? &tmp->netid : NULL, daemon->dhcp_opts, 2);
 		  boot = find_boot(tagif_netid);


option_filter() might be the better place for a general fix (the DHCPv6 path
in rfc3315.c passes state->context_tags the same way), but I have only
tested the proxy case.

Since a single client can stop dnsmasq on the whole segment, you may want to
treat this as a denial-of-service issue; I have not posted it anywhere else.

Regards,
<name>
