dns/dnsmasq: cherry-pick upstream-fixes

*  Handle DHCPREBIND requests in the DHCPv6 server.
 *  Fix bug in TCP process handling.
This commit is contained in:
Matthias Andree 2021-05-15 11:22:50 +02:00
parent 9367fd9f70
commit 505d608290
3 changed files with 260 additions and 1 deletions

View file

@ -3,11 +3,12 @@
PORTNAME= dnsmasq
DISTVERSION= 2.85
# Leave the PORTREVISION in even if 0 to avoid accidental PORTEPOCH bumps:
PORTREVISION= 0
PORTREVISION= 1
PORTEPOCH= 1
CATEGORIES= dns
MASTER_SITES= https://www.thekelleys.org.uk/dnsmasq/ \
LOCAL/mandree/
PATCH_STRIP= -p1
MAINTAINER= mandree@FreeBSD.org
COMMENT= Lightweight DNS forwarder, DHCP, and TFTP server

View file

@ -0,0 +1,135 @@
commit ad90eb075dfeeb1936e8bc0f323fcc23f89364d4
Author: Simon Kelley <simon@thekelleys.org.uk>
Date: Fri Apr 9 16:08:05 2021 +0100
Fix bug in TCP process handling.
Fix bug which caused dnsmasq to lose track of processes forked
to handle TCP DNS connections under heavy load. The code
checked that at least one free process table slot was
available before listening on TCP sockets, but didn't take
into account that more than one TCP connection could
arrive, so that check was not sufficient to ensure that
there would be slots for all new processes. It compounded
this error by silently failing to store the process when
it did run out of slots. Even when this bug is triggered,
all the right things happen, and answers are still returned.
Only under very exceptional circumstances, does the bug
manifest itself: see
https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2021q2/014976.html
Thanks to Tijs Van Buggenhout for finding the conditions under
which the bug manifests itself, and then working out
exactly what was going on.
diff --git a/CHANGELOG b/CHANGELOG
index ab4aa42..87adaf7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,24 @@ version 2.86
Thanks to Aichun Li for spotting this ommision, and the initial
patch.
+ Fix bug which caused dnsmasq to lose track of processes forked
+ to handle TCP DNS connections under heavy load. The code
+ checked that at least one free process table slot was
+ available before listening on TCP sockets, but didn't take
+ into account that more than one TCP connection could
+ arrive, so that check was not sufficient to ensure that
+ there would be slots for all new processes. It compounded
+ this error by silently failing to store the process when
+ it did run out of slots. Even when this bug is triggered,
+ all the right things happen, and answers are still returned.
+ Only under very exceptional circumstances, does the bug
+ manifest itself: see
+ https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2021q2/014976.html
+ Thanks to Tijs Van Buggenhout for finding the conditions under
+ which the bug manifests itself, and then working out
+ exactly what was going on.
+
+
version 2.85
Fix problem with DNS retries in 2.83/2.84.
diff --git a/src/dnsmasq.c b/src/dnsmasq.c
index 256d2bc..bf031a1 100644
--- a/src/dnsmasq.c
+++ b/src/dnsmasq.c
@@ -1716,22 +1716,24 @@ static int set_dns_listeners(time_t now)
for (rfl = daemon->rfl_poll; rfl; rfl = rfl->next)
poll_listen(rfl->rfd->fd, POLLIN);
+ /* check to see if we have free tcp process slots. */
+ for (i = MAX_PROCS - 1; i >= 0; i--)
+ if (daemon->tcp_pids[i] == 0 && daemon->tcp_pipes[i] == -1)
+ break;
+
for (listener = daemon->listeners; listener; listener = listener->next)
{
/* only listen for queries if we have resources */
if (listener->fd != -1 && wait == 0)
poll_listen(listener->fd, POLLIN);
- /* death of a child goes through the select loop, so
- we don't need to explicitly arrange to wake up here */
- if (listener->tcpfd != -1)
- for (i = 0; i < MAX_PROCS; i++)
- if (daemon->tcp_pids[i] == 0 && daemon->tcp_pipes[i] == -1)
- {
- poll_listen(listener->tcpfd, POLLIN);
- break;
- }
-
+ /* Only listen for TCP connections when a process slot
+ is available. Death of a child goes through the select loop, so
+ we don't need to explicitly arrange to wake up here,
+ we'll be called again when a slot becomes available. */
+ if (listener->tcpfd != -1 && i >= 0)
+ poll_listen(listener->tcpfd, POLLIN);
+
#ifdef HAVE_TFTP
/* tftp == 0 in single-port mode. */
if (tftp <= daemon->tftp_max && listener->tftpfd != -1)
@@ -1797,7 +1799,16 @@ static void check_dns_listeners(time_t now)
tftp_request(listener, now);
#endif
- if (listener->tcpfd != -1 && poll_check(listener->tcpfd, POLLIN))
+ /* check to see if we have a free tcp process slot.
+ Note that we can't assume that because we had
+ at least one a poll() time, that we still do.
+ There may be more waiting connections after
+ poll() returns then free process slots. */
+ for (i = MAX_PROCS - 1; i >= 0; i--)
+ if (daemon->tcp_pids[i] == 0 && daemon->tcp_pipes[i] == -1)
+ break;
+
+ if (listener->tcpfd != -1 && i >= 0 && poll_check(listener->tcpfd, POLLIN))
{
int confd, client_ok = 1;
struct irec *iface = NULL;
@@ -1887,7 +1898,6 @@ static void check_dns_listeners(time_t now)
close(pipefd[0]);
else
{
- int i;
#ifdef HAVE_LINUX_NETWORK
/* The child process inherits the netlink socket,
which it never uses, but when the parent (us)
@@ -1907,13 +1917,9 @@ static void check_dns_listeners(time_t now)
read_write(pipefd[0], &a, 1, 1);
#endif
- for (i = 0; i < MAX_PROCS; i++)
- if (daemon->tcp_pids[i] == 0 && daemon->tcp_pipes[i] == -1)
- {
- daemon->tcp_pids[i] = p;
- daemon->tcp_pipes[i] = pipefd[0];
- break;
- }
+ /* i holds index of free slot */
+ daemon->tcp_pids[i] = p;
+ daemon->tcp_pipes[i] = pipefd[0];
}
close(confd);

View file

@ -0,0 +1,123 @@
commit d55e2d086d1ff30c427fa5e0ecc79746de8a81b7
Author: Simon Kelley <simon@thekelleys.org.uk>
Date: Fri Apr 9 15:19:28 2021 +0100
Handle DHCPREBIND requests in the DHCPv6 server.
Patch by srk, based on submitted patch from liaichun@huawei.com
diff --git a/CHANGELOG b/CHANGELOG
index ca555ed..ab4aa42 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,9 @@
+version 2.86
+ Handle DHCPREBIND requests in the DHCPv6 server code.
+ Thanks to Aichun Li for spotting this ommision, and the initial
+ patch.
+
+
version 2.85
Fix problem with DNS retries in 2.83/2.84.
The new logic in 2.83/2.84 which merges distinct requests
diff --git a/src/rfc3315.c b/src/rfc3315.c
index 982c68a..5c2ff97 100644
--- a/src/rfc3315.c
+++ b/src/rfc3315.c
@@ -919,11 +919,14 @@ static int dhcp6_no_relay(struct state *state, int msg_type, void *inbuff, size_
case DHCP6RENEW:
+ case DHCP6REBIND:
{
+ int address_assigned = 0;
+
/* set reply message type */
*outmsgtypep = DHCP6REPLY;
- log6_quiet(state, "DHCPRENEW", NULL, NULL);
+ log6_quiet(state, msg_type == DHCP6RENEW ? "DHCPRENEW" : "DHCPREBIND", NULL, NULL);
for (opt = state->packet_options; opt; opt = opt6_next(opt, state->end))
{
@@ -952,24 +955,35 @@ static int dhcp6_no_relay(struct state *state, int msg_type, void *inbuff, size_
state->ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA,
state->iaid, &req_addr)))
{
- /* If the server cannot find a client entry for the IA the server
- returns the IA containing no addresses with a Status Code option set
- to NoBinding in the Reply message. */
- save_counter(iacntr);
- t1cntr = 0;
-
- log6_packet(state, "DHCPREPLY", &req_addr, _("lease not found"));
-
- o1 = new_opt6(OPTION6_STATUS_CODE);
- put_opt6_short(DHCP6NOBINDING);
- put_opt6_string(_("no binding found"));
- end_opt6(o1);
-
- preferred_time = valid_time = 0;
- break;
+ if (msg_type == DHCP6REBIND)
+ {
+ /* When rebinding, we can create a lease if it doesn't exist. */
+ lease = lease6_allocate(&req_addr, state->ia_type == OPTION6_IA_NA ? LEASE_NA : LEASE_TA);
+ if (lease)
+ lease_set_iaid(lease, state->iaid);
+ else
+ break;
+ }
+ else
+ {
+ /* If the server cannot find a client entry for the IA the server
+ returns the IA containing no addresses with a Status Code option set
+ to NoBinding in the Reply message. */
+ save_counter(iacntr);
+ t1cntr = 0;
+
+ log6_packet(state, "DHCPREPLY", &req_addr, _("lease not found"));
+
+ o1 = new_opt6(OPTION6_STATUS_CODE);
+ put_opt6_short(DHCP6NOBINDING);
+ put_opt6_string(_("no binding found"));
+ end_opt6(o1);
+
+ preferred_time = valid_time = 0;
+ break;
+ }
}
-
if ((this_context = address6_available(state->context, &req_addr, tagif, 1)) ||
(this_context = address6_valid(state->context, &req_addr, tagif, 1)))
{
@@ -1000,6 +1014,8 @@ static int dhcp6_no_relay(struct state *state, int msg_type, void *inbuff, size_
if (preferred_time == 0)
message = _("deprecated");
+
+ address_assigned = 1;
}
else
{
@@ -1022,10 +1038,18 @@ static int dhcp6_no_relay(struct state *state, int msg_type, void *inbuff, size_
end_ia(t1cntr, min_time, 1);
end_opt6(o);
}
+
+ if (!address_assigned && msg_type == DHCP6REBIND)
+ {
+ /* can't create lease for any address, return error */
+ o1 = new_opt6(OPTION6_STATUS_CODE);
+ put_opt6_short(DHCP6NOADDRS);
+ put_opt6_string(_("no addresses available"));
+ end_opt6(o1);
+ }
tagif = add_options(state, 0);
break;
-
}
case DHCP6CONFIRM: