From 5c274555c3f6d5e188cb7e3e0adafa9ae55d5f49 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 11:32:48 +0900 Subject: [PATCH 1/4] resolve: use dns_name_hash_ops_free for dnssec negative trust anchor --- src/resolve/resolved-dns-trust-anchor.c | 8 ++------ src/resolve/resolved-link-bus.c | 7 +++---- src/resolve/resolved-link.c | 12 ++++++------ 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c index 52764601bb..94e0da404b 100644 --- a/src/resolve/resolved-dns-trust-anchor.c +++ b/src/resolve/resolved-dns-trust-anchor.c @@ -196,10 +196,6 @@ static int dns_trust_anchor_add_builtin_negative(DnsTrustAnchor *d) { if (!set_isempty(d->negative_by_name)) return 0; - r = set_ensure_allocated(&d->negative_by_name, &dns_name_hash_ops); - if (r < 0) - return r; - /* We add a couple of domains as default negative trust * anchors, where it's very unlikely they will be installed in * the root zone. If they exist they must be private, and thus @@ -209,7 +205,7 @@ static int dns_trust_anchor_add_builtin_negative(DnsTrustAnchor *d) { if (dns_trust_anchor_knows_domain_positive(d, name)) continue; - r = set_put_strdup(&d->negative_by_name, name); + r = set_put_strdup_full(&d->negative_by_name, &dns_name_hash_ops_free, name); if (r < 0) return r; } @@ -547,7 +543,7 @@ void dns_trust_anchor_flush(DnsTrustAnchor *d) { d->positive_by_key = hashmap_free_with_destructor(d->positive_by_key, dns_answer_unref); d->revoked_by_rr = set_free_with_destructor(d->revoked_by_rr, dns_resource_record_unref); - d->negative_by_name = set_free_free(d->negative_by_name); + d->negative_by_name = set_free(d->negative_by_name); } int dns_trust_anchor_lookup_positive(DnsTrustAnchor *d, const DnsResourceKey *key, DnsAnswer **ret) { diff --git a/src/resolve/resolved-link-bus.c b/src/resolve/resolved-link-bus.c index 77d2663e4f..c5c986bd53 100644 --- a/src/resolve/resolved-link-bus.c +++ b/src/resolve/resolved-link-bus.c @@ -658,7 +658,7 @@ int bus_link_method_set_dnssec(sd_bus_message *message, void *userdata, sd_bus_e } int bus_link_method_set_dnssec_negative_trust_anchors(sd_bus_message *message, void *userdata, sd_bus_error *error) { - _cleanup_set_free_free_ Set *ns = NULL; + _cleanup_set_free_ Set *ns = NULL; _cleanup_strv_free_ char **ntas = NULL; _cleanup_free_ char *j = NULL; Link *l = ASSERT_PTR(userdata); @@ -670,7 +670,7 @@ int bus_link_method_set_dnssec_negative_trust_anchors(sd_bus_message *message, v if (r < 0) return r; - ns = set_new(&dns_name_hash_ops); + ns = set_new(&dns_name_hash_ops_free); if (!ns) return -ENOMEM; @@ -708,8 +708,7 @@ int bus_link_method_set_dnssec_negative_trust_anchors(sd_bus_message *message, v bus_client_log(message, "DNSSEC NTA change"); if (!set_equal(ns, l->dnssec_negative_trust_anchors)) { - set_free_free(l->dnssec_negative_trust_anchors); - l->dnssec_negative_trust_anchors = TAKE_PTR(ns); + set_free_and_replace(l->dnssec_negative_trust_anchors, ns); (void) link_save_user(l); diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index df4b5993a3..5d0bcc6a80 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -73,7 +73,7 @@ void link_flush_settings(Link *l) { dns_server_unlink_all(l->dns_servers); dns_search_domain_unlink_all(l->search_domains); - l->dnssec_negative_trust_anchors = set_free_free(l->dnssec_negative_trust_anchors); + l->dnssec_negative_trust_anchors = set_free(l->dnssec_negative_trust_anchors); } Link *link_free(Link *l) { @@ -459,12 +459,12 @@ static int link_update_dnssec_mode(Link *l) { static int link_update_dnssec_negative_trust_anchors(Link *l) { _cleanup_strv_free_ char **ntas = NULL; - _cleanup_set_free_free_ Set *ns = NULL; + _cleanup_set_free_ Set *ns = NULL; int r; assert(l); - l->dnssec_negative_trust_anchors = set_free_free(l->dnssec_negative_trust_anchors); + l->dnssec_negative_trust_anchors = set_free(l->dnssec_negative_trust_anchors); r = sd_network_link_get_dnssec_negative_trust_anchors(l->ifindex, &ntas); if (r == -ENODATA) @@ -472,7 +472,7 @@ static int link_update_dnssec_negative_trust_anchors(Link *l) { if (r < 0) return r; - ns = set_new(&dns_name_hash_ops); + ns = set_new(&dns_name_hash_ops_free); if (!ns) return -ENOMEM; @@ -1425,9 +1425,9 @@ int link_load_user(Link *l) { } if (ntas) { - _cleanup_set_free_free_ Set *ns = NULL; + _cleanup_set_free_ Set *ns = NULL; - ns = set_new(&dns_name_hash_ops); + ns = set_new(&dns_name_hash_ops_free); if (!ns) { r = -ENOMEM; goto fail; From f7880e58f2f1b5a4f2fed83e4ea93926d902fbea Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Apr 2025 23:19:51 +0900 Subject: [PATCH 2/4] resolved-dns-scope: use hash_ops with destructor for conflict_queue --- src/resolve/resolved-dns-rr.c | 9 +++++++++ src/resolve/resolved-dns-rr.h | 1 + src/resolve/resolved-dns-scope.c | 25 +++++++------------------ 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 2ad7deab78..7f0bd35d3c 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -1688,6 +1688,15 @@ int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResour DEFINE_HASH_OPS(dns_resource_record_hash_ops, DnsResourceRecord, dns_resource_record_hash_func, dns_resource_record_compare_func); +DEFINE_HASH_OPS_FULL( + dns_resource_record_hash_ops_by_key, + DnsResourceKey, + dns_resource_key_hash_func, + dns_resource_key_compare_func, + dns_resource_key_unref, + DnsResourceRecord, + dns_resource_record_unref); + DnsResourceRecord *dns_resource_record_copy(DnsResourceRecord *rr) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *copy = NULL; DnsResourceRecord *t; diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 23bf2f66d4..e039e43cec 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -414,6 +414,7 @@ int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResour extern const struct hash_ops dns_resource_key_hash_ops; extern const struct hash_ops dns_resource_record_hash_ops; +extern const struct hash_ops dns_resource_record_hash_ops_by_key; int dnssec_algorithm_to_string_alloc(int i, char **ret); int dnssec_algorithm_from_string(const char *s) _pure_; diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 3f137468cc..33e1732183 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -112,7 +112,7 @@ DnsScope* dns_scope_free(DnsScope *s) { hashmap_free(s->transactions_by_key); - ordered_hashmap_free_with_destructor(s->conflict_queue, dns_resource_record_unref); + ordered_hashmap_free(s->conflict_queue); sd_event_source_disable_unref(s->conflict_event_source); sd_event_source_disable_unref(s->announce_event_source); @@ -1238,13 +1238,10 @@ static int on_conflict_dispatch(sd_event_source *es, usec_t usec, void *userdata _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr = NULL; _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; - key = ordered_hashmap_first_key(scope->conflict_queue); - if (!key) + rr = ordered_hashmap_steal_first_key_and_value(scope->conflict_queue, (void**) &key); + if (!rr) 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"); @@ -1265,18 +1262,10 @@ int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr) { assert(scope); assert(rr); - /* We don't send these queries immediately. Instead, we queue - * them, and send them after some jitter delay. */ - r = ordered_hashmap_ensure_allocated(&scope->conflict_queue, &dns_resource_key_hash_ops); - if (r < 0) { - log_oom(); - return r; - } - - /* We only place one RR per key in the conflict - * messages, not all of them. That should be enough to - * indicate where there might be a conflict */ - r = ordered_hashmap_put(scope->conflict_queue, rr->key, rr); + /* We don't send these queries immediately. Instead, we queue them, and send them after some jitter + * delay. We only place one RR per key in the conflict messages, not all of them. That should be + * enough to indicate where there might be a conflict */ + r = ordered_hashmap_ensure_put(&scope->conflict_queue, &dns_resource_record_hash_ops_by_key, rr->key, rr); if (IN_SET(r, 0, -EEXIST)) return 0; if (r < 0) From 442ac1b1b0392aa1a714e3c6508bd91059e1a059 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Apr 2025 23:24:21 +0900 Subject: [PATCH 3/4] resolved-dns-trust-anchor: use hash_ops with destructor for managing RRs --- src/resolve/resolved-dns-rr.c | 7 ++++++- src/resolve/resolved-dns-trust-anchor.c | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 7f0bd35d3c..6837c9ece4 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -1686,7 +1686,12 @@ int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResour return CMP(x, y); } -DEFINE_HASH_OPS(dns_resource_record_hash_ops, DnsResourceRecord, dns_resource_record_hash_func, dns_resource_record_compare_func); +DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR( + dns_resource_record_hash_ops, + DnsResourceRecord, + dns_resource_record_hash_func, + dns_resource_record_compare_func, + dns_resource_record_unref); DEFINE_HASH_OPS_FULL( dns_resource_record_hash_ops_by_key, diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c index 94e0da404b..ae20f712cb 100644 --- a/src/resolve/resolved-dns-trust-anchor.c +++ b/src/resolve/resolved-dns-trust-anchor.c @@ -542,7 +542,7 @@ void dns_trust_anchor_flush(DnsTrustAnchor *d) { assert(d); d->positive_by_key = hashmap_free_with_destructor(d->positive_by_key, dns_answer_unref); - d->revoked_by_rr = set_free_with_destructor(d->revoked_by_rr, dns_resource_record_unref); + d->revoked_by_rr = set_free(d->revoked_by_rr); d->negative_by_name = set_free(d->negative_by_name); } From d6c8db650d54a8095e82e8cbb9889161e1cf8493 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Apr 2025 23:30:57 +0900 Subject: [PATCH 4/4] resolved-dns-trust-anchor: use hash_ops with destructor for managing DnsAnswer --- src/resolve/resolved-dns-rr.c | 4 ++-- src/resolve/resolved-dns-rr.h | 2 ++ src/resolve/resolved-dns-trust-anchor.c | 22 +++++++++++++--------- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 6837c9ece4..a78085d0d4 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -308,7 +308,7 @@ int dns_resource_key_match_soa(const DnsResourceKey *key, const DnsResourceKey * return dns_name_endswith(dns_resource_key_name(key), dns_resource_key_name(soa)); } -static void dns_resource_key_hash_func(const DnsResourceKey *k, struct siphash *state) { +void dns_resource_key_hash_func(const DnsResourceKey *k, struct siphash *state) { assert(k); dns_name_hash_func(dns_resource_key_name(k), state); @@ -316,7 +316,7 @@ static void dns_resource_key_hash_func(const DnsResourceKey *k, struct siphash * siphash24_compress_typesafe(k->type, state); } -static int dns_resource_key_compare_func(const DnsResourceKey *x, const DnsResourceKey *y) { +int dns_resource_key_compare_func(const DnsResourceKey *x, const DnsResourceKey *y) { int r; r = dns_name_compare_func(dns_resource_key_name(x), dns_resource_key_name(y)); diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index e039e43cec..3ad98ee571 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -409,6 +409,8 @@ int dns_resource_key_to_json(DnsResourceKey *key, sd_json_variant **ret); int dns_resource_key_from_json(sd_json_variant *v, DnsResourceKey **ret); int dns_resource_record_to_json(DnsResourceRecord *rr, sd_json_variant **ret); +void dns_resource_key_hash_func(const DnsResourceKey *k, struct siphash *state); +int dns_resource_key_compare_func(const DnsResourceKey *x, const DnsResourceKey *y); void dns_resource_record_hash_func(const DnsResourceRecord *i, struct siphash *state); int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y); diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c index ae20f712cb..962e159a12 100644 --- a/src/resolve/resolved-dns-trust-anchor.c +++ b/src/resolve/resolved-dns-trust-anchor.c @@ -18,6 +18,14 @@ #include "string-util.h" #include "strv.h" +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + dns_answer_hash_ops_by_key, + DnsResourceKey, + dns_resource_key_hash_func, + dns_resource_key_compare_func, + DnsAnswer, + dns_answer_unref); + static const char trust_anchor_dirs[] = CONF_PATHS_NULSTR("dnssec-trust-anchors.d"); /* The second DS RR from https://data.iana.org/root-anchors/root-anchors.xml, retrieved February 2017 */ @@ -78,10 +86,6 @@ static int dns_trust_anchor_add_builtin_positive(DnsTrustAnchor *d) { assert(d); - r = hashmap_ensure_allocated(&d->positive_by_key, &dns_resource_key_hash_ops); - if (r < 0) - return r; - /* Only add the built-in trust anchor if there's neither a DS nor a DNSKEY defined for the root domain. That * way users have an easy way to override the root domain DS/DNSKEY data. */ if (dns_trust_anchor_knows_domain_positive(d, ".")) @@ -103,11 +107,11 @@ static int dns_trust_anchor_add_builtin_positive(DnsTrustAnchor *d) { if (r < 0) return r; - r = hashmap_put(d->positive_by_key, key, answer); + r = hashmap_ensure_put(&d->positive_by_key, &dns_answer_hash_ops_by_key, key, answer); if (r < 0) return r; - answer = NULL; + TAKE_PTR(answer); return 0; } @@ -365,7 +369,7 @@ static int dns_trust_anchor_load_positive(DnsTrustAnchor *d, const char *path, u return -EINVAL; } - r = hashmap_ensure_allocated(&d->positive_by_key, &dns_resource_key_hash_ops); + r = hashmap_ensure_allocated(&d->positive_by_key, &dns_answer_hash_ops_by_key); if (r < 0) return log_oom(); @@ -381,7 +385,7 @@ static int dns_trust_anchor_load_positive(DnsTrustAnchor *d, const char *path, u return log_error_errno(r, "Failed to add answer to trust anchor: %m"); old_answer = dns_answer_unref(old_answer); - answer = NULL; + TAKE_PTR(answer); return 0; } @@ -541,7 +545,7 @@ int dns_trust_anchor_load(DnsTrustAnchor *d) { void dns_trust_anchor_flush(DnsTrustAnchor *d) { assert(d); - d->positive_by_key = hashmap_free_with_destructor(d->positive_by_key, dns_answer_unref); + d->positive_by_key = hashmap_free(d->positive_by_key); d->revoked_by_rr = set_free(d->revoked_by_rr); d->negative_by_name = set_free(d->negative_by_name); }