Previously, there was a chance of memory corruption, because when
switching to the next DNS server we didn't care whether they linked list
of DNS servers was still valid.
Clean up lifecycle of the dns server logic:
- When a DnsServer object is still in the linked list of DnsServers for
a link or the manager, indicate so with a "linked" boolean field, and
never follow the linked list if that boolean is not set.
- When picking a DnsServer to use for a link ot manager, always
explicitly take a reference.
This also rearranges some logic, to make the tracking of dns servers by
link and globally more alike.
int manager_add_dns_server_by_string(Manager *m, DnsServerType type, const char *word) {
union in_addr_union address;
int manager_add_dns_server_by_string(Manager *m, DnsServerType type, const char *word) {
union in_addr_union address;
- first = type == DNS_SERVER_FALLBACK ? m->fallback_dns_servers : m->dns_servers;
-
/* Filter out duplicates */
/* Filter out duplicates */
- LIST_FOREACH(servers, s, first)
- if (s->family == family && in_addr_equal(family, &s->address, &address))
- break;
-
+ s = manager_find_dns_server(m, type, family, &address);
if (s) {
/*
* Drop the marker. This is used to find the servers
if (s) {
/*
* Drop the marker. This is used to find the servers
assert_not_reached("Unknown server type");
s->manager = m;
assert_not_reached("Unknown server type");
s->manager = m;
/* A new DNS server that isn't fallback is added and the one
* we used so far was a fallback one? Then let's try to pick
/* A new DNS server that isn't fallback is added and the one
* we used so far was a fallback one? Then let's try to pick
return NULL;
assert(s->n_ref > 0);
return NULL;
assert(s->n_ref > 0);
-static DnsServer* dns_server_free(DnsServer *s) {
+DnsServer* dns_server_unref(DnsServer *s) {
- if (s->link && s->link->current_dns_server == s)
- link_set_dns_server(s->link, NULL);
+ assert(s->n_ref > 0);
+ s->n_ref --;
- if (s->manager && s->manager->current_dns_server == s)
- manager_set_dns_server(s->manager, NULL);
+ if (s->n_ref > 0)
+ return NULL;
-DnsServer* dns_server_unref(DnsServer *s) {
- if (!s)
- return NULL;
+void dns_server_unlink(DnsServer *s) {
+ assert(s);
+ assert(s->manager);
+ /* This removes the specified server from the linked list of
+ * servers, but any server might still stay around if it has
+ * refs, for example from an ongoing transaction. */
- if (s->n_ref == 1)
- dns_server_free(s);
- else
- s->n_ref --;
+ if (!s->linked)
+ return;
+ switch (s->type) {
+
+ case DNS_SERVER_LINK:
+ assert(s->link);
+ LIST_REMOVE(servers, s->link->dns_servers, s);
+ break;
+
+ case DNS_SERVER_SYSTEM:
+ LIST_REMOVE(servers, s->manager->dns_servers, s);
+ break;
+
+ case DNS_SERVER_FALLBACK:
+ LIST_REMOVE(servers, s->manager->fallback_dns_servers, s);
+ break;
+ }
+
+ s->linked = false;
+
+ if (s->link && s->link->current_dns_server == s)
+ link_set_dns_server(s->link, NULL);
+
+ if (s->manager->current_dns_server == s)
+ manager_set_dns_server(s->manager, NULL);
+
+ dns_server_unref(s);
}
void dns_server_packet_received(DnsServer *s, usec_t rtt) {
}
void dns_server_packet_received(DnsServer *s, usec_t rtt) {
.compare = dns_server_compare_func
};
.compare = dns_server_compare_func
};
-void manager_flush_dns_servers(Manager *m, DnsServerType type) {
- DnsServer **first, *s;
+DnsServer *manager_get_first_dns_server(Manager *m, DnsServerType t) {
+ assert(m);
+
+ switch (t) {
+
+ case DNS_SERVER_SYSTEM:
+ return m->dns_servers;
+
+ case DNS_SERVER_FALLBACK:
+ return m->fallback_dns_servers;
+ default:
+ return NULL;
+ }
+}
+
+void manager_flush_dns_servers(Manager *m, DnsServerType type) {
- first = type == DNS_SERVER_FALLBACK ? &m->fallback_dns_servers : &m->dns_servers;
+ for (;;) {
+ DnsServer *first;
- while (*first) {
- s = *first;
+ first = manager_get_first_dns_server(m, type);
+ if (!first)
+ break;
- LIST_REMOVE(servers, *first, s);
- dns_server_unref(s);
+ dns_server_unlink(first);
}
}
void manager_flush_marked_dns_servers(Manager *m, DnsServerType type) {
}
}
void manager_flush_marked_dns_servers(Manager *m, DnsServerType type) {
- DnsServer **first, *s, *next;
+ DnsServer *first, *s, *next;
- first = type == DNS_SERVER_FALLBACK ? &m->fallback_dns_servers : &m->dns_servers;
+ first = manager_get_first_dns_server(m, type);
- LIST_FOREACH_SAFE(servers, s, next, *first) {
+ LIST_FOREACH_SAFE(servers, s, next, first) {
if (!s->marked)
continue;
if (!s->marked)
continue;
- LIST_REMOVE(servers, *first, s);
- dns_server_unref(s);
- first = type == DNS_SERVER_FALLBACK ? m->fallback_dns_servers : m->dns_servers;
-
+ first = manager_get_first_dns_server(m, type);
LIST_FOREACH(servers, s, first)
s->marked = true;
}
LIST_FOREACH(servers, s, first)
s->marked = true;
}
-DnsServer* manager_find_dns_server(Manager *m, int family, const union in_addr_union *in_addr) {
- DnsServer *s;
+DnsServer* manager_find_dns_server(Manager *m, DnsServerType type, int family, const union in_addr_union *in_addr) {
+ DnsServer *first, *s;
assert(m);
assert(in_addr);
assert(m);
assert(in_addr);
- LIST_FOREACH(servers, s, m->dns_servers)
- if (s->family == family && in_addr_equal(family, &s->address, in_addr) > 0)
- return s;
+ first = manager_get_first_dns_server(m, type);
- LIST_FOREACH(servers, s, m->fallback_dns_servers)
+ LIST_FOREACH(servers, s, first)
if (s->family == family && in_addr_equal(family, &s->address, in_addr) > 0)
return s;
if (s->family == family && in_addr_equal(family, &s->address, in_addr) > 0)
return s;
log_info("Switching to system DNS server %s.", strna(ip));
}
log_info("Switching to system DNS server %s.", strna(ip));
}
- m->current_dns_server = s;
+ dns_server_unref(m->current_dns_server);
+ m->current_dns_server = dns_server_ref(s);
if (m->unicast_scope)
dns_cache_flush(&m->unicast_scope->cache);
if (m->unicast_scope)
dns_cache_flush(&m->unicast_scope->cache);
if (!m->current_dns_server)
return;
if (!m->current_dns_server)
return;
- /* Change to the next one */
- if (m->current_dns_server->servers_next) {
+ /* Change to the next one, but make sure to follow the linked
+ * list only if the server is still linked. */
+ if (m->current_dns_server->linked && m->current_dns_server->servers_next) {
manager_set_dns_server(m, m->current_dns_server->servers_next);
return;
}
manager_set_dns_server(m, m->current_dns_server->servers_next);
return;
}
+ /* If linked is set, then this server appears in the servers linked list */
+ bool linked:1;
LIST_FIELDS(DnsServer, servers);
};
LIST_FIELDS(DnsServer, servers);
};
DnsServer* dns_server_ref(DnsServer *s);
DnsServer* dns_server_unref(DnsServer *s);
DnsServer* dns_server_ref(DnsServer *s);
DnsServer* dns_server_unref(DnsServer *s);
+void dns_server_unlink(DnsServer *s);
+
void dns_server_packet_received(DnsServer *s, usec_t rtt);
void dns_server_packet_lost(DnsServer *s, usec_t usec);
void dns_server_packet_received(DnsServer *s, usec_t rtt);
void dns_server_packet_lost(DnsServer *s, usec_t usec);
-DnsServer *manager_set_dns_server(Manager *m, DnsServer *s);
-DnsServer *manager_find_dns_server(Manager *m, int family, const union in_addr_union *in_addr);
-DnsServer *manager_get_dns_server(Manager *m);
-void manager_next_dns_server(Manager *m);
+DnsServer *manager_get_first_dns_server(Manager *m, DnsServerType t);
void manager_flush_dns_servers(Manager *m, DnsServerType t);
void manager_flush_marked_dns_servers(Manager *m, DnsServerType type);
void manager_mark_dns_servers(Manager *m, DnsServerType type);
void manager_flush_dns_servers(Manager *m, DnsServerType t);
void manager_flush_marked_dns_servers(Manager *m, DnsServerType type);
void manager_mark_dns_servers(Manager *m, DnsServerType type);
+DnsServer *manager_set_dns_server(Manager *m, DnsServer *s);
+DnsServer *manager_find_dns_server(Manager *m, DnsServerType t, int family, const union in_addr_union *in_addr);
+DnsServer *manager_get_dns_server(Manager *m);
+void manager_next_dns_server(Manager *m);
+
DEFINE_TRIVIAL_CLEANUP_FUNC(DnsServer*, dns_server_unref);
extern const struct hash_ops dns_server_hash_ops;
DEFINE_TRIVIAL_CLEANUP_FUNC(DnsServer*, dns_server_unref);
extern const struct hash_ops dns_server_hash_ops;
+ link_flush_dns_servers(l);
+
while (l->addresses)
link_address_free(l->addresses);
if (l->manager)
hashmap_remove(l->manager->links, INT_TO_PTR(l->ifindex));
while (l->addresses)
link_address_free(l->addresses);
if (l->manager)
hashmap_remove(l->manager->links, INT_TO_PTR(l->ifindex));
- while (l->dns_servers) {
- DnsServer *s = l->dns_servers;
-
- LIST_REMOVE(servers, l->dns_servers, s);
- dns_server_unref(s);
- }
-
dns_scope_free(l->unicast_scope);
dns_scope_free(l->llmnr_ipv4_scope);
dns_scope_free(l->llmnr_ipv6_scope);
dns_scope_free(l->unicast_scope);
dns_scope_free(l->llmnr_ipv4_scope);
dns_scope_free(l->llmnr_ipv6_scope);
static int link_update_dns_servers(Link *l) {
_cleanup_strv_free_ char **nameservers = NULL;
char **nameserver;
static int link_update_dns_servers(Link *l) {
_cleanup_strv_free_ char **nameservers = NULL;
char **nameserver;
- LIST_FOREACH(servers, s, l->dns_servers)
- s->marked = true;
+ link_mark_dns_servers(l);
STRV_FOREACH(nameserver, nameservers) {
union in_addr_union a;
STRV_FOREACH(nameserver, nameservers) {
union in_addr_union a;
int family;
r = in_addr_from_string_auto(*nameserver, &family, &a);
int family;
r = in_addr_from_string_auto(*nameserver, &family, &a);
- LIST_FOREACH_SAFE(servers, s, nx, l->dns_servers)
- if (s->marked) {
- LIST_REMOVE(servers, l->dns_servers, s);
- dns_server_unref(s);
- }
-
+ link_flush_marked_dns_servers(l);
- while (l->dns_servers) {
- s = l->dns_servers;
-
- LIST_REMOVE(servers, l->dns_servers, s);
- dns_server_unref(s);
- }
-
+ link_flush_dns_servers(l);
+void link_flush_dns_servers(Link *l) {
+ assert(l);
+
+ while (l->dns_servers)
+ dns_server_unlink(l->dns_servers);
+}
+
+void link_flush_marked_dns_servers(Link *l) {
+ DnsServer *s, *next;
+
+ assert(l);
+
+ LIST_FOREACH_SAFE(servers, s, next, l->dns_servers) {
+ if (!s->marked)
+ continue;
+
+ dns_server_unlink(s);
+ }
+}
+
+void link_mark_dns_servers(Link *l) {
+ DnsServer *s;
+
+ assert(l);
+
+ LIST_FOREACH(servers, s, l->dns_servers)
+ s->marked = true;
+}
+
DnsServer* link_find_dns_server(Link *l, int family, const union in_addr_union *in_addr) {
DnsServer *s;
assert(l);
DnsServer* link_find_dns_server(Link *l, int family, const union in_addr_union *in_addr) {
DnsServer *s;
assert(l);
LIST_FOREACH(servers, s, l->dns_servers)
if (s->family == family && in_addr_equal(family, &s->address, in_addr))
LIST_FOREACH(servers, s, l->dns_servers)
if (s->family == family && in_addr_equal(family, &s->address, in_addr))
log_info("Switching to DNS server %s for interface %s.", strna(ip), l->name);
}
log_info("Switching to DNS server %s for interface %s.", strna(ip), l->name);
}
- l->current_dns_server = s;
+ dns_server_unref(l->current_dns_server);
+ l->current_dns_server = dns_server_ref(s);
if (l->unicast_scope)
dns_cache_flush(&l->unicast_scope->cache);
if (l->unicast_scope)
dns_cache_flush(&l->unicast_scope->cache);
if (!l->current_dns_server)
return;
if (!l->current_dns_server)
return;
- if (l->current_dns_server->servers_next) {
+ /* Change to the next one, but make sure to follow the linked
+ * list only if this server is actually still linked. */
+ if (l->current_dns_server->linked && l->current_dns_server->servers_next) {
link_set_dns_server(l, l->current_dns_server->servers_next);
return;
}
link_set_dns_server(l, l->current_dns_server->servers_next);
return;
}
LinkAddress* link_find_address(Link *l, int family, const union in_addr_union *in_addr);
void link_add_rrs(Link *l, bool force_remove);
LinkAddress* link_find_address(Link *l, int family, const union in_addr_union *in_addr);
void link_add_rrs(Link *l, bool force_remove);
+void link_flush_dns_servers(Link *l);
+void link_flush_marked_dns_servers(Link *l);
+void link_mark_dns_servers(Link *l);
+
DnsServer* link_set_dns_server(Link *l, DnsServer *s);
DnsServer* link_find_dns_server(Link *l, int family, const union in_addr_union *in_addr);
DnsServer* link_get_dns_server(Link *l);
DnsServer* link_set_dns_server(Link *l, DnsServer *s);
DnsServer* link_find_dns_server(Link *l, int family, const union in_addr_union *in_addr);
DnsServer* link_get_dns_server(Link *l);
+ manager_flush_dns_servers(m, DNS_SERVER_SYSTEM);
+ manager_flush_dns_servers(m, DNS_SERVER_FALLBACK);
+
while ((l = hashmap_first(m->links)))
link_free(l);
while (m->dns_queries)
dns_query_free(m->dns_queries);
while ((l = hashmap_first(m->links)))
link_free(l);
while (m->dns_queries)
dns_query_free(m->dns_queries);
- manager_flush_dns_servers(m, DNS_SERVER_SYSTEM);
- manager_flush_dns_servers(m, DNS_SERVER_FALLBACK);
-
dns_scope_free(m->unicast_scope);
hashmap_free(m->links);
dns_scope_free(m->unicast_scope);
hashmap_free(m->links);
_cleanup_fclose_ FILE *f = NULL;
struct stat st, own;
char line[LINE_MAX];
_cleanup_fclose_ FILE *f = NULL;
struct stat st, own;
char line[LINE_MAX];
if (errno == ENOENT)
r = 0;
else
if (errno == ENOENT)
r = 0;
else
- r = log_warning_errno(errno, "Failed to open /etc/resolv.conf: %m");
+ r = log_warning_errno(errno, "Failed to stat /etc/resolv.conf: %m");
manager_mark_dns_servers(m, DNS_SERVER_SYSTEM);
FOREACH_LINE(line, f, r = -errno; goto clear) {
manager_mark_dns_servers(m, DNS_SERVER_SYSTEM);
FOREACH_LINE(line, f, r = -errno; goto clear) {
- _cleanup_strv_free_ char **d = NULL;
- while (m->dns_servers) {
- s = m->dns_servers;
-
- LIST_REMOVE(servers, m->dns_servers, s);
- dns_server_unref(s);
- }
-
+ manager_flush_dns_servers(m, DNS_SERVER_SYSTEM);