From: Stefan Eissing <stefan@eissing.org>
Date: Wed, 17 Jul 2024 12:46:47 +0000 (+0200)
Subject: doh: fix cleanup
X-Git-Tag: curl-8_9_0~26
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d8696dc8c0a8879e2673870d09adc5fa9ab1af22;p=thirdparty%2Fcurl.git

doh: fix cleanup

When removing an easy handle that had DoH sub-easy handles going, those
were not removed from the multi handle. Their memory was reclaimed on
curl_easy_cleanup() of the owning handle, but multi still had them in
their list.

Add `Curl_doh_close()` and `Curl_doh_cleanup()` as common point for
handling the DoH resource management. Use the `multi` present in the doh
handles (if so), for removal, as the `data->multi` might already have
been NULLed at this time.

Reported-by: 罗朝辉
Fixes #14207
Closes #14212
---

diff --git a/lib/doh.c b/lib/doh.c
index 72b0c969d9..0dcca17ccd 100644
--- a/lib/doh.c
+++ b/lib/doh.c
@@ -400,7 +400,6 @@ struct Curl_addrinfo *Curl_doh(struct Curl_easy *data,
                                int *waitp)
 {
   CURLcode result = CURLE_OK;
-  int slot;
   struct dohdata *dohp;
   struct connectdata *conn = data->conn;
 #ifdef USE_HTTPSRR
@@ -484,13 +483,7 @@ struct Curl_addrinfo *Curl_doh(struct Curl_easy *data,
   return NULL;
 
 error:
-  curl_slist_free_all(dohp->headers);
-  data->req.doh->headers = NULL;
-  for(slot = 0; slot < DOH_PROBE_SLOTS; slot++) {
-    (void)curl_multi_remove_handle(data->multi, dohp->probe[slot].easy);
-    Curl_close(&dohp->probe[slot].easy);
-  }
-  Curl_safefree(data->req.doh);
+  Curl_doh_cleanup(data);
   return NULL;
 }
 
@@ -1325,10 +1318,7 @@ CURLcode Curl_doh_is_resolved(struct Curl_easy *data,
     struct dohentry de;
     int slot;
     /* remove DoH handles from multi handle and close them */
-    for(slot = 0; slot < DOH_PROBE_SLOTS; slot++) {
-      curl_multi_remove_handle(data->multi, dohp->probe[slot].easy);
-      Curl_close(&dohp->probe[slot].easy);
-    }
+    Curl_doh_close(data);
     /* parse the responses, create the struct and return it! */
     de_init(&de);
     for(slot = 0; slot < DOH_PROBE_SLOTS; slot++) {
@@ -1415,4 +1405,32 @@ CURLcode Curl_doh_is_resolved(struct Curl_easy *data,
   return CURLE_OK;
 }
 
+void Curl_doh_close(struct Curl_easy *data)
+{
+  struct dohdata *doh = data->req.doh;
+  if(doh) {
+    size_t slot;
+    for(slot = 0; slot < DOH_PROBE_SLOTS; slot++) {
+      if(!doh->probe[slot].easy)
+        continue;
+      /* data->multi might already be reset at this time */
+      if(doh->probe[slot].easy->multi)
+        curl_multi_remove_handle(doh->probe[slot].easy->multi,
+                                 doh->probe[slot].easy);
+      Curl_close(&doh->probe[slot].easy);
+    }
+  }
+}
+
+void Curl_doh_cleanup(struct Curl_easy *data)
+{
+  struct dohdata *doh = data->req.doh;
+  if(doh) {
+    Curl_doh_close(data);
+    curl_slist_free_all(doh->headers);
+    data->req.doh->headers = NULL;
+    Curl_safefree(data->req.doh);
+  }
+}
+
 #endif /* CURL_DISABLE_DOH */
diff --git a/lib/doh.h b/lib/doh.h
index 68924a4172..5e86bf44a5 100644
--- a/lib/doh.h
+++ b/lib/doh.h
@@ -140,6 +140,8 @@ struct dohentry {
 #endif
 };
 
+void Curl_doh_close(struct Curl_easy *data);
+void Curl_doh_cleanup(struct Curl_easy *data);
 
 #ifdef UNITTESTS
 UNITTEST DOHcode doh_encode(const char *host,
diff --git a/lib/multi.c b/lib/multi.c
index c5330ebe10..521af63b90 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -2799,6 +2799,9 @@ CURLMcode curl_multi_cleanup(struct Curl_multi *multi)
     /* First remove all remaining easy handles */
     data = multi->easyp;
     while(data) {
+      if(!GOOD_EASY_HANDLE(data))
+        return CURLM_BAD_HANDLE;
+
       nextdata = data->next;
       if(!data->state.done && data->conn)
         /* if DONE was never called for this handle */
diff --git a/lib/request.c b/lib/request.c
index a4227de981..5c8d52f696 100644
--- a/lib/request.c
+++ b/lib/request.c
@@ -116,10 +116,7 @@ void Curl_req_hard_reset(struct SingleRequest *req, struct Curl_easy *data)
     Curl_bufq_reset(&req->sendbuf);
 
 #ifndef CURL_DISABLE_DOH
-  if(req->doh) {
-    Curl_close(&req->doh->probe[0].easy);
-    Curl_close(&req->doh->probe[1].easy);
-  }
+  Curl_doh_close(data);
 #endif
   /* Can no longer memset() this struct as we need to keep some state */
   req->size = -1;
@@ -173,14 +170,7 @@ void Curl_req_free(struct SingleRequest *req, struct Curl_easy *data)
   Curl_client_cleanup(data);
 
 #ifndef CURL_DISABLE_DOH
-  if(req->doh) {
-    Curl_close(&req->doh->probe[0].easy);
-    Curl_close(&req->doh->probe[1].easy);
-    Curl_dyn_free(&req->doh->probe[0].serverdoh);
-    Curl_dyn_free(&req->doh->probe[1].serverdoh);
-    curl_slist_free_all(req->doh->headers);
-    Curl_safefree(req->doh);
-  }
+  Curl_doh_cleanup(data);
 #endif
 }