From 904dcaf9d4933499f8334859f52ea8497f2d24ff Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 4 Dec 2018 22:09:08 +0100
Subject: [PATCH] resolved: take particular care when detaching DnsServer from
 its default stream

DnsStream and DnsServer have a symbiotic relationship: one DnsStream is
the current "default" stream of the server (and thus reffed by it), but
each stream also refs the server it is connected to. This cyclic
dependency can result in weird situations: when one is
destroyed/unlinked/stopped it needs to unregister itself from the other,
but doing this will trigger unregistration of the other. Hence, let's
make sure we unregister the stream from the server before destroying it,
to break this cycle.

Most likely fixes: #10725
---
 src/resolve/resolved-dns-server.c      | 22 +++++++++++++++++++++-
 src/resolve/resolved-dns-server.h      |  2 ++
 src/resolve/resolved-dns-transaction.c |  2 +-
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c
index c7f9de2cbd..3e69741b88 100644
--- a/src/resolve/resolved-dns-server.c
+++ b/src/resolve/resolved-dns-server.c
@@ -103,7 +103,7 @@ int dns_server_new(
 static DnsServer* dns_server_free(DnsServer *s)  {
         assert(s);
 
-        dns_stream_unref(s->stream);
+        dns_server_unref_stream(s);
 
 #if ENABLE_DNS_OVER_TLS
         dnstls_server_free(s);
@@ -158,6 +158,9 @@ void dns_server_unlink(DnsServer *s) {
         if (s->manager->current_dns_server == s)
                 manager_set_dns_server(s->manager, NULL);
 
+        /* No need to keep a default stream around anymore */
+        dns_server_unref_stream(s);
+
         dns_server_unref(s);
 }
 
@@ -826,6 +829,9 @@ void dns_server_reset_features(DnsServer *s) {
         s->warned_downgrade = false;
 
         dns_server_reset_counters(s);
+
+        /* Let's close the default stream, so that we reprobe with the new features */
+        dns_server_unref_stream(s);
 }
 
 void dns_server_reset_features_all(DnsServer *s) {
@@ -886,6 +892,20 @@ void dns_server_dump(DnsServer *s, FILE *f) {
                 yes_no(s->packet_rrsig_missing));
 }
 
+void dns_server_unref_stream(DnsServer *s) {
+        DnsStream *ref;
+
+        assert(s);
+
+        /* Detaches the default stream of this server. Some special care needs to be taken here, as that stream and
+         * this server reference each other. First, take the stream out of the server. It's destructor will check if it
+         * is registered with us, hence let's invalidate this separatly, so that it is already unregistered. */
+        ref = TAKE_PTR(s->stream);
+
+        /* And then, unref it */
+        dns_stream_unref(ref);
+}
+
 static const char* const dns_server_type_table[_DNS_SERVER_TYPE_MAX] = {
         [DNS_SERVER_SYSTEM] = "system",
         [DNS_SERVER_FALLBACK] = "fallback",
diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h
index a6022ad97f..6e73f32df4 100644
--- a/src/resolve/resolved-dns-server.h
+++ b/src/resolve/resolved-dns-server.h
@@ -151,3 +151,5 @@ void dns_server_reset_features(DnsServer *s);
 void dns_server_reset_features_all(DnsServer *s);
 
 void dns_server_dump(DnsServer *s, FILE *f);
+
+void dns_server_unref_stream(DnsServer *s);
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 73e6306ba1..f29a68e444 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -639,7 +639,7 @@ static int dns_transaction_emit_tcp(DnsTransaction *t) {
 #endif
 
                 if (t->server) {
-                        dns_stream_unref(t->server->stream);
+                        dns_server_unref_stream(t->server);
                         t->server->stream = dns_stream_ref(s);
                         s->server = dns_server_ref(t->server);
                 }