From d50a58e7252b763043485aa79a61094bfae9d7ff Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 1 Jul 2022 14:46:20 +0900 Subject: [PATCH 1/9] resolve: mdns: fix use-after-free Fixes #23843 and #23873. --- src/resolve/resolved-mdns.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c index d5c71f4080..a311f54e44 100644 --- a/src/resolve/resolved-mdns.c +++ b/src/resolve/resolved-mdns.c @@ -410,12 +410,28 @@ static int on_mdns_packet(sd_event_source *s, int fd, uint32_t revents, void *us } } - LIST_FOREACH(transactions_by_scope, t, scope->transactions) { - r = dns_answer_match_key(p->answer, t->key, NULL); - if (r < 0) - log_debug_errno(r, "Failed to match resource key, ignoring: %m"); - else if (r > 0) /* This packet matches the transaction, let's pass it on as reply */ + for (bool match = true; match;) { + match = false; + LIST_FOREACH(transactions_by_scope, t, scope->transactions) { + if (t->state != DNS_TRANSACTION_PENDING) + continue; + + r = dns_answer_match_key(p->answer, dns_transaction_key(t), NULL); + if (r <= 0) { + if (r < 0) + log_debug_errno(r, "Failed to match resource key, ignoring: %m"); + continue; + } + + /* This packet matches the transaction, let's pass it on as reply */ dns_transaction_process_reply(t, p, false); + + /* The dns_transaction_process_reply() -> dns_transaction_complete() -> + * dns_query_candidate_stop() may free multiple transactions. Hence, restart + * the loop. */ + match = true; + break; + } } dns_cache_put(&scope->cache, scope->manager->enable_cache, NULL, DNS_PACKET_RCODE(p), p->answer, NULL, false, _DNSSEC_RESULT_INVALID, UINT32_MAX, p->family, &p->sender); From c3dbb13288ba97a2ba461fd0996862a9819d73d1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Jul 2022 05:24:52 +0900 Subject: [PATCH 2/9] Revert "resolve: mDNS transaction max attempts fix" This reverts commit 127b26f3d8b589907ed75a34d34ab330995778f9. The commit made mDNS queries quite unstable. Note that, RFC 6762 does not explicitly prohibit to send a request multiple times. --- src/resolve/resolved-dns-transaction.c | 2 +- src/resolve/resolved-dns-transaction.h | 34 ++++++-------------------- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 6acf251d60..30429cd9b6 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1622,7 +1622,7 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) { return 0; } - if (t->n_attempts >= dns_transaction_attempts_max(t->scope->protocol, t->probing)) { + if (t->n_attempts >= TRANSACTION_ATTEMPTS_MAX(t->scope->protocol)) { DnsTransactionState result; if (t->scope->protocol == DNS_PROTOCOL_LLMNR) diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index f4cb5d3d8d..498cabb7e5 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -214,31 +214,11 @@ DnsTransactionSource dns_transaction_source_from_string(const char *s) _pure_; /* Maximum attempts to send LLMNR requests, see RFC 4795 Section 2.7 */ #define LLMNR_TRANSACTION_ATTEMPTS_MAX 3 -/* Maximum attempts to send MDNS requests is one except for probe requests, see RFC 6762 Section 8.1 - * RFC 6762 differentiates between normal (single-shot/continuous) and probe requests. - * It therefore makes sense to attempt each normal query only once with no retries. - * Otherwise we'd be sending out three attempts for even a normal query. */ -#define MDNS_TRANSACTION_ATTEMPTS_MAX 1 +/* Maximum attempts to send MDNS requests, see RFC 6762 Section 8.1 */ +#define MDNS_TRANSACTION_ATTEMPTS_MAX 3 -#define MDNS_PROBE_TRANSACTION_ATTEMPTS_MAX 3 - -static inline unsigned dns_transaction_attempts_max(DnsProtocol p, bool probing) { - - switch (p) { - - case DNS_PROTOCOL_LLMNR: - return LLMNR_TRANSACTION_ATTEMPTS_MAX; - - case DNS_PROTOCOL_MDNS: - if (probing) - return MDNS_PROBE_TRANSACTION_ATTEMPTS_MAX; - else - return MDNS_TRANSACTION_ATTEMPTS_MAX; - - case DNS_PROTOCOL_DNS: - return DNS_TRANSACTION_ATTEMPTS_MAX; - - default: - return 0; - } -} +#define TRANSACTION_ATTEMPTS_MAX(p) (((p) == DNS_PROTOCOL_LLMNR) ? \ + LLMNR_TRANSACTION_ATTEMPTS_MAX : \ + (((p) == DNS_PROTOCOL_MDNS) ? \ + MDNS_TRANSACTION_ATTEMPTS_MAX : \ + DNS_TRANSACTION_ATTEMPTS_MAX)) From 4b2ceb8a48c3aeef4147e335b5f31bc2ed4aa6fb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Jul 2022 05:34:25 +0900 Subject: [PATCH 3/9] resolve: drop unnecessary else, and add short comment --- src/resolve/resolved-dns-transaction.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 30429cd9b6..8dfad1dc7d 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1568,11 +1568,12 @@ static usec_t transaction_get_resend_timeout(DnsTransaction *t) { return DNS_TIMEOUT_USEC; case DNS_PROTOCOL_MDNS: - assert(t->n_attempts > 0); if (t->probing) return MDNS_PROBING_INTERVAL_USEC; - else - return (1 << (t->n_attempts - 1)) * USEC_PER_SEC; + + /* See RFC 6762 Section 5.1 suggests that timeout should be a few seconds. */ + assert(t->n_attempts > 0); + return (1 << (t->n_attempts - 1)) * USEC_PER_SEC; case DNS_PROTOCOL_LLMNR: return t->scope->resend_timeout; From 765647ba805727e93ac8607e38c7b60da2aab2dd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Jul 2022 05:36:20 +0900 Subject: [PATCH 4/9] resolve: fix misuse of accuracy parameter in sd_event_add_time() Also, this makes mDNS regular queries sent without delay (except for one caused by the default accuracy of sd-event). Note, RFC 6762 Section 5.2 is about continuous mDNS query, which is not implemented yet. --- src/resolve/resolved-dns-scope.c | 10 +++------- src/resolve/resolved-dns-transaction.c | 17 ++++++++++------- src/resolve/resolved-dns-transaction.h | 4 ---- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index a872e9d255..4fbcd3d449 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -1204,7 +1204,6 @@ static int on_conflict_dispatch(sd_event_source *es, usec_t usec, void *userdata } int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr) { - usec_t jitter; int r; assert(scope); @@ -1233,15 +1232,12 @@ int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr) { if (scope->conflict_event_source) return 0; - random_bytes(&jitter, sizeof(jitter)); - jitter %= LLMNR_JITTER_INTERVAL_USEC; - r = sd_event_add_time_relative( scope->manager->event, &scope->conflict_event_source, CLOCK_BOOTTIME, - jitter, - LLMNR_JITTER_INTERVAL_USEC, + random_u64_range(LLMNR_JITTER_INTERVAL_USEC), + 0, on_conflict_dispatch, scope); if (r < 0) return log_debug_errno(r, "Failed to add conflict dispatch event: %m"); @@ -1511,7 +1507,7 @@ int dns_scope_announce(DnsScope *scope, bool goodbye) { &scope->announce_event_source, CLOCK_BOOTTIME, MDNS_ANNOUNCE_DELAY, - MDNS_JITTER_RANGE_USEC, + 0, on_announcement_timeout, scope); if (r < 0) return log_debug_errno(r, "Failed to schedule second announcement: %m"); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 8dfad1dc7d..b6e94322a0 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1951,10 +1951,12 @@ int dns_transaction_go(DnsTransaction *t) { if (!t->initial_jitter_scheduled && IN_SET(t->scope->protocol, DNS_PROTOCOL_LLMNR, DNS_PROTOCOL_MDNS)) { - usec_t jitter, accuracy; + usec_t jitter; - /* RFC 4795 Section 2.7 suggests all queries should be delayed by a random time from 0 to - * JITTER_INTERVAL. */ + /* RFC 4795 Section 2.7 suggests all LLMNR queries should be delayed by a random time from 0 to + * JITTER_INTERVAL. + * RFC 6762 Section 8.1 suggests initial probe queries should be delayed by a random time from + * 0 to 250ms. */ t->initial_jitter_scheduled = true; @@ -1962,12 +1964,13 @@ int dns_transaction_go(DnsTransaction *t) { case DNS_PROTOCOL_LLMNR: jitter = random_u64_range(LLMNR_JITTER_INTERVAL_USEC); - accuracy = LLMNR_JITTER_INTERVAL_USEC; break; case DNS_PROTOCOL_MDNS: - jitter = usec_add(random_u64_range(MDNS_JITTER_RANGE_USEC), MDNS_JITTER_MIN_USEC); - accuracy = MDNS_JITTER_RANGE_USEC; + if (t->probing) + jitter = random_u64_range(MDNS_PROBING_INTERVAL_USEC); + else + jitter = 0; break; default: assert_not_reached(); @@ -1979,7 +1982,7 @@ int dns_transaction_go(DnsTransaction *t) { t->scope->manager->event, &t->timeout_event_source, CLOCK_BOOTTIME, - jitter, accuracy, + jitter, 0, on_transaction_timeout, t); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index 498cabb7e5..ab86f0f01f 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -201,10 +201,6 @@ DnsTransactionSource dns_transaction_source_from_string(const char *s) _pure_; /* LLMNR Jitter interval, see RFC 4795 Section 7 */ #define LLMNR_JITTER_INTERVAL_USEC (100 * USEC_PER_MSEC) -/* mDNS Jitter interval, see RFC 6762 Section 5.2 */ -#define MDNS_JITTER_MIN_USEC (20 * USEC_PER_MSEC) -#define MDNS_JITTER_RANGE_USEC (100 * USEC_PER_MSEC) - /* mDNS probing interval, see RFC 6762 Section 8.1 */ #define MDNS_PROBING_INTERVAL_USEC (250 * USEC_PER_MSEC) From 87b91644dbc14d9cdf4c614a3ac2dd8c15733cf6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Jul 2022 06:02:21 +0900 Subject: [PATCH 5/9] resolve: introduce dns_transaction_setup_timeout() This also fixes timeout in dns_transaction_make_packet_mdns(), which was incremented multiple times. --- src/resolve/resolved-dns-transaction.c | 73 ++++++++++++-------------- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index b6e94322a0..4db9404e1d 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1551,6 +1551,33 @@ static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdat return 0; } +static int dns_transaction_setup_timeout( + DnsTransaction *t, + usec_t timeout_usec /* relative */, + usec_t next_usec /* CLOCK_BOOTTIME */) { + + int r; + + assert(t); + + dns_transaction_stop_timeout(t); + + r = sd_event_add_time_relative( + t->scope->manager->event, + &t->timeout_event_source, + CLOCK_BOOTTIME, + timeout_usec, 0, + on_transaction_timeout, t); + if (r < 0) + return r; + + (void) sd_event_source_set_description(t->timeout_event_source, "dns-transaction-timeout"); + + t->next_attempt_after = next_usec; + t->state = DNS_TRANSACTION_PENDING; + return 0; +} + static usec_t transaction_get_resend_timeout(DnsTransaction *t) { assert(t); assert(t->scope); @@ -1827,22 +1854,11 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) { if (r <= 0) continue; - ts += transaction_get_resend_timeout(other); - - r = sd_event_add_time( - other->scope->manager->event, - &other->timeout_event_source, - CLOCK_BOOTTIME, - ts, 0, - on_transaction_timeout, other); + usec_t timeout = transaction_get_resend_timeout(other); + r = dns_transaction_setup_timeout(other, timeout, usec_add(ts, timeout)); if (r < 0) return r; - (void) sd_event_source_set_description(other->timeout_event_source, "dns-transaction-timeout"); - - other->state = DNS_TRANSACTION_PENDING; - other->next_attempt_after = ts; - qdcount++; if (dns_key_is_shared(dns_transaction_key(other))) @@ -1959,6 +1975,7 @@ int dns_transaction_go(DnsTransaction *t) { * 0 to 250ms. */ t->initial_jitter_scheduled = true; + t->n_attempts = 0; switch (t->scope->protocol) { @@ -1976,23 +1993,10 @@ int dns_transaction_go(DnsTransaction *t) { assert_not_reached(); } - assert(!t->timeout_event_source); - - r = sd_event_add_time_relative( - t->scope->manager->event, - &t->timeout_event_source, - CLOCK_BOOTTIME, - jitter, 0, - on_transaction_timeout, t); + r = dns_transaction_setup_timeout(t, jitter, ts); if (r < 0) return r; - (void) sd_event_source_set_description(t->timeout_event_source, "dns-transaction-timeout"); - - t->n_attempts = 0; - t->next_attempt_after = ts; - t->state = DNS_TRANSACTION_PENDING; - log_debug("Delaying %s transaction %" PRIu16 " for " USEC_FMT "us.", dns_protocol_to_string(t->scope->protocol), t->id, @@ -2066,22 +2070,11 @@ int dns_transaction_go(DnsTransaction *t) { return dns_transaction_go(t); } - ts += transaction_get_resend_timeout(t); - - r = sd_event_add_time( - t->scope->manager->event, - &t->timeout_event_source, - CLOCK_BOOTTIME, - ts, 0, - on_transaction_timeout, t); + usec_t timeout = transaction_get_resend_timeout(t); + r = dns_transaction_setup_timeout(t, timeout, usec_add(ts, timeout)); if (r < 0) return r; - (void) sd_event_source_set_description(t->timeout_event_source, "dns-transaction-timeout"); - - t->state = DNS_TRANSACTION_PENDING; - t->next_attempt_after = ts; - return 1; } From d3887b2b484004f6d5b393f57b01fe2eb917981f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Jul 2022 06:11:00 +0900 Subject: [PATCH 6/9] resolve: shorten code a bit --- src/resolve/resolved-mdns.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c index a311f54e44..0a66bccba3 100644 --- a/src/resolve/resolved-mdns.c +++ b/src/resolve/resolved-mdns.c @@ -207,10 +207,10 @@ static bool mdns_should_reply_using_unicast(DnsPacket *p) { } /* All the questions in the query had a QU bit set, RFC 6762, section 5.4 */ - DNS_QUESTION_FOREACH_ITEM(item, p->question) { + DNS_QUESTION_FOREACH_ITEM(item, p->question) if (!FLAGS_SET(item->flags, DNS_QUESTION_WANTS_UNICAST_REPLY)) return false; - } + return true; } From f2605af1f2e770818bbc6bad2561acdbd25a38ad Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Jul 2022 11:23:33 +0900 Subject: [PATCH 7/9] resolve: mdns_packet_extract_matching_rrs() may return 0 Fixes the following assertion: --- Assertion 'r > 0' failed at src/resolve/resolved-mdns.c:180, function mdns_do_tiebreak(). Aborting. --- --- src/resolve/resolved-mdns.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c index 0a66bccba3..2ad3a792da 100644 --- a/src/resolve/resolved-mdns.c +++ b/src/resolve/resolved-mdns.c @@ -177,8 +177,6 @@ static int mdns_do_tiebreak(DnsResourceKey *key, DnsAnswer *answer, DnsPacket *p if (r < 0) return r; - assert(r > 0); - if (proposed_rrs_cmp(remote, r, our, size) > 0) return 1; From 055acd4d8b385fd9ff29e49e0c46856a9e705433 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Jul 2022 11:44:46 +0900 Subject: [PATCH 8/9] resolve: do not trigger assertions on invalid query --- src/resolve/resolved-dns-scope.c | 4 +++- src/resolve/resolved-mdns.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 4fbcd3d449..473e397013 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -1011,7 +1011,9 @@ void dns_scope_process_query(DnsScope *s, DnsStream *stream, DnsPacket *p) { return; } - assert(dns_question_size(p->question) == 1); + if (dns_question_size(p->question) != 1) + return (void) log_debug("Received LLMNR query without question or multiple questions, ignoring."); + key = dns_question_first_key(p->question); r = dns_zone_lookup(&s->zone, key, 0, &answer, &soa, &tentative); diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c index 2ad3a792da..e1965c3833 100644 --- a/src/resolve/resolved-mdns.c +++ b/src/resolve/resolved-mdns.c @@ -254,7 +254,8 @@ static int mdns_scope_process_query(DnsScope *s, DnsPacket *p) { if (r < 0) return log_debug_errno(r, "Failed to extract resource records from incoming packet: %m"); - assert_return((dns_question_size(p->question) > 0), -EINVAL); + if (dns_question_size(p->question) <= 0) + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Received mDNS query without question, ignoring."); unicast_reply = mdns_should_reply_using_unicast(p); if (unicast_reply && !sender_on_local_subnet(s, p)) { From 325513bc776c739a814996cc5c483235ca92be86 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Jul 2022 18:09:58 +0900 Subject: [PATCH 9/9] 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. --- src/resolve/resolved-dns-cache.c | 32 +++-- src/resolve/resolved-dns-cache.h | 2 +- src/resolve/resolved-dns-transaction.c | 172 ++++++++++++++++--------- 3 files changed, 138 insertions(+), 68 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 0856976d3e..f793a659ee 100644 --- 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 621b52f892..fb2e61a65b 100644 --- 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 4db9404e1d..a300a214e7 100644 --- 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) - continue; - - 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_transaction_key(other)->type == DNS_TYPE_ANY) { - r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(other)); + r = dns_transaction_prepare(other, ts); if (r < 0) return r; + if (r == 0) { + if (append) + dns_packet_truncate(p, saved_packet_size); + + /* 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; + + 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; + } + + 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); + } + + /* 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); + + /* Known answers */ + if (add_known_answers) { + r = dns_cache_export_shared_to_packet(&t->scope->cache, p, ts, ancount); if (r < 0) return r; } - SET_FOREACH(tkey, keys) { - _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL; - bool tentative; - - r = dns_zone_lookup(&t->scope->zone, tkey, t->scope->link->ifindex, &answer, NULL, &tentative); - 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; }