resolve: mdns: calculate required packet size to store questions and authorities

Otherwise, if we have many cached entries or pending transactions with
TYPE_ANY, then dns_transaction_make_packet_mdns() fails with -EMSGSIZE.

This also fixes use-after-free.

Fixes #23894.

diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c
index 0856976d3efdb95df99a47a5300c4f0c98785ff0..f793a659ee89a50fe6317c51aaf6b502068fe1c7 100644 (file)
--- a/src/resolve/resolved-dns-cache.c
+++ b/src/resolve/resolved-dns-cache.c
@@ -1242,16 +1242,14 @@ int dns_cache_check_conflicts(DnsCache *cache, DnsResourceRecord *rr, int owner_
         return 1;
 }
 
-int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p) {
+int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p, usec_t ts, unsigned max_rr) {
         unsigned ancount = 0;
         DnsCacheItem *i;
-        usec_t t;
         int r;
 
         assert(cache);
         assert(p);
-
-        t = now(CLOCK_BOOTTIME);
+        assert(p->protocol == DNS_PROTOCOL_MDNS);
 
         HASHMAP_FOREACH(i, cache->by_key)
                 LIST_FOREACH(by_key, j, i) {
@@ -1263,14 +1261,17 @@ int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p) {
 
                         /* RFC6762 7.1: Don't append records with less than half the TTL remaining
                          * as known answers. */
-                        if (usec_sub_unsigned(j->until, t) < j->rr->ttl * USEC_PER_SEC / 2)
+                        if (usec_sub_unsigned(j->until, ts) < j->rr->ttl * USEC_PER_SEC / 2)
                                 continue;
 
                         r = dns_packet_append_rr(p, j->rr, 0, NULL, NULL);
-                        if (r == -EMSGSIZE && p->protocol == DNS_PROTOCOL_MDNS) {
-                                /* For mDNS, if we're unable to stuff all known answers into the given packet,
-                                 * allocate a new one, push the RR into that one and link it to the current one.
-                                 */
+                        if (r == -EMSGSIZE) {
+                                if (max_rr == 0)
+                                        /* If max_rr == 0, do not allocate more packets. */
+                                        goto finalize;
+
+                                /* If we're unable to stuff all known answers into the given packet, allocate
+                                 * a new one, push the RR into that one and link it to the current one. */
 
                                 DNS_PACKET_HEADER(p)->ancount = htobe16(ancount);
                                 ancount = 0;
@@ -1288,8 +1289,21 @@ int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p) {
                                 return r;
 
                         ancount++;
+                        if (max_rr > 0 && ancount >= max_rr) {
+                                DNS_PACKET_HEADER(p)->ancount = htobe16(ancount);
+                                ancount = 0;
+
+                                r = dns_packet_new_query(&p->more, p->protocol, 0, true);
+                                if (r < 0)
+                                        return r;
+
+                                p = p->more;
+
+                                max_rr = UINT_MAX;
+                        }
                 }
 
+finalize:
         DNS_PACKET_HEADER(p)->ancount = htobe16(ancount);
 
         return 0;

diff --git a/src/resolve/resolved-dns-cache.h b/src/resolve/resolved-dns-cache.h
index 621b52f8926f8267bc55fde75fe93366bef18e1d..fb2e61a65bcab73140dd6f54d585b3ec20410454 100644 (file)
--- a/src/resolve/resolved-dns-cache.h
+++ b/src/resolve/resolved-dns-cache.h
@@ -53,4 +53,4 @@ bool dns_cache_is_empty(DnsCache *cache);
 
 unsigned dns_cache_size(DnsCache *cache);
 
-int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p);
+int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p, usec_t ts, unsigned max_rr);

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 4db9404e1db47457db7439b4d0d3b4c8afc4047d..a300a214e7bb81df8a2717fe8103e73140f00424 100644 (file)
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -1780,13 +1780,30 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) {
         return 1;
 }
 
+static int dns_packet_append_zone(DnsPacket *p, DnsTransaction *t, DnsResourceKey *k, unsigned *nscount) {
+        _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL;
+        bool tentative;
+        int r;
+
+        assert(p);
+        assert(t);
+        assert(k);
+
+        if (k->type != DNS_TYPE_ANY)
+                return 0;
+
+        r = dns_zone_lookup(&t->scope->zone, k, t->scope->link->ifindex, &answer, NULL, &tentative);
+        if (r < 0)
+                return r;
+
+        return dns_packet_append_answer(p, answer, nscount);
+}
+
 static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
         _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
-        bool add_known_answers = false;
-        DnsResourceKey *tkey;
         _cleanup_set_free_ Set *keys = NULL;
-        unsigned qdcount;
-        unsigned nscount = 0;
+        unsigned qdcount, ancount = 0 /* avoid false maybe-uninitialized warning */, nscount;
+        bool add_known_answers = false;
         usec_t ts;
         int r;
 
@@ -1796,6 +1813,7 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
         /* Discard any previously prepared packet, so we can start over and coalesce again */
         t->sent = dns_packet_unref(t->sent);
 
+        /* First, create a dummy packet to calculate packet size. */
         r = dns_packet_new_query(&p, t->scope->protocol, 0, false);
         if (r < 0)
                 return r;
@@ -1809,11 +1827,14 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
         if (dns_key_is_shared(dns_transaction_key(t)))
                 add_known_answers = true;
 
-        if (dns_transaction_key(t)->type == DNS_TYPE_ANY) {
-                r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(t));
-                if (r < 0)
-                        return r;
-        }
+        r = dns_packet_append_zone(p, t, dns_transaction_key(t), NULL);
+        if (r < 0)
+                return r;
+
+        /* Save appended keys */
+        r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(t));
+        if (r < 0)
+                return r;
 
         /*
          * For mDNS, we want to coalesce as many open queries in pending transactions into one single
@@ -1823,79 +1844,114 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
 
         assert_se(sd_event_now(t->scope->manager->event, CLOCK_BOOTTIME, &ts) >= 0);
 
-        LIST_FOREACH(transactions_by_scope, other, t->scope->transactions) {
+        for (bool restart = true; restart;) {
+                restart = false;
+                LIST_FOREACH(transactions_by_scope, other, t->scope->transactions) {
+                        size_t saved_packet_size;
+                        bool append = false;
 
-                /* Skip ourselves */
-                if (other == t)
-                        continue;
+                        /* Skip ourselves */
+                        if (other == t)
+                                continue;
 
-                if (other->state != DNS_TRANSACTION_PENDING)
-                        continue;
+                        if (other->state != DNS_TRANSACTION_PENDING)
+                                continue;
 
-                if (other->next_attempt_after > ts)
-                        continue;
+                        if (other->next_attempt_after > ts)
+                                continue;
 
-                if (qdcount >= UINT16_MAX)
-                        break;
+                        if (!set_contains(keys, dns_transaction_key(other))) {
+                                r = dns_packet_append_key(p, dns_transaction_key(other), 0, &saved_packet_size);
+                                /* If we can't stuff more questions into the packet, just give up.
+                                 * One of the 'other' transactions will fire later and take care of the rest. */
+                                if (r == -EMSGSIZE)
+                                        break;
+                                if (r < 0)
+                                        return r;
 
-                r = dns_packet_append_key(p, dns_transaction_key(other), 0, NULL);
+                                r = dns_packet_append_zone(p, t, dns_transaction_key(other), NULL);
+                                if (r == -EMSGSIZE)
+                                        break;
+                                if (r < 0)
+                                        return r;
 
-                /*
-                 * If we can't stuff more questions into the packet, just give up.
-                 * One of the 'other' transactions will fire later and take care of the rest.
-                 */
-                if (r == -EMSGSIZE)
-                        break;
+                                append = true;
+                        }
 
-                if (r < 0)
-                        return r;
+                        r = dns_transaction_prepare(other, ts);
+                        if (r < 0)
+                                return r;
+                        if (r == 0) {
+                                if (append)
+                                        dns_packet_truncate(p, saved_packet_size);
 
-                r = dns_transaction_prepare(other, ts);
-                if (r <= 0)
-                        continue;
+                                /* In this case, not only this transaction, but multiple transactions may be
+                                 * freed. Hence, we need to restart the loop. */
+                                restart = true;
+                                break;
+                        }
 
-                usec_t timeout = transaction_get_resend_timeout(other);
-                r = dns_transaction_setup_timeout(other, timeout, usec_add(ts, timeout));
-                if (r < 0)
-                        return r;
+                        usec_t timeout = transaction_get_resend_timeout(other);
+                        r = dns_transaction_setup_timeout(other, timeout, usec_add(ts, timeout));
+                        if (r < 0)
+                                return r;
 
-                qdcount++;
+                        if (dns_key_is_shared(dns_transaction_key(other)))
+                                add_known_answers = true;
 
-                if (dns_key_is_shared(dns_transaction_key(other)))
-                        add_known_answers = true;
+                        if (append) {
+                                r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(other));
+                                if (r < 0)
+                                        return r;
+                        }
 
-                if (dns_transaction_key(other)->type == DNS_TYPE_ANY) {
-                        r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(other));
-                        if (r < 0)
-                                return r;
+                        qdcount++;
+                        if (qdcount >= UINT16_MAX)
+                                break;
                 }
         }
 
-        DNS_PACKET_HEADER(p)->qdcount = htobe16(qdcount);
-
         /* Append known answer section if we're asking for any shared record */
         if (add_known_answers) {
-                r = dns_cache_export_shared_to_packet(&t->scope->cache, p);
+                r = dns_cache_export_shared_to_packet(&t->scope->cache, p, ts, 0);
                 if (r < 0)
                         return r;
+
+                ancount = be16toh(DNS_PACKET_HEADER(p)->ancount);
         }
 
-        SET_FOREACH(tkey, keys) {
-                _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL;
-                bool tentative;
+        /* Then, create acctual packet. */
+        p = dns_packet_unref(p);
+        r = dns_packet_new_query(&p, t->scope->protocol, 0, false);
+        if (r < 0)
+                return r;
+
+        /* Questions */
+        DnsResourceKey *k;
+        SET_FOREACH(k, keys) {
+                r = dns_packet_append_key(p, k, 0, NULL);
+                if (r < 0)
+                        return r;
+        }
+        DNS_PACKET_HEADER(p)->qdcount = htobe16(qdcount);
 
-                r = dns_zone_lookup(&t->scope->zone, tkey, t->scope->link->ifindex, &answer, NULL, &tentative);
+        /* Known answers */
+        if (add_known_answers) {
+                r = dns_cache_export_shared_to_packet(&t->scope->cache, p, ts, ancount);
                 if (r < 0)
                         return r;
+        }
 
-                r = dns_packet_append_answer(p, answer, &nscount);
+        /* Authorities */
+        nscount = 0;
+        SET_FOREACH(k, keys) {
+                r = dns_packet_append_zone(p, t, k, &nscount);
                 if (r < 0)
                         return r;
         }
         DNS_PACKET_HEADER(p)->nscount = htobe16(nscount);
 
         t->sent = TAKE_PTR(p);
-
         return 0;
 }