From 432d108c25a9705f1564d7620c38cdf890df40ba Mon Sep 17 00:00:00 2001 From: Dmitry Rozhkov Date: Wed, 3 Jan 2018 14:26:53 +0200 Subject: [PATCH 1/3] resolved: fix refcounting DnsScope's conflict_queue Refcounting for a RR's key is done separately from refcounting for the RR itself, but in dns_scope_notify_conflict() we don't do that. This may lead to a situation when a RR key put in the conflict_queue hash as a value's key gets freed upon cache reduction when it's still referenced by the hash. Thus increase refcount for the key when putting it into the hash and unreference it upon removing from the hash. Closes #6456 --- src/resolve/resolved-dns-scope.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index ea4459a89a..9247bb34e6 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -885,13 +885,17 @@ static int on_conflict_dispatch(sd_event_source *es, usec_t usec, void *userdata scope->conflict_event_source = sd_event_source_unref(scope->conflict_event_source); for (;;) { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr = NULL; _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; - rr = ordered_hashmap_steal_first(scope->conflict_queue); - if (!rr) + key = ordered_hashmap_first_key(scope->conflict_queue); + if (!key) break; + rr = ordered_hashmap_remove(scope->conflict_queue, key); + assert(rr); + r = dns_scope_make_conflict_packet(scope, rr, &p); if (r < 0) { log_error_errno(r, "Failed to make conflict packet: %m"); @@ -930,6 +934,7 @@ int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr) { if (r < 0) return log_debug_errno(r, "Failed to queue conflicting RR: %m"); + dns_resource_key_ref(rr->key); dns_resource_record_ref(rr); if (scope->conflict_event_source) From cfcc8dcc86b4c18cc5885031c661c7f9ae32f781 Mon Sep 17 00:00:00 2001 From: Dmitry Rozhkov Date: Wed, 3 Jan 2018 14:42:13 +0200 Subject: [PATCH 2/3] resolved: skip conflict notifications for DNS-SD PTR RRs Enumerating DNS-SD PTR resource records are a special case and are supposed to have non-unique keys pointing to services of the same type running on different hosts. There's no need for them to be checked for conflicts. Thus don't check for conflicts such RRs. --- src/resolve/resolved-dns-scope.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 9247bb34e6..9935398c17 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -991,6 +991,10 @@ void dns_scope_check_conflicts(DnsScope *scope, DnsPacket *p) { for (i = 0; i < p->answer->n_rrs; i++) { + /* No conflict if it is DNS-SD RR used for service enumeration. */ + if (dns_resource_key_is_dnssd_ptr(p->answer->items[i].rr->key)) + continue; + /* Check for conflicts against the local zone. If we * found one, we won't check any further */ r = dns_zone_check_conflicts(&scope->zone, p->answer->items[i].rr); From c1227a1840ea1aa42f77cf9d7a03c3eef070b8a9 Mon Sep 17 00:00:00 2001 From: Dmitry Rozhkov Date: Wed, 3 Jan 2018 15:00:27 +0200 Subject: [PATCH 3/3] resolved: use DNS_ANSWER_FOREACH instead of for --- src/resolve/resolved-dns-scope.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 9935398c17..bf6aac8300 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -958,7 +958,7 @@ int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr) { } void dns_scope_check_conflicts(DnsScope *scope, DnsPacket *p) { - unsigned i; + DnsResourceRecord *rr; int r; assert(scope); @@ -989,25 +989,24 @@ void dns_scope_check_conflicts(DnsScope *scope, DnsPacket *p) { log_debug("Checking for conflicts..."); - for (i = 0; i < p->answer->n_rrs; i++) { - + DNS_ANSWER_FOREACH(rr, p->answer) { /* No conflict if it is DNS-SD RR used for service enumeration. */ - if (dns_resource_key_is_dnssd_ptr(p->answer->items[i].rr->key)) + if (dns_resource_key_is_dnssd_ptr(rr->key)) continue; /* Check for conflicts against the local zone. If we * found one, we won't check any further */ - r = dns_zone_check_conflicts(&scope->zone, p->answer->items[i].rr); + r = dns_zone_check_conflicts(&scope->zone, rr); if (r != 0) continue; /* Check for conflicts against the local cache. If so, * send out an advisory query, to inform everybody */ - r = dns_cache_check_conflicts(&scope->cache, p->answer->items[i].rr, p->family, &p->sender); + r = dns_cache_check_conflicts(&scope->cache, rr, p->family, &p->sender); if (r <= 0) continue; - dns_scope_notify_conflict(scope, p->answer->items[i].rr); + dns_scope_notify_conflict(scope, rr); } }