hostip.c: fix leak of addrinfo

When creating a dns entry, the addrinfo is passed into the entry on
success and needed deallocation by the caller on failure.

Change the signature to have Curl_dnscache_mk_entry() *always* take
ownership of the addrinfo, even on failure. Change parameter to address
of pointer so that call always clears it.

This makes the handling of failures to Curl_dnscache_mk_entry() simpler.

Fixes #20465
Closes #20468

diff --git a/lib/asyn-ares.c b/lib/asyn-ares.c
index 0d4a28d8348670b9b9a0afb637ac4266fdc0aedd..80126ce056d5aafd41d5e154bf2a133690155578 100644 (file)
--- a/lib/asyn-ares.c
+++ b/lib/asyn-ares.c
@@ -332,14 +332,13 @@ CURLcode Curl_async_is_resolved(struct Curl_easy *data,
     result = ares->result;
     if(ares->ares_status == ARES_SUCCESS && !result) {
       data->state.async.dns =
-        Curl_dnscache_mk_entry(data, ares->temp_ai,
+        Curl_dnscache_mk_entry(data, &ares->temp_ai,
                                data->state.async.hostname, 0,
                                data->state.async.port, FALSE);
       if(!data->state.async.dns) {
         result = CURLE_OUT_OF_MEMORY;
         goto out;
       }
-      ares->temp_ai = NULL; /* temp_ai now owned by entry */
 #ifdef HTTPSRR_WORKS
       {
         struct Curl_https_rrinfo *lhrr = Curl_httpsrr_dup_move(&ares->hinfo);

diff --git a/lib/asyn-thrdd.c b/lib/asyn-thrdd.c
index 3bafaf04a8a2a003e961c7200bb384f0636a7ad6..0e06db22ca0cab9e291e14a85eb3a005b863ad5e 100644 (file)
--- a/lib/asyn-thrdd.c
+++ b/lib/asyn-thrdd.c
@@ -604,10 +604,9 @@ CURLcode Curl_async_is_resolved(struct Curl_easy *data,
 
     if(thrdd->addr->res) {
       data->state.async.dns =
-        Curl_dnscache_mk_entry(data, thrdd->addr->res,
+        Curl_dnscache_mk_entry(data, &thrdd->addr->res,
                                data->state.async.hostname, 0,
                                data->state.async.port, FALSE);
-      thrdd->addr->res = NULL;
       if(!data->state.async.dns)
         result = CURLE_OUT_OF_MEMORY;
 

diff --git a/lib/doh.c b/lib/doh.c
index 88c09889bcf6011e062bebc3b54df1538e22f4ad..fd22eeefbcce013417c967bf944728fa4225b7ae 100644 (file)
--- a/lib/doh.c
+++ b/lib/doh.c
@@ -1253,7 +1253,8 @@ CURLcode Curl_doh_is_resolved(struct Curl_easy *data,
         goto error;
 
       /* we got a response, create a dns entry. */
-      dns = Curl_dnscache_mk_entry(data, ai, dohp->host, 0, dohp->port, FALSE);
+      dns = Curl_dnscache_mk_entry(data, &ai, dohp->host, 0,
+                                   dohp->port, FALSE);
       if(dns) {
         /* Now add and HTTPSRR information if we have */
 #ifdef USE_HTTPSRR
@@ -1278,8 +1279,6 @@ CURLcode Curl_doh_is_resolved(struct Curl_easy *data,
         result = Curl_dnscache_add(data, dns);
         *dnsp = data->state.async.dns;
       }
-      else
-        Curl_freeaddrinfo(ai);
     } /* address processing done */
 
     /* All done */

diff --git a/lib/hostip.c b/lib/hostip.c
index f82b1ee950148df8e96861979ece916803c4582f..cfa7995dbac607c486a1a678abeb0dafd7858bfc 100644 (file)
--- a/lib/hostip.c
+++ b/lib/hostip.c
@@ -559,20 +559,20 @@ UNITTEST CURLcode Curl_shuffle_addr(struct Curl_easy *data,
 
 struct Curl_dns_entry *
 Curl_dnscache_mk_entry(struct Curl_easy *data,
-                       struct Curl_addrinfo *addr,
+                       struct Curl_addrinfo **paddr,
                        const char *hostname,
                        size_t hostlen, /* length or zero */
                        int port,
                        bool permanent)
 {
-  struct Curl_dns_entry *dns;
+  struct Curl_dns_entry *dns = NULL;
 
 #ifndef CURL_DISABLE_SHUFFLE_DNS
   /* shuffle addresses if requested */
-  if(data->set.dns_shuffle_addresses) {
-    CURLcode result = Curl_shuffle_addr(data, &addr);
+  if(data->set.dns_shuffle_addresses && paddr) {
+    CURLcode result = Curl_shuffle_addr(data, paddr);
     if(result)
-      return NULL;
+      goto out;
   }
 #else
   (void)data;
@@ -583,10 +583,10 @@ Curl_dnscache_mk_entry(struct Curl_easy *data,
   /* Create a new cache entry */
   dns = curlx_calloc(1, sizeof(struct Curl_dns_entry) + hostlen);
   if(!dns)
-    return NULL;
+    goto out;
 
   dns->refcount = 1; /* the cache has the first reference */
-  dns->addr = addr; /* this is the address(es) */
+  dns->addr = paddr ? *paddr : NULL; /* this is the address(es) */
   if(permanent) {
     dns->timestamp.tv_sec = 0; /* an entry that never goes stale */
     dns->timestamp.tv_usec = 0; /* an entry that never goes stale */
@@ -598,13 +598,19 @@ Curl_dnscache_mk_entry(struct Curl_easy *data,
   if(hostlen)
     memcpy(dns->hostname, hostname, hostlen);
 
+out:
+  if(paddr) {
+    if(!dns)
+      Curl_freeaddrinfo(*paddr);
+    *paddr = NULL;
+  }
   return dns;
 }
 
 static struct Curl_dns_entry *
 dnscache_add_addr(struct Curl_easy *data,
                   struct Curl_dnscache *dnscache,
-                  struct Curl_addrinfo *addr,
+                  struct Curl_addrinfo **paddr,
                   const char *hostname,
                   size_t hlen, /* length or zero */
                   int port,
@@ -615,7 +621,7 @@ dnscache_add_addr(struct Curl_easy *data,
   struct Curl_dns_entry *dns;
   struct Curl_dns_entry *dns2;
 
-  dns = Curl_dnscache_mk_entry(data, addr, hostname, hlen, port, permanent);
+  dns = Curl_dnscache_mk_entry(data, paddr, hostname, hlen, port, permanent);
   if(!dns)
     return NULL;
 
@@ -627,7 +633,6 @@ dnscache_add_addr(struct Curl_easy *data,
   dns2 = Curl_hash_add(&dnscache->entries, entry_id, entry_len + 1,
                        (void *)dns);
   if(!dns2) {
-    dns->addr = NULL;
     dnscache_entry_free(dns);
     return NULL;
   }
@@ -977,13 +982,9 @@ out:
   }
   else if(addr) {
     /* we got a response, create a dns entry, add to cache, return */
-    dns = Curl_dnscache_mk_entry(data, addr, hostname, 0, port, FALSE);
+    dns = Curl_dnscache_mk_entry(data, &addr, hostname, 0, port, FALSE);
     if(!dns || Curl_dnscache_add(data, dns)) {
       /* this is OOM or similar, do not store such negative resolves */
-      Curl_freeaddrinfo(addr);
-      if(dns)
-        /* avoid a dangling pointer to addr in the dying dns entry */
-        dns->addr = NULL;
       result = CURLE_OUT_OF_MEMORY;
       goto error;
     }
@@ -1442,14 +1443,12 @@ err:
       }
 
       /* put this new host in the cache */
-      dns = dnscache_add_addr(data, dnscache, head, curlx_str(&source),
+      dns = dnscache_add_addr(data, dnscache, &head, curlx_str(&source),
                               curlx_strlen(&source), (int)port, permanent);
       if(dns)
         /* release the returned reference; the cache itself will keep the
          * entry alive: */
         dns->refcount--;
-      else
-        Curl_freeaddrinfo(head);
 
       dnscache_unlock(data, dnscache);
 

diff --git a/lib/hostip.h b/lib/hostip.h
index 645868eb4319ec41b94979942aca8ea78602563f..e70eac841bf2f1181598e9e3fbeaf6602338a086 100644 (file)
--- a/lib/hostip.h
+++ b/lib/hostip.h
@@ -151,13 +151,14 @@ void Curl_printable_address(const struct Curl_addrinfo *ip,
  * The entry is created with a reference count of 1.
  * Use `Curl_resolv_unlink()` to release your hold on it.
  *
- * The call takes ownership of `addr`and makes a copy of `hostname`.
+ * The call takes ownership of `addr`, even in case of failure, and always
+ * clears `*paddr`. It makes a copy of `hostname`.
  *
  * Returns entry or NULL on OOM.
  */
 struct Curl_dns_entry *
 Curl_dnscache_mk_entry(struct Curl_easy *data,
-                       struct Curl_addrinfo *addr,
+                       struct Curl_addrinfo **paddr,
                        const char *hostname,
                        size_t hostlen, /* length or zero */
                        int port,