Skip to content

Commit

Permalink
resolved: disable event sources before unreffing them
Browse files Browse the repository at this point in the history
We generally operate on the assumption that a source is "gone" as soon
as we unref it. This is generally true because we have the only reference.
But if something else holds the reference, our unref doesn't really stop
the source and it could fire again.

In particular, on_query_timeout() is called with DnsQuery* as userdata, and
it calls dns_query_stop() which invalidates that pointer. If it was ever
called again, we'd be accessing already-freed memory.

I don't see what would hold the reference. sd-event takes a temporary reference,
but on the sd_event object, not on the individual sources. And our sources
are non-floating, so there is no reference from the sd_event object to the
sources.

For #18427.

(cherry picked from commit 9793530)
  • Loading branch information
keszybz committed Mar 12, 2021
1 parent 6345ef6 commit 78a43c3
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/resolve/resolved-dns-query.c
Expand Up @@ -297,7 +297,7 @@ static void dns_query_stop(DnsQuery *q) {

assert(q);

q->timeout_event_source = sd_event_source_unref(q->timeout_event_source);
q->timeout_event_source = sd_event_source_disable_unref(q->timeout_event_source);

LIST_FOREACH(candidates_by_query, c, q->candidates)
dns_query_candidate_stop(c);
Expand Down
8 changes: 4 additions & 4 deletions src/resolve/resolved-dns-scope.c
Expand Up @@ -110,9 +110,9 @@ DnsScope* dns_scope_free(DnsScope *s) {
hashmap_free(s->transactions_by_key);

ordered_hashmap_free_with_destructor(s->conflict_queue, dns_resource_record_unref);
sd_event_source_unref(s->conflict_event_source);
sd_event_source_disable_unref(s->conflict_event_source);

sd_event_source_unref(s->announce_event_source);
sd_event_source_disable_unref(s->announce_event_source);

dns_cache_flush(&s->cache);
dns_zone_flush(&s->zone);
Expand Down Expand Up @@ -980,7 +980,7 @@ static int on_conflict_dispatch(sd_event_source *es, usec_t usec, void *userdata
assert(es);
assert(scope);

scope->conflict_event_source = sd_event_source_unref(scope->conflict_event_source);
scope->conflict_event_source = sd_event_source_disable_unref(scope->conflict_event_source);

for (;;) {
_cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL;
Expand Down Expand Up @@ -1191,7 +1191,7 @@ static int on_announcement_timeout(sd_event_source *s, usec_t usec, void *userda

assert(s);

scope->announce_event_source = sd_event_source_unref(scope->announce_event_source);
scope->announce_event_source = sd_event_source_disable_unref(scope->announce_event_source);

(void) dns_scope_announce(scope, false);
return 0;
Expand Down
4 changes: 2 additions & 2 deletions src/resolve/resolved-dns-stream.c
Expand Up @@ -18,8 +18,8 @@
static void dns_stream_stop(DnsStream *s) {
assert(s);

s->io_event_source = sd_event_source_unref(s->io_event_source);
s->timeout_event_source = sd_event_source_unref(s->timeout_event_source);
s->io_event_source = sd_event_source_disable_unref(s->io_event_source);
s->timeout_event_source = sd_event_source_disable_unref(s->timeout_event_source);
s->fd = safe_close(s->fd);

/* Disconnect us from the server object if we are now not usable anymore */
Expand Down
10 changes: 5 additions & 5 deletions src/resolve/resolved-dns-stub.c
Expand Up @@ -79,8 +79,8 @@ DnsStubListenerExtra *dns_stub_listener_extra_free(DnsStubListenerExtra *p) {
if (!p)
return NULL;

p->udp_event_source = sd_event_source_unref(p->udp_event_source);
p->tcp_event_source = sd_event_source_unref(p->tcp_event_source);
p->udp_event_source = sd_event_source_disable_unref(p->udp_event_source);
p->tcp_event_source = sd_event_source_disable_unref(p->tcp_event_source);

return mfree(p);
}
Expand Down Expand Up @@ -763,12 +763,12 @@ int manager_dns_stub_start(Manager *m) {
void manager_dns_stub_stop(Manager *m) {
assert(m);

m->dns_stub_udp_event_source = sd_event_source_unref(m->dns_stub_udp_event_source);
m->dns_stub_tcp_event_source = sd_event_source_unref(m->dns_stub_tcp_event_source);
m->dns_stub_udp_event_source = sd_event_source_disable_unref(m->dns_stub_udp_event_source);
m->dns_stub_tcp_event_source = sd_event_source_disable_unref(m->dns_stub_tcp_event_source);
}

static const char* const dns_stub_listener_mode_table[_DNS_STUB_LISTENER_MODE_MAX] = {
[DNS_STUB_LISTENER_NO] = "no",
[DNS_STUB_LISTENER_NO] = "no",
[DNS_STUB_LISTENER_UDP] = "udp",
[DNS_STUB_LISTENER_TCP] = "tcp",
[DNS_STUB_LISTENER_YES] = "yes",
Expand Down
5 changes: 3 additions & 2 deletions src/resolve/resolved-dns-transaction.c
Expand Up @@ -59,14 +59,15 @@ static void dns_transaction_close_connection(DnsTransaction *t) {
t->stream = dns_stream_unref(t->stream);
}

t->dns_udp_event_source = sd_event_source_unref(t->dns_udp_event_source);
t->dns_udp_event_source = sd_event_source_disable_unref(t->dns_udp_event_source);

t->dns_udp_fd = safe_close(t->dns_udp_fd);
}

static void dns_transaction_stop_timeout(DnsTransaction *t) {
assert(t);

t->timeout_event_source = sd_event_source_unref(t->timeout_event_source);
t->timeout_event_source = sd_event_source_disable_unref(t->timeout_event_source);
}

DnsTransaction* dns_transaction_free(DnsTransaction *t) {
Expand Down
8 changes: 4 additions & 4 deletions src/resolve/resolved-llmnr.c
Expand Up @@ -11,16 +11,16 @@
void manager_llmnr_stop(Manager *m) {
assert(m);

m->llmnr_ipv4_udp_event_source = sd_event_source_unref(m->llmnr_ipv4_udp_event_source);
m->llmnr_ipv4_udp_event_source = sd_event_source_disable_unref(m->llmnr_ipv4_udp_event_source);
m->llmnr_ipv4_udp_fd = safe_close(m->llmnr_ipv4_udp_fd);

m->llmnr_ipv6_udp_event_source = sd_event_source_unref(m->llmnr_ipv6_udp_event_source);
m->llmnr_ipv6_udp_event_source = sd_event_source_disable_unref(m->llmnr_ipv6_udp_event_source);
m->llmnr_ipv6_udp_fd = safe_close(m->llmnr_ipv6_udp_fd);

m->llmnr_ipv4_tcp_event_source = sd_event_source_unref(m->llmnr_ipv4_tcp_event_source);
m->llmnr_ipv4_tcp_event_source = sd_event_source_disable_unref(m->llmnr_ipv4_tcp_event_source);
m->llmnr_ipv4_tcp_fd = safe_close(m->llmnr_ipv4_tcp_fd);

m->llmnr_ipv6_tcp_event_source = sd_event_source_unref(m->llmnr_ipv6_tcp_event_source);
m->llmnr_ipv6_tcp_event_source = sd_event_source_disable_unref(m->llmnr_ipv6_tcp_event_source);
m->llmnr_ipv6_tcp_fd = safe_close(m->llmnr_ipv6_tcp_fd);
}

Expand Down
4 changes: 2 additions & 2 deletions src/resolve/resolved-mdns.c
Expand Up @@ -15,10 +15,10 @@
void manager_mdns_stop(Manager *m) {
assert(m);

m->mdns_ipv4_event_source = sd_event_source_unref(m->mdns_ipv4_event_source);
m->mdns_ipv4_event_source = sd_event_source_disable_unref(m->mdns_ipv4_event_source);
m->mdns_ipv4_fd = safe_close(m->mdns_ipv4_fd);

m->mdns_ipv6_event_source = sd_event_source_unref(m->mdns_ipv6_event_source);
m->mdns_ipv6_event_source = sd_event_source_disable_unref(m->mdns_ipv6_event_source);
m->mdns_ipv6_fd = safe_close(m->mdns_ipv6_fd);
}

Expand Down

0 comments on commit 78a43c3

Please sign in to comment.