From e4557573464ce7fd66ae8fe9723dc7c3dce1f349 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 4 Feb 2025 17:51:57 +0100 Subject: [PATCH] asyn-thread: fix HTTPS RR crash By removing 'data' from the thread struct and passing it in as an argument we avoid the case it could be dereferenced before stored when shutting down HTTPS RR. Also reordered the struct fields a little to remove holes. Closes #16169 --- lib/asyn-thread.c | 22 ++++++++++++---------- lib/asyn.h | 7 +++---- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/asyn-thread.c b/lib/asyn-thread.c index b8c304943d..8d6a9deb4a 100644 --- a/lib/asyn-thread.c +++ b/lib/asyn-thread.c @@ -138,14 +138,14 @@ CURLcode Curl_resolver_duphandle(struct Curl_easy *easy, void **to, void *from) return Curl_resolver_init(easy, to); } -static void destroy_async_data(struct Curl_async *); +static void destroy_async_data(struct Curl_easy *); /* * Cancel all possibly still on-going resolves for this connection. */ void Curl_resolver_cancel(struct Curl_easy *data) { - destroy_async_data(&data->state.async); + destroy_async_data(data); } /* This function is used to init a threaded resolve */ @@ -377,14 +377,17 @@ CURL_STDCALL gethostbyname_thread(void *arg) /* * destroy_async_data() cleans up async resolver data and thread handle. */ -static void destroy_async_data(struct Curl_async *async) +static void destroy_async_data(struct Curl_easy *data) { + struct Curl_async *async; + DEBUGASSERT(data); + async = &data->state.async; + DEBUGASSERT(async); if(async->tdata) { struct thread_data *td = async->tdata; bool done; #ifndef CURL_DISABLE_SOCKETPAIR curl_socket_t sock_rd = td->tsd.sock_pair[0]; - struct Curl_easy *data = td->tsd.data; #endif #ifdef USE_HTTPSRR_ARES @@ -500,7 +503,7 @@ static bool init_resolve_thread(struct Curl_easy *data, return TRUE; err_exit: - destroy_async_data(asp); + destroy_async_data(data); errno_exit: errno = err; @@ -539,7 +542,7 @@ static CURLcode thread_wait_resolv(struct Curl_easy *data, /* a name was not resolved, report error */ result = Curl_resolver_error(data); - destroy_async_data(&data->state.async); + destroy_async_data(data); if(!data->state.async.dns && report) connclose(data->conn, "asynch resolve failed"); @@ -617,7 +620,7 @@ CURLcode Curl_resolver_is_resolved(struct Curl_easy *data, if(!data->state.async.dns) { CURLcode result = Curl_resolver_error(data); - destroy_async_data(&data->state.async); + destroy_async_data(data); return result; } #ifdef USE_HTTPSRR_ARES @@ -625,13 +628,13 @@ CURLcode Curl_resolver_is_resolved(struct Curl_easy *data, struct Curl_https_rrinfo *lhrr = Curl_memdup(&td->hinfo, sizeof(struct Curl_https_rrinfo)); if(!lhrr) { - destroy_async_data(&data->state.async); + destroy_async_data(data); return CURLE_OUT_OF_MEMORY; } data->state.async.dns->hinfo = lhrr; } #endif - destroy_async_data(&data->state.async); + destroy_async_data(data); *entry = data->state.async.dns; } else { @@ -685,7 +688,6 @@ int Curl_resolver_getsock(struct Curl_easy *data, curl_socket_t *socks) if(td) { /* return read fd to client for polling the DNS resolution status */ socks[socketi] = td->tsd.sock_pair[0]; - td->tsd.data = data; ret_val = GETSOCK_READSOCK(socketi); } else { diff --git a/lib/asyn.h b/lib/asyn.h index c674541256..5a21329cf3 100644 --- a/lib/asyn.h +++ b/lib/asyn.h @@ -40,20 +40,19 @@ struct Curl_dns_entry; /* Data for synchronization between resolver thread and its parent */ struct thread_sync_data { curl_mutex_t *mtx; - bool done; - int port; char *hostname; /* hostname to resolve, Curl_async.hostname duplicate */ #ifndef CURL_DISABLE_SOCKETPAIR - struct Curl_easy *data; curl_socket_t sock_pair[2]; /* eventfd/pipes/socket pair */ #endif - int sock_error; struct Curl_addrinfo *res; #ifdef HAVE_GETADDRINFO struct addrinfo hints; #endif struct thread_data *td; /* for thread-self cleanup */ + int port; + int sock_error; + bool done; }; struct thread_data { -- 2.47.3