resolved: disable event sources before unreffing them

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.

diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c
index 7fb2e110e07130dd927b80936a658a9322283b8d..7554d1e82f41baf12f2768fa4e873dfd389be35d 100644 (file)
--- a/src/resolve/resolved-dns-query.c
+++ b/src/resolve/resolved-dns-query.c
@@ -326,7 +326,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);

diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index 8eb91a013b52afd42afa4ceb93db27d34f7b14f3..67c6f54dc5d9f17dbc47544240fc866961b567d2 100644 (file)
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -111,9 +111,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);
@@ -1147,7 +1147,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;
@@ -1358,7 +1358,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;

diff --git a/src/resolve/resolved-dns-stream.c b/src/resolve/resolved-dns-stream.c
index 2ab6f6236dcc0744bda94149f4fec70b531e2973..10641f6ac59d943aece88fb1b50caa770071c2bc 100644 (file)
--- a/src/resolve/resolved-dns-stream.c
+++ b/src/resolve/resolved-dns-stream.c
@@ -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 */

diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c
index 2cfabb3629eddecd491ad6b5c43442eaea08a385..c2734e57b9bcb66af3eef4353e0527ac70842850 100644 (file)
--- a/src/resolve/resolved-dns-stub.c
+++ b/src/resolve/resolved-dns-stub.c
@@ -81,8 +81,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);
 
         hashmap_free(p->queries_by_packet);
 
@@ -1257,12 +1257,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",

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 590652e5d35bbcac310cf85abbfb153770c08525..07c6eca3ba4ff2293a6a52a94cd899025adf5418 100644 (file)
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -66,7 +66,7 @@ static void dns_transaction_close_connection(
                 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);
 
         /* If we have an UDP socket where we sent a packet, but never received one, then add it to the socket
          * graveyard, instead of closing it right away. That way it will stick around for a moment longer,
@@ -87,7 +87,7 @@ static void dns_transaction_close_connection(
 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) {

diff --git a/src/resolve/resolved-llmnr.c b/src/resolve/resolved-llmnr.c
index ccdf138db4be79f2030175e8066b7092064a6895..ce2db7d4b02fe17f3552781b55caa84ec41a03be 100644 (file)
--- a/src/resolve/resolved-llmnr.c
+++ b/src/resolve/resolved-llmnr.c
@@ -12,16 +12,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);
 }
 

diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index 0b14436709755d0025d93771419a0c42e1d8b8cb..e815dacd7f1e94eed7a83cfdad94665660c52aa8 100644 (file)
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -342,7 +342,7 @@ static int manager_clock_change_listen(Manager *m) {
 
         assert(m);
 
-        m->clock_change_event_source = sd_event_source_unref(m->clock_change_event_source);
+        m->clock_change_event_source = sd_event_source_disable_unref(m->clock_change_event_source);
 
         fd = time_change_fd();
         if (fd < 0)

diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c
index 15843e90b9fe5c0437c903d2965222665384bde8..e3a46f4ca1672701762332fd8a4642f7bfdb5ad1 100644 (file)
--- a/src/resolve/resolved-mdns.c
+++ b/src/resolve/resolved-mdns.c
@@ -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);
 }
 

diff --git a/src/resolve/resolved-socket-graveyard.c b/src/resolve/resolved-socket-graveyard.c
index 067cb666d459c32a866057300e9780807174f577..471fe1d57881c972e7928d8da5d77c9a48010c2f 100644 (file)
--- a/src/resolve/resolved-socket-graveyard.c
+++ b/src/resolve/resolved-socket-graveyard.c
@@ -36,7 +36,7 @@ static SocketGraveyard* socket_graveyard_free(SocketGraveyard *g) {
 
         if (g->io_event_source) {
                 log_debug("Closing graveyard socket fd %i", sd_event_source_get_io_fd(g->io_event_source));
-                sd_event_source_unref(g->io_event_source);
+                sd_event_source_disable_unref(g->io_event_source);
         }
 
         return mfree(g);