(RHEL-93425) resolved: fix use-after-free with queries hitting the cache

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;

src/resolve/resolved-dns-query.c

@@ -146,23 +146,42 @@ static int dns_query_candidate_go(DnsQueryCandidate *c) {
         Iterator i;
         int r;
         unsigned n = 0;
+        bool notify = false;
 
         assert(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)
                         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;
+
+                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++;
         }
 
+        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;
@@ -247,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 */
@@ -544,7 +565,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 +579,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) {

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);
@@ -102,6 +103,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);

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 */