I think I've found a regression in dnsmasq 2.92 that breaks PXE boot server
(PXEBS) responses when running in proxy DHCP mode. Fair warning: I'm not
familiar with the dnsmasq codebase and used AI tooling to help trace through
the source and identify the issue, so please take the analysis below with
appropriate skepticism. PXE boot works fine on 2.91
but fails on 2.92 — the client gets the initial proxy DHCPOFFER, but the PXEBS
ACK on port 4011 never reaches it.
My setup is dnsmasq in proxy DHCP mode serving iPXE to Proxmox VMs via their
virtio-net ROM. Here's a stripped-down version of my config:
port=0
enable-tftp
tftp-root=/tftpboot
dhcp-range=172.19.74.0,proxy,255.255.255.0
interface=eno1
bind-interfaces
dhcp-match=set:ipxe,175
pxe-service=tag:ipxe,x86PC,"Network Boot",http://server:8081/boot.ipxe
pxe-service=tag:!ipxe,x86PC,"Network Boot",undionly.kpxe
log-dhcp
The issue seems to be in src/dhcp.c in the response routing logic after
dhcp_reply() returns. In 2.91, the destination selection was an if/else-if
chain:
if (pxe_fd)
{ ... }
else if (mess->giaddr.s_addr && !is_relay_reply)
{ ... }
else if (mess->ciaddr.s_addr)
{ ... }
else
{ ... broadcast to 255.255.255.255:68 ... }
In 2.92, the else between the pxe_fd block and the giaddr/relay check was
removed in commit 4fbe1ad ("Implement RFC-4388 DHCPv4 leasequery") to
accommodate the new is_relay_use_source logic:
if (pxe_fd)
{ ... }
if ((is_relay_use_source || mess->giaddr.s_addr) && !is_relay_reply)
{ ... }
else if (mess->ciaddr.s_addr)
{ ... }
else
{ ... broadcast to 255.255.255.255:68 ... }
For PXEBS responses, dhcp_reply() in rfc2131.c (around line 924-925) does:
mess->yiaddr = mess->ciaddr;
mess->ciaddr.s_addr = 0;
So after dhcp_reply() returns for a PXEBS request, ciaddr is 0, giaddr is 0
(no relay), and is_relay_use_source is 0. In 2.91, the pxe_fd block runs and
the rest of the chain is skipped — dest stays as received from recvmsg, and the
response goes back to the client correctly. In 2.92, the pxe_fd block runs but
then falls through to the standalone if, which is false, so the else block runs
and sets dest to 255.255.255.255 port 68. The client is listening on port 4011
and ignores it.
Here are the relevant dnsmasq logs. With 2.91 (working), I see normal proxy
DHCP and PXE boot server exchanges:
dnsmasq-dhcp: DHCPDISCOVER(eno1) bc:24:11:59:85:90
dnsmasq-dhcp: DHCPOFFER(eno1) 172.19.74.60 bc:24:11:59:85:90
dnsmasq-dhcp: DHCPREQUEST(eno1) 172.19.74.60 bc:24:11:59:85:90
dnsmasq-dhcp: DHCPACK(eno1) 172.19.74.60 bc:24:11:59:85:90
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 http://infra1.oneill.net:8081/boot.ipxe
With 2.92 (broken), the DHCPDISCOVER/OFFER/REQUEST/ACK cycle and the proxy
PXE response work, but the PXEBS response never reaches the client — it times
out after repeated attempts. The dnsmasq side shows it sending the response,
but the client keeps retrying:
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
I tested by restoring the else keyword and the fix appears to work — 2.92 with
the patch below PXE boots successfully. I believe this change preserves the
leasequery behavior since that path only applies when pxe_fd is false (normal
DHCP handling, not port 4011).
This was supposed to disallow addresses which are in use by
the DHCP server, and the code works fine when only dhcp-range
is defined per broadcast domain. It's confused when there are
multiple dhcp-ranges which are valid for a particular request.
When using PXE proxy-DHCP, dnsmasq supplies PXE information to
the client, which also talks to another "normal" DHCP server
for address allocation and similar. The normal DHCP server may
be on the local network, but it may also be remote, and accessed via
a DHCP relay. This change allows dnsmasq to act as both a
PXE proxy-DHCP server AND a DHCP relay for the same network.
A bug in gentoo linux https://bugs.gentoo.org/945183 reported that dnsmasq 2.90 fails to compile with GCC 15.
The issue is that while previous versions of GCC defaulted to the C17 standard and C23 could be selected with
"-std=c23" or "-std=gnu23", GCC 15 defaults to C23. In C23 incompatible pointer types are an error instead of
a warning, so the "int (*callback)()" incomplete prototypes cause errors.
For example, compiling dnsmasq 2.90 with gcc 14.2.1 and "-std=gnu23" fails with errors such as:
lease.c: In function `lease_find_interfaces':
lease.c:467:34: warning: passing argument 3 of `iface_enumerate' from incompatible pointer type [-Wincompatible-pointer-types[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wincompatible-pointer-types]]
467 | iface_enumerate(AF_INET, &now, find_interface_v4);
| ^~~~~~~~~~~~~~~~~
| |
| int (*)(struct in_addr, int, char *, struct in_addr, struct in_addr, void *)
In file included from lease.c:17:
dnsmasq.h:1662:50: note: expected `int (*)(void)' but argument is of type `int (*)(struct in_addr, int, char *, struct in_addr, struct in_addr, void *)'
1662 | int iface_enumerate(int family, void *parm, int (callback)());
| ~~~~~^~~~~~~~~~~
This patch uses a typedef'ed union of pointer types to get type checking of the pointers. If that's too complicated,
another way might be to use (void *) casts to disable type checking.
Also, some of the IPv6 callbacks had "int preferred, int valid" and some had
"unsigned int preferred, unsigned int valid". This patch changes them all to "unsigned int"
so they're the same and to avoid casting "u32" to "int", eg:
u32 preferred = 0xffffffff;
callback(..., (int)preferred, ...)
Even if those cast values aren't used in the callback, casting u32 to "int" feels bad, especially if "int" is 32 bits.
The msg_controllen field passed to sendmsg is computed using the
CMSG_SPACE(), which is correct, but CMSG_SPACE() can include
padding bytes at the end of the passed structure if they are required
to align subsequent structures in the buffer. Make sure these
bytes are zeroed to avoid passing uninitiased memory to the kernel,
even though it will never touch these bytes.
Also tidy up the mashalling code in send_from to use pointers to
the structure being filled out, rather than a temporary copy which
then gets memcpy()'d into place. The DHCP sendmsg code has always
worked like this.
Thanks to Dominik Derigs for running Memcheck as submitting the
initial patch.
As defined in the C standard:
In all cases the argument is an int, the value of which shall
be representable as an unsigned char or shall equal the value
of the macro EOF. If the argument has any other value, the
behavior is undefined.
This is because they're designed to work with the int values returned
by getc or fgetc; they need extra work to handle a char value.
If EOF is -1 (as it almost always is), with 8-bit bytes, the allowed
inputs to the ctype(3) functions are:
{-1, 0, 1, 2, 3, ..., 255}.
However, on platforms where char is signed, such as x86 with the
usual ABI, code like
char *arg = ...;
... isspace(*arg) ...
may pass in values in the range:
{-128, -127, -126, ..., -2, -1, 0, 1, ..., 127}.
This has two problems:
1. Inputs in the set {-128, -127, -126, ..., -2} are forbidden.
2. The non-EOF byte 0xff is conflated with the value EOF = -1, so
even though the input is not forbidden, it may give the wrong
answer.
Casting char to int first before passing the result to ctype(3)
doesn't help: inputs like -128 are unchanged by this cast. It is
necessary to cast char inputs to unsigned char first; you can then
cast to int if you like but there's no need because the functions
will always convert the argument to int by definition. So the above
fragment needs to be:
char *arg = ...;
... isspace((unsigned char)*arg) ...
This patch inserts unsigned char casts where necessary, and changes
int casts to unsigned char casts where the input is char.
I left alone int casts where the input is unsigned char already --
they're not immediately harmful, although they would have the effect
of suppressing some compiler warnings if the input is ever changed to
be char instead of unsigned char, so it might be better to remove
those casts too.
I also left alone calls where the input is int to begin with because
it came from getc; casting to unsigned char here would be wrong, of
course.
This change also removes a previous bug
where --dhcp-alternate-port would affect the port used
to relay _to_ as well as the port being listened on.
The new feature allows configuration to provide bug-for-bug
compatibility, if required. Thanks to Damian Kaczkowski
for the feature suggestion.
On machines with many interfaces, enumerating them
via netlink on each packet reciept is slow,
and unneccesary. All we need is the local address->interface
mapping, which can be cached in the relay structures.
Avoid offering the same address after a recieving a DECLINE message
to stop an infinite protocol loop. This has long been done in
default address allocation mode: this adds similar behaviour
when allocaing addresses consecutively.
Adds option to delay replying to DHCP packets by one or more seconds.
This provides a workaround for a PXE boot firmware implementation
that has a bug causing it to fail if it receives a (proxy) DHCP
reply instantly.
On Linux it looks up the exact receive time of the UDP packet with
the SIOCGSTAMP ioctl to prevent multiple delays if multiple packets
come in around the same time.
From f3d832b41f44c856003517c583fbd7af4dca722c Mon Sep 17 00:00:00 2001
From: Neil Jerram <Neil.Jerram@metaswitch.com>
Date: Fri, 8 Apr 2016 19:23:47 +0100
Subject: [PATCH] Fix DHCPv4 reply via --bridge-interface alias interface
Sending a DHCPv4 reply through a --bridge-interface alias interface
was inadvertently broken by
commit 65c7212000
Author: Lung-Pin Chang <changlp@cs.nctu.edu.tw>
Date: Thu Mar 19 23:22:21 2015 +0000
dhcp: set outbound interface via cmsg in unicast reply
If multiple routes to the same network exist, Linux blindly picks
the first interface (route) based on destination address, which might not be
the one we're actually offering leases. Rather than relying on this,
always set the interface for outgoing unicast DHCP packets.
because in the aliasing case, iface_index is changed from the index of
the interface on which the packet was received, to be the interface
index of the 'bridge' interface (where the DHCP context is expected to
be defined, and so needs to be looked up).
For the cmsg code that the cited commit added, we need the original
iface_index; so this commit saves that off before the aliasing code
can change it, as rcvd_iface_index, and then uses rcvd_iface_index
instead of iface_index for the cmsg code.
Simon Kelley