From 4d56ec8b6860651e8d2aa8a9b2a19f37cf651ab4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 3 Jun 2020 12:53:27 +0200 Subject: [PATCH 1/4] resolved: add dns_query_candidate_freep() (cherry picked from commit 7877e5ca7c7f654478d7bce458a28edb7a157fa3) Related: RHEL-93425 --- src/resolve/resolved-dns-query.c | 13 +++++-------- src/resolve/resolved-dns-query.h | 2 ++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 573e27d662..13276abdcd 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -544,7 +544,7 @@ static int on_query_timeout(sd_event_source *s, usec_t usec, void *userdata) { } static int dns_query_add_candidate(DnsQuery *q, DnsScope *s) { - DnsQueryCandidate *c; + _cleanup_(dns_query_candidate_freep) DnsQueryCandidate *c = NULL; int r; assert(q); @@ -558,25 +558,22 @@ static int dns_query_add_candidate(DnsQuery *q, DnsScope *s) { if ((q->flags & SD_RESOLVED_NO_SEARCH) == 0) { r = dns_scope_name_needs_search_domain(s, dns_question_first_name(q->question_idna)); if (r < 0) - goto fail; + return r; if (r > 0) { /* OK, we need a search domain now. Let's find one for this scope */ r = dns_query_candidate_next_search_domain(c); if (r <= 0) /* if there's no search domain, then we won't add any transaction. */ - goto fail; + return r; } } r = dns_query_candidate_setup_transactions(c); if (r < 0) - goto fail; + return r; + TAKE_PTR(c); return 0; - -fail: - dns_query_candidate_free(c); - return r; } static int dns_query_synthesize_reply(DnsQuery *q, DnsTransactionState *state) { diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index 5ee946bc75..fd70b15311 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -102,6 +102,8 @@ enum { }; DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c); +DEFINE_TRIVIAL_CLEANUP_FUNC(DnsQueryCandidate*, dns_query_candidate_free); + void dns_query_candidate_notify(DnsQueryCandidate *c); int dns_query_new(Manager *m, DnsQuery **q, DnsQuestion *question_utf8, DnsQuestion *question_idna, int family, uint64_t flags); From eec31ef9a8c6ad4d9045f73185e14614eadeb535 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 17 Jan 2021 19:51:28 +0100 Subject: [PATCH 2/4] resolved: fix use-after-free with queries hitting the cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When dns_transaction_complete() manages to resolve a query, it invalidates the query candidate object. It shall not be accessed afterwards. We have the following chain of calls: dns_query_candidate_go → dns_transaction_go → dns_transaction_prepare → dns_cache_lookup (success: 1) → dns_transaction_complete After returning back to dns_query_candidate_go(), we'd attempt to continue iteration over the list of transactions attached to the query candidate, accessing already freed (and overwritten) memory: (gdb) bt 0 0x00007f637297cf47 in hashmap_iterate_entry (i=0x7ffe7e15cc90, h=0x706f746b73656465) at ../src/basic/hashmap.c:703 1 _hashmap_iterate (h=0x706f746b73656465, i=i@entry=0x7ffe7e15cc90, value=value@entry=0x7ffe7e15cc88, key=key@entry=0x0) at ../src/basic/hashmap.c:712 2 0x00007f637297d01b in set_iterate (s=, i=i@entry=0x7ffe7e15cc90, value=value@entry=0x7ffe7e15cc88) at ../src/basic/hashmap.c:733 hence we crash 3 0x0000557bc99eb80f in dns_query_candidate_go (c=c@entry=0x557bcaf86890) at ../src/resolve/resolved-dns-query.c:139 ...but c is not valid here in the second iteration of the loop 4 0x0000557bc99eb720 in dns_query_candidate_notify (c=0x557bcaf86890) at ../src/resolve/resolved-dns-query.c:271 c was valid here at entry... 5 0x0000557bc99efe28 in dns_transaction_complete (t=0x557bcac072f0, state=) at ../src/resolve/resolved-dns-transaction.c:350 t is a valid transaction (11481 in the backtrace below) 6 0x0000557bc99f1efb in dns_transaction_process_reply (t=0x557bcac072f0, p=) at ../src/resolve/resolved-dns-transaction.c:1171 7 0x0000557bc99f2d41 in on_dns_packet (s=, fd=, revents=, userdata=0x557bcac072f0) at ../src/resolve/resolved-dns-transaction.c:1223 8 0x00007f6372a25217 in source_dispatch (s=s@entry=0x557bcb162c50) at ../src/libsystemd/sd-event/sd-event.c:3181 9 0x00007f6372a254fd in sd_event_dispatch (e=0x557bcb15b050) at ../src/libsystemd/sd-event/sd-event.c:3620 10 0x00007f6372a267c8 in sd_event_run (e=e@entry=0x557bcb15b050, timeout=timeout@entry=18446744073709551615) at ../src/libsystemd/sd-event/sd-event.c:3678 11 0x00007f6372a269ef in sd_event_loop (e=0x557bcb15b050) at ../src/libsystemd/sd-event/sd-event.c:3700 12 0x0000557bc99ddc14 in run (argc=, argv=) at ../src/resolve/resolved.c:92 13 0x0000557bc99d260a in main (argc=, argv=) at ../src/resolve/resolved.c:99 xxx.name.net systemd-resolved[31705]: Got message type=method_call sender=:1.3644 destination=org.freedesktop.resolve1 path=/org/freedesktop/resolve1 interface=org.freedesktop.resolve1.Manager member=ResolveHostname cookie=2 reply_cookie=0 signature=isit error-name=n/a error-message=n/a xxx.name.net systemd-resolved[31705]: idn2_lookup_u8: xxx → xxx xxx.name.net systemd-resolved[31705]: Looking up RR for xxx IN A. xxx.name.net systemd-resolved[31705]: Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=AddMatch cookie=1102 reply_cookie=0 signature=s error-name=n/a error-message=n/a xxx.name.net systemd-resolved[31705]: Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=GetNameOwner cookie=1103 reply_cookie=0 signature=s error-name=n/a error-message=n/a xxx.name.net systemd-resolved[31705]: Got message type=method_return sender=org.freedesktop.DBus destination=:1.3324 path=n/a interface=n/a member=n/a cookie=4294967295 reply_cookie=1103 signature=s error-name=n/a error-message=n/a xxx.name.net systemd-resolved[31705]: Cache miss for xxx.name.net IN A xxx.name.net systemd-resolved[31705]: Transaction 11481 for scope dns on enp42s0/*. xxx.name.net systemd-resolved[31705]: Using feature level UDP for transaction 11481. xxx.name.net systemd-resolved[31705]: Using DNS server 192.168.1.1 for transaction 11481. xxx.name.net systemd-resolved[31705]: Sending query packet with id 11481 of size 35. xxx.name.net systemd-resolved[31705]: Got message type=method_return sender=org.freedesktop.DBus destination=:1.3324 path=n/a interface=n/a member=n/a cookie=4294967295 reply_cookie=1102 signature= error-name=n/a error-message=n/a xxx.name.net systemd-resolved[31705]: Match type='signal',sender='org.freedesktop.DBus',path='/org/freedesktop/DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged',arg0=':1.3644' successfully installed. xxx.name.net systemd-resolved[31705]: Processing incoming packet on transaction 11481 (rcode=NXDOMAIN). xxx.name.net systemd-resolved[31705]: Not caching negative entry without a SOA record: xxx.name.net IN A xxx.name.net systemd-resolved[31705]: Transaction 11481 for on scope dns on enp42s0/* now complete with from network (unsigned). xxx.name.net systemd-resolved[31705]: Positive cache hit for xxx.lan IN A xxx.name.net systemd-resolved[31705]: Transaction 64364 for on scope dns on enp42s0/* now complete with from cache (unsigned). xxx.name.net systemd-resolved[31705]: Sent message type=method_return sender=n/a destination=:1.3644 path=n/a interface=n/a member=n/a cookie=1104 reply_cookie=2 signature=a(iiay)st error-name=n/a error-message=n/a xxx.name.net systemd-resolved[31705]: Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=RemoveMatch cookie=1105 reply_cookie=0 signature=s error-name=n/a error-message=n/a xxx.name.net systemd-resolved[31705]: Freeing transaction 64364. xxx.name.net systemd[1]: systemd-resolved.service: Main process exited, code=dumped, status=11/SEGV xxx.name.net systemd[1]: systemd-resolved.service: Failed with result 'core-dump'. Fixes #16168, https://bugzilla.redhat.com/show_bug.cgi?id=1895937. (cherry picked from commit 4ea8b443de8be0f7a932f325dfafa1ee2a843795) Resolves: RHEL-93425 --- src/resolve/resolved-dns-query.c | 14 ++++++++++++-- src/resolve/resolved-dns-transaction.c | 16 ++++++++++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 13276abdcd..55ce0e0a63 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -146,23 +146,33 @@ static int dns_query_candidate_go(DnsQueryCandidate *c) { Iterator i; int r; unsigned n = 0; + bool notify = false; assert(c); + c->query->block_ready++; + /* Start the transactions that are not started yet */ SET_FOREACH(t, c->transactions, i) { if (t->state != DNS_TRANSACTION_NULL) continue; r = dns_transaction_go(t); - if (r < 0) + if (r < 0) { + c->query->block_ready--; return r; + } + if (r == 0) + /* A transaction is complete. */ + notify = true; n++; } + c->query->block_ready--; + /* If there was nothing to start, then let's proceed immediately */ - if (n == 0) + if (n == 0 || notify) dns_query_candidate_notify(c); return 0; diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 1ca6c9abc8..5a2b254526 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -175,8 +175,8 @@ bool dns_transaction_gc(DnsTransaction *t) { static uint16_t pick_new_id(Manager *m) { uint16_t new_id; - /* Find a fresh, unused transaction id. Note that this loop is bounded because there's a limit on the number of - * transactions, and it's much lower than the space of IDs. */ + /* Find a fresh, unused transaction id. Note that this loop is bounded because there's a limit on the + * number of transactions, and it's much lower than the space of IDs. */ assert_cc(TRANSACTIONS_MAX < 0xFFFF); @@ -1404,6 +1404,10 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) { assert(t); + /* Returns 0 if dns_transaction_complete() has been called. In that case the transaction and query + * candidate objects may have been invalidated and must not be accessed. Returns 1 if the transaction + * has been prepared. */ + dns_transaction_stop_timeout(t); r = dns_scope_network_good(t->scope); @@ -1525,7 +1529,6 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) { } static int dns_transaction_make_packet_mdns(DnsTransaction *t) { - _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; bool add_known_answers = false; DnsTransaction *other; @@ -1702,8 +1705,9 @@ int dns_transaction_go(DnsTransaction *t) { assert(t); - /* Returns > 0 if the transaction is now pending, returns 0 if could be processed immediately and has finished - * now. */ + /* Returns > 0 if the transaction is now pending, returns 0 if could be processed immediately and has + * finished now. In the latter case, the transaction and query candidate objects must not be accessed. + */ assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &ts) >= 0); @@ -1761,7 +1765,7 @@ int dns_transaction_go(DnsTransaction *t) { t->state = DNS_TRANSACTION_PENDING; log_debug("Delaying %s transaction for " USEC_FMT "us.", dns_protocol_to_string(t->scope->protocol), jitter); - return 0; + return 1; } /* Otherwise, we need to ask the network */ From 0243672d2dbea9094f6b81411b011607e9d02604 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 15 May 2025 07:18:38 +0900 Subject: [PATCH 3/4] resolve: exit from loop for transactions when transactions has been regenerated Fixes #37458. (cherry picked from commit 5814acca9aa4354d121de4bf174851f092a6b643) Related: RHEL-93425 --- src/resolve/resolved-dns-query.c | 11 +++++++++++ src/resolve/resolved-dns-query.h | 1 + 2 files changed, 12 insertions(+) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 55ce0e0a63..6972cc4f29 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -152,6 +152,8 @@ static int dns_query_candidate_go(DnsQueryCandidate *c) { c->query->block_ready++; + uint64_t generation = c->generation; + /* Start the transactions that are not started yet */ SET_FOREACH(t, c->transactions, i) { if (t->state != DNS_TRANSACTION_NULL) @@ -166,6 +168,13 @@ static int dns_query_candidate_go(DnsQueryCandidate *c) { /* A transaction is complete. */ notify = true; + if (c->generation != generation) + /* The transaction has been completed, and dns_transaction_complete() -> + * dns_query_candidate_notify() has been already called. Moreover, the query + * candidate has been regenerated, and the query should be already restarted. + * Let's exit from the loop now. */ + return 0; + n++; } @@ -257,6 +266,8 @@ static int dns_query_candidate_setup_transactions(DnsQueryCandidate *c) { dns_query_candidate_stop(c); + c->generation++; + question = dns_query_question_for_protocol(c->query, c->scope->protocol); /* Create one transaction per question key */ diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index fd70b15311..09d3f9cc92 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -20,6 +20,7 @@ struct DnsQueryCandidate { DnsSearchDomain *search_domain; int error_code; + uint64_t generation; Set *transactions; LIST_FIELDS(DnsQueryCandidate, candidates_by_query); From 19086f73456c4f5dd13b8682bd242fbe54beb1e9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 23 Nov 2023 00:32:48 +0900 Subject: [PATCH 4/4] locale-util: do not call setlocale() when multi-threaded Fixes #30141. (cherry picked from commit ca13432d600593b8eda76721118763b63746eb33) Related: RHEL-93425 --- src/basic/locale-util.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/basic/locale-util.c b/src/basic/locale-util.c index 42ef309ebd..99cc7b5ef5 100644 --- a/src/basic/locale-util.c +++ b/src/basic/locale-util.c @@ -20,6 +20,7 @@ #include "fd-util.h" #include "hashmap.h" #include "locale-util.h" +#include "missing_syscall.h" #include "path-util.h" #include "set.h" #include "string-table.h" @@ -234,6 +235,12 @@ bool is_locale_utf8(void) { if (cached_answer >= 0) goto out; + /* This function may be called from libsystemd, and setlocale() is not thread safe. Assuming yes. */ + if (gettid() != raw_getpid()) { + cached_answer = true; + goto out; + } + if (!setlocale(LC_ALL, "")) { cached_answer = true; goto out;