]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
resolved: disable event sources before unreffing them
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Mon, 1 Mar 2021 22:10:06 +0000 (23:10 +0100)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Fri, 12 Mar 2021 16:49:10 +0000 (17:49 +0100)
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 97935302283729c9206b84f5e00b1aff0f78ad19)

src/resolve/resolved-dns-query.c
src/resolve/resolved-dns-scope.c
src/resolve/resolved-dns-stream.c
src/resolve/resolved-dns-stub.c
src/resolve/resolved-dns-transaction.c
src/resolve/resolved-llmnr.c
src/resolve/resolved-mdns.c

index 8ee4fd8309348647d86d0c68204686e42d7daf51..f6ff3b5388e23eb9f477707ffe52ed60ac306cf1 100644 (file)
@@ -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);
index 509a2060e8831905f2520c544e538839d4a8d3a6..2063f0a2b6e5b7ee4175af699b2e6d05ddbc403e 100644 (file)
@@ -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);
@@ -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;
@@ -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;
index 1aab0899347bf41bfc47a5369fa1377276d0d1f9..1471628fc32b389edb12cf35eeaf51657dc09955 100644 (file)
@@ -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 */
index 6a3dc9901ae8326e93147a373be567739b865d9c..5043c607cf7b01ae0332955570e0d10b36570b57 100644 (file)
@@ -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);
 }
@@ -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",
index 37f0ddde6c4eef51ec859963d691bfb78a5a617c..8cf4a9637ad734e468c81f9ba8726c64067ee2eb 100644 (file)
@@ -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) {
index 2ddf08815ad40774131553c4f5e24cb7e4c2c7d1..ad3591626851b9d14dca3c945656e9fb13aa1ac6 100644 (file)
 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);
 }
 
index 3996fcab2ef5e5f92683f9205da2551ecb8702d1..aee7bf7e4758ce4a46fd1957249475b8345bb144 100644 (file)
 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);
 }