It got reported to me on bug:
https://bugzilla.redhat.com/show_bug.cgi?id=2061944
This issue were introduced by commit
553c4c99cca173e9964d0edbd0676ed96c30f62b, which I backported to Fedora.
I believe it is still unfixed even in master branch.
It is usually simple to reproduce it, when dns=dnsmasq is set in NM.
Then when I connect both on ethernet and wifi, it happens always. I am
not sure why it happens only on two interfaces configured, that might be
just coincidence. Or it requires existing dbus configuration to break.
It does not matter, the problem is elsewhere.
When dbus calls add_update_server function, it has first server in
daemon->servers with flags == 0, domain set from dnsmasq configuration
file. When add_update_server is called from dbus update, it marks all
dbus servers. Then when at least two dbus servers were configured, it
will forget servers at the beginning without mark and replace
daemon->servers with serv->next pointer. Which is some server set by
dbus in this case. The first servers get leaked and their redirection
does not work anymore.
That happens because up pointer were not updated in else branch. up
needs to be updated when not marked, just like it is done in
cleanup_servers function. I have moved out the loop closer to it into
separate replace_server function. It makes it easier to compare again
similar loops above it.
Cheers,
Petr
--
Petr Menšík
Software Engineer, RHEL
Red Hat, http://www.redhat.com/
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
From 7e2e0a573835aa312f0a099c80faaa7a1afb60c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?=
Date: Fri, 17 Jun 2022 11:40:56 +0200
Subject: [PATCH] Correct add_update_server losing first unmarked entries
Beginning of servers list were updated when first server(s) record is
not marked. That was a mistake, which forgot updating also up pointer to
correct value. Move that loop to separate reuse_server function, which
is close to similar loop in cleanup_servers. Makes it easier to compare
and do correct. Removed tmp variable, because this code does not
invalidate previous serv pointer.
---
src/domain-match.c | 50 ++
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/src/domain-match.c b/src/domain-match.c
index 32a269e..1882ffd 100644
--- a/src/domain-match.c
+++ b/src/domain-match.c
@@ -606,6 +606,33 @@ void cleanup_servers(void)
build_server_array();
}
+/* Upstream servers. See if there is a suitable candidate, if so unmark
+ and move to the end of the list, for order. The entry found may already
+ be at the end. */
+static struct server *reuse_server(char *alloc_domain)
+{
+ struct server **up, *serv;
+
+ for (serv = daemon->servers, up = &daemon->servers; serv; serv = serv->next)
+{
+ if ((serv->flags & SERV_MARK) &&
+ hostname_isequal(alloc_domain, serv->domain))
+ {
+ /* Need to move down? */
+ if (serv->next)
+ {
+ *up = serv->next;
+ daemon->servers_tail->next = serv;
+ daemon->servers_tail = serv;
+ serv->next = NULL;
+ }
+ return serv;
+ }
+ up = &serv->next;
+}
+ return NULL;
+}
+
int add_update_server(int flags,
union mysockaddr *addr,
union mysockaddr *source_addr,
@@ -665,28 +692,7 @@ int add_update_server(int flags,
}
else
{
- /* Upstream servers. See if there is a suitable candidate, if so unmark
- and move to the end of the list, for order. The entry found may already
- be at the end. */
- struct server **up, *tmp;
-
- for (serv = daemon->servers, up = &daemon->servers; serv; serv = tmp)
- {
- tmp = serv->next;
- if ((serv->flags & SERV_MARK) &&
- hostname_isequal(alloc_domain, serv->domain))
- {
- /* Need to move down? */
- if (serv->next)
- {
- *up = serv->next;
- daemon->servers_tail->next = serv;
- daemon->servers_tail = serv;
- serv->next = NULL;
- }
- break;
- }
- }
+ serv = reuse_server(alloc_domain);
if (serv)
{
--
2.35.3
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
