Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update embedded dnsmasq #1543

Merged
merged 9 commits into from
Mar 22, 2023
Merged

Update embedded dnsmasq #1543

merged 9 commits into from
Mar 22, 2023

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Mar 22, 2023

Merge bug fixes and improvements

Related changelog:

	Fix reversion in --rev-server introduced in 2.88 which
	caused breakage if the prefix length is not exactly divisible
	by 8 (IPv4) or 4 (IPv6).

	Fix possible SEGV when there server(s) for a particular
	domain are configured, but no server which is not qualified
	for a particular domain. Thanks to Daniel Danzberger for
	spotting this bug.

	Set the default maximum DNS UDP packet sice to 1232. This
	has been the recommended value since 2020 because it's the
	largest value that avoid fragmentation, and fragmentation
	is just not reliable on the modern internet, especially
	for IPv6. It's still possible to override this with
	--edns-packet-max for special circumstances.

Taylor R Campbell and others added 9 commits March 18, 2023 13:58
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.

Signed-off-by: DL6ER <dl6er@dl6er.de>
…ceadd3668b74d when resolving upstream servers by name was extended to --rev-server without accounting for the fact that re-using one and the same upstream server for each of the x.y.z.in-addr.arpa is actually a wanted feature

Signed-off-by: DL6ER <dl6er@dl6er.de>
If there exists a --address=/<domain>/  or --server=/<domain>/#
configuration but no upstream server config unqualified by
domain then when a query which doesnt match the domain is
recieved it will use the qualfied server config and in the process
possibly make an out-of-bounds memory access.

Thanks to Daniel Danzberger for spotting the bug.

Signed-off-by: DL6ER <dl6er@dl6er.de>
http://www.dnsflagday.net/2020/ refers.

Thanks to Xiang Li for the prompt.

Signed-off-by: DL6ER <dl6er@dl6er.de>
We can cache an NXDOMAIN reply to a query for any RRTYPE
and reply from a cached NXDOMAIN to any RRTYPE.

Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
…e message type correctly.

Thanks to Petr Menšík for spotting the problem.

Signed-off-by: DL6ER <dl6er@dl6er.de>
Dynamic-host was implemented to ignore interface addresses with /32
(or /128 for IPv6) prefix lengths, since they are not useful for
synthesising addresses.

Due to a bug before 2.88, this didn't work for IPv4, and some have
used --dynamic-host=example.com,0.0.0.0,eth0 to do the equivalent of
--interface-name for such interfaces. When the bug was fixed in 2.88
these uses broke.

Since this behaviour seems to violate the principle of least surprise,
and since the 2.88 fix is breaking existing imstallations, this
commit removes the check on /32 and /128 prefix lengths to solve both
problems.

Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER requested a review from a team March 22, 2023 20:58
@PromoFaux PromoFaux merged commit e408bd9 into development Mar 22, 2023
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-22-web-v5-19-and-core-v5-16-1-released/61999/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants