From 474ebb524708ecddf4fe0fdb5493e8d638bdc04d Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Wed, 22 Jul 2026 11:06:39 +0200 Subject: [PATCH] api-guard: check lock on session cache Add a property to easy/multi API calls that prohibit calling the function when the involved SSL session cache is under lock by the current thread. Checks are only in effect when pthreads/Windows threads are available. Closes #22367 --- lib/api.c | 106 +++++++++++++++++++++++++++-------------- lib/curl_threads.c | 21 ++++++++ lib/curl_threads.h | 6 +++ lib/vtls/vtls_scache.c | 50 ++++++++++++++++--- lib/vtls/vtls_scache.h | 4 +- 5 files changed, 141 insertions(+), 46 deletions(-) diff --git a/lib/api.c b/lib/api.c index adcc51b3f8..9303502978 100644 --- a/lib/api.c +++ b/lib/api.c @@ -25,33 +25,37 @@ #include "urldata.h" #include "api.h" +#include "curl_threads.h" #include "multiif.h" +#include "vtls/vtls_scache.h" struct Curl_eapi_fn_props { Curl_eapi_fn fn; uint8_t data_is_killed; /* easy handle is killed in call */ uint8_t recurse; /* may be called when another call is in progress */ uint8_t no_event_cb; /* may not be called during a multi event callback */ + uint8_t no_scache_lock; /* may not be called with easy's vtls_scache + locked by current thread */ }; static const struct Curl_eapi_fn_props eapi_fn_props[CURL_EAPI_FN_LAST] = { - /* function kill rec !ev */ - { CURL_EAPI_FN_easy_cleanup, 1, 0, 0 }, - { CURL_EAPI_FN_easy_duphandle, 0, 1, 0 }, - { CURL_EAPI_FN_easy_getinfo, 0, 1, 0 }, - { CURL_EAPI_FN_easy_pause, 0, 1, 1 }, - { CURL_EAPI_FN_easy_perform_ev, 0, 0, 0 }, - { CURL_EAPI_FN_easy_perform, 0, 0, 0 }, - { CURL_EAPI_FN_easy_recv, 0, 0, 0 }, - { CURL_EAPI_FN_easy_reset, 0, 0, 0 }, - { CURL_EAPI_FN_easy_send, 0, 0, 0 }, - { CURL_EAPI_FN_easy_setopt, 0, 1, 0 }, - { CURL_EAPI_FN_easy_ssls_export, 0, 0, 0 }, - { CURL_EAPI_FN_easy_ssls_import, 0, 0, 0 }, - { CURL_EAPI_FN_easy_upkeep, 0, 0, 0 }, - { CURL_EAPI_FN_ws_recv, 0, 1, 0 }, - { CURL_EAPI_FN_ws_send, 0, 1, 0 }, - { CURL_EAPI_FN_ws_start_frame, 0, 1, 0 }, + /* function kill rec !ev !scach */ + { CURL_EAPI_FN_easy_cleanup, 1, 0, 0, 0 }, + { CURL_EAPI_FN_easy_duphandle, 0, 1, 0, 0 }, + { CURL_EAPI_FN_easy_getinfo, 0, 1, 0, 0 }, + { CURL_EAPI_FN_easy_pause, 0, 1, 1, 0 }, + { CURL_EAPI_FN_easy_perform_ev, 0, 0, 0, 1 }, + { CURL_EAPI_FN_easy_perform, 0, 0, 0, 1 }, + { CURL_EAPI_FN_easy_recv, 0, 0, 0, 1 }, + { CURL_EAPI_FN_easy_reset, 0, 0, 0, 0 }, + { CURL_EAPI_FN_easy_send, 0, 0, 0, 1 }, + { CURL_EAPI_FN_easy_setopt, 0, 1, 0, 0 }, + { CURL_EAPI_FN_easy_ssls_export, 0, 0, 0, 1 }, + { CURL_EAPI_FN_easy_ssls_import, 0, 0, 0, 1 }, + { CURL_EAPI_FN_easy_upkeep, 0, 0, 0, 1 }, + { CURL_EAPI_FN_ws_recv, 0, 1, 0, 1 }, + { CURL_EAPI_FN_ws_send, 0, 1, 0, 1 }, + { CURL_EAPI_FN_ws_start_frame, 0, 1, 0, 0 }, }; struct Curl_mapi_fn_props { @@ -59,29 +63,31 @@ struct Curl_mapi_fn_props { uint8_t multi_is_killed; /* multi handle is killed during call */ uint8_t recurse; /* may be called when another call is in progress */ uint8_t allow_ntfy_cb; /* may be called during a notify callback */ + uint8_t no_scache_lock; /* may not be called with multi's vtls_scache + locked by current thread */ }; static const struct Curl_mapi_fn_props mapi_fn_props[CURL_MAPI_FN_LAST] = { - /* function kill rec ntfy */ - { CURL_MAPI_FN_multi_add_handle, 0, 0, 1 }, - { CURL_MAPI_FN_multi_assign, 0, 1, 1 }, - { CURL_MAPI_FN_multi_cleanup, 1, 0, 0 }, - { CURL_MAPI_FN_multi_fdset, 0, 0, 1 }, - { CURL_MAPI_FN_multi_get_handles, 0, 1, 1 }, - { CURL_MAPI_FN_multi_get_offt, 0, 1, 1 }, - { CURL_MAPI_FN_multi_info_read, 0, 1, 1 }, - { CURL_MAPI_FN_multi_notify_disable, 0, 1, 1 }, - { CURL_MAPI_FN_multi_notify_enable, 0, 1, 1 }, - { CURL_MAPI_FN_multi_perform, 0, 0, 0 }, - { CURL_MAPI_FN_multi_poll, 0, 0, 1 }, - { CURL_MAPI_FN_multi_remove_handle, 0, 0, 1 }, - { CURL_MAPI_FN_multi_setopt, 0, 0, 1 }, - { CURL_MAPI_FN_multi_socket_action, 0, 0, 0 }, - { CURL_MAPI_FN_multi_socket_all, 0, 0, 0 }, - { CURL_MAPI_FN_multi_socket, 0, 0, 0 }, - { CURL_MAPI_FN_multi_timeout, 0, 0, 1 }, - { CURL_MAPI_FN_multi_wait, 0, 0, 1 }, - { CURL_MAPI_FN_multi_waitfds, 0, 0, 1 }, + /* function kill rec ntfy !scach */ + { CURL_MAPI_FN_multi_add_handle, 0, 0, 1, 0 }, + { CURL_MAPI_FN_multi_assign, 0, 1, 1, 0 }, + { CURL_MAPI_FN_multi_cleanup, 1, 0, 0, 1 }, + { CURL_MAPI_FN_multi_fdset, 0, 0, 1, 0 }, + { CURL_MAPI_FN_multi_get_handles, 0, 1, 1, 0 }, + { CURL_MAPI_FN_multi_get_offt, 0, 1, 1, 0 }, + { CURL_MAPI_FN_multi_info_read, 0, 1, 1, 0 }, + { CURL_MAPI_FN_multi_notify_disable, 0, 1, 1, 0 }, + { CURL_MAPI_FN_multi_notify_enable, 0, 1, 1, 0 }, + { CURL_MAPI_FN_multi_perform, 0, 0, 0, 1 }, + { CURL_MAPI_FN_multi_poll, 0, 0, 1, 0 }, + { CURL_MAPI_FN_multi_remove_handle, 0, 0, 1, 0 }, + { CURL_MAPI_FN_multi_setopt, 0, 0, 1, 0 }, + { CURL_MAPI_FN_multi_socket_action, 0, 0, 0, 1 }, + { CURL_MAPI_FN_multi_socket_all, 0, 0, 0, 1 }, + { CURL_MAPI_FN_multi_socket, 0, 0, 0, 1 }, + { CURL_MAPI_FN_multi_timeout, 0, 0, 1, 1 }, + { CURL_MAPI_FN_multi_wait, 0, 0, 1, 0 }, + { CURL_MAPI_FN_multi_waitfds, 0, 0, 1, 0 }, }; struct Curl_cbapi_fn_props { @@ -248,6 +254,19 @@ bool Curl_eapi_enter(struct Curl_eapi_guard *guard, goto out; } +#if defined(USE_SSL) && defined(USE_MUTEX) + if(fn_props->no_scache_lock && + Curl_ssl_scache_is_locked_by_current_thread(data)) { +#ifdef CURLVERBOSE + DEBUGF(curl_mfprintf(stderr, + "EAPI guard: calling %u while vtls_scache is locked by " + "current thread\n", (uint16_t)fn)); +#endif + result = CURLE_RECURSIVE_API_CALL; + goto out; + } +#endif + /* all fine, add to data's callstack */ if(data->callstack.count >= CURL_EAPI_MAX_RECURSION) { result = CURLE_RECURSIVE_API_CALL; @@ -318,6 +337,19 @@ bool Curl_mapi_enter(struct Curl_mapi_guard *guard, goto out; } +#if defined(USE_SSL) && defined(USE_MUTEX) + if(fn_props->no_scache_lock && multi->ssl_scache && + Curl_ssl_scache_is_locked_by_current_thread(multi->admin)) { +#ifdef CURLVERBOSE + DEBUGF(curl_mfprintf(stderr, + "MAPI guard: calling %u while its vtls_scache is locked by " + "current thread\n", (uint16_t)fn)); +#endif + mresult = CURLM_RECURSIVE_API_CALL; + goto out; + } +#endif + /* all fine, add to data's callstack */ if(multi->callstack.count >= CURL_MAPI_MAX_RECURSION) { mresult = CURLM_RECURSIVE_API_CALL; diff --git a/lib/curl_threads.c b/lib/curl_threads.c index 01041f63c3..16f14a364c 100644 --- a/lib/curl_threads.c +++ b/lib/curl_threads.c @@ -186,6 +186,16 @@ CURLcode Curl_cond_timedwait(pthread_cond_t *c, pthread_mutex_t *m, return rc ? CURLE_UNRECOVERABLE_POLL : CURLE_OK; } +curl_thread_id_t Curl_thread_get_current_id(void) +{ + return pthread_self(); +} + +bool Curl_thread_is_current(curl_thread_id_t tid) +{ + return !!pthread_equal(tid, pthread_self()); +} + #elif defined(_WIN32) void Curl_cond_signal(CONDITION_VARIABLE *c) @@ -208,6 +218,17 @@ CURLcode Curl_cond_timedwait(CONDITION_VARIABLE *c, CRITICAL_SECTION *m, } return CURLE_OK; } + +curl_thread_id_t Curl_thread_get_current_id(void) +{ + return GetCurrentThreadId(); +} + +bool Curl_thread_is_current(curl_thread_id_t tid) +{ + return tid == GetCurrentThreadId(); +} + #else #error neither HAVE_THREADS_POSIX nor _WIN32 defined #endif diff --git a/lib/curl_threads.h b/lib/curl_threads.h index 081d155695..452d5eb44f 100644 --- a/lib/curl_threads.h +++ b/lib/curl_threads.h @@ -31,6 +31,7 @@ # define CURL_STDCALL # define curl_mutex_t pthread_mutex_t # define curl_thread_t pthread_t * +# define curl_thread_id_t pthread_t # define curl_thread_t_null (pthread_t *)0 # define Curl_mutex_init(m) pthread_mutex_init(m, NULL) # define Curl_mutex_acquire(m) pthread_mutex_lock(m) @@ -44,6 +45,7 @@ # define CURL_STDCALL WINAPI # define curl_mutex_t CRITICAL_SECTION # define curl_thread_t HANDLE +# define curl_thread_id_t DWORD # define curl_thread_t_null (HANDLE)0 # define Curl_mutex_init(m) InitializeCriticalSectionEx(m, 0, 1) # define Curl_mutex_acquire(m) EnterCriticalSection(m) @@ -61,6 +63,10 @@ void Curl_cond_wait(curl_cond_t *c, curl_mutex_t *m); /* Returns CURLE_OPERATION_TIMEDOUT on timeout */ CURLcode Curl_cond_timedwait(curl_cond_t *c, curl_mutex_t *m, uint32_t timeout_ms); + +curl_thread_id_t Curl_thread_get_current_id(void); +bool Curl_thread_is_current(curl_thread_id_t tid); + #endif /* USE_MUTEX */ #ifdef USE_THREADS diff --git a/lib/vtls/vtls_scache.c b/lib/vtls/vtls_scache.c index 7148bf3297..3c0881262f 100644 --- a/lib/vtls/vtls_scache.c +++ b/lib/vtls/vtls_scache.c @@ -41,6 +41,7 @@ #include "url.h" #include "llist.h" #include "curl_share.h" +#include "curl_threads.h" #include "curl_trc.h" #include "curl_sha256.h" #include "rand.h" @@ -360,6 +361,10 @@ struct Curl_ssl_scache { size_t peer_count; int default_lifetime_secs; long age; +#ifdef USE_MUTEX + curl_mutex_t mutex; + curl_thread_id_t locking_thread; +#endif BIT(is_locked); }; @@ -649,7 +654,9 @@ CURLcode Curl_ssl_scache_create(size_t max_peers, Curl_llist_init(&scache->peers[i].sessions, cf_ssl_scache_session_ldestroy); } - +#ifdef USE_MUTEX + Curl_mutex_init(&scache->mutex); +#endif *pscache = scache; return CURLE_OK; } @@ -663,6 +670,9 @@ void Curl_ssl_scache_destroy(struct Curl_ssl_scache *scache) cf_ssl_scache_clear_peer(&scache->peers[i]); } curlx_free(scache->peers); +#ifdef USE_MUTEX + Curl_mutex_destroy(&scache->mutex); +#endif curlx_free(scache); } } @@ -684,8 +694,16 @@ void Curl_ssl_scache_lock(struct Curl_easy *data) if(CURL_SHARE_ssl_scache(data)) Curl_share_lock(data, CURL_LOCK_DATA_SSL_SESSION, CURL_LOCK_ACCESS_SINGLE); +#ifdef USE_MUTEX + Curl_mutex_acquire(&scache->mutex); + scache->locking_thread = Curl_thread_get_current_id(); DEBUGASSERT(!scache->is_locked); scache->is_locked = TRUE; + Curl_mutex_release(&scache->mutex); +#else + DEBUGASSERT(!scache->is_locked); + scache->is_locked = TRUE; +#endif } } @@ -694,13 +712,37 @@ void Curl_ssl_scache_unlock(struct Curl_easy *data) { struct Curl_ssl_scache *scache = cf_ssl_scache_get(data); if(scache) { +#ifdef USE_MUTEX + Curl_mutex_acquire(&scache->mutex); + scache->locking_thread = 0; DEBUGASSERT(scache->is_locked); scache->is_locked = FALSE; + Curl_mutex_release(&scache->mutex); +#else + DEBUGASSERT(scache->is_locked); + scache->is_locked = FALSE; +#endif if(CURL_SHARE_ssl_scache(data)) Curl_share_unlock(data, CURL_LOCK_DATA_SSL_SESSION); } } +bool Curl_ssl_scache_is_locked_by_current_thread(struct Curl_easy *data) +{ + struct Curl_ssl_scache *scache = cf_ssl_scache_get(data); + bool locked = FALSE; + if(!scache) + return FALSE; +#ifdef USE_MUTEX + Curl_mutex_acquire(&scache->mutex); + locked = scache->is_locked && Curl_thread_is_current(scache->locking_thread); + Curl_mutex_release(&scache->mutex); +#else + locked = (bool)scache->is_locked; +#endif + return locked; +} + static bool cf_ssl_scache_match_auth(struct Curl_ssl_scache_peer *peer, struct ssl_primary_config *conn_config) { @@ -1096,12 +1138,6 @@ void Curl_ssl_scache_remove_all(struct Curl_cfilter *cf, #define CURL_SSL_TICKET_MAX (16 * 1024) -bool Curl_ssl_scache_is_locked(struct Curl_easy *data) -{ - struct Curl_ssl_scache *scache = cf_ssl_scache_get(data); - return scache && scache->is_locked; -} - static CURLcode cf_ssl_scache_peer_set_hmac(struct Curl_ssl_scache_peer *peer) { CURLcode result; diff --git a/lib/vtls/vtls_scache.h b/lib/vtls/vtls_scache.h index e6f359395b..c9f35e800f 100644 --- a/lib/vtls/vtls_scache.h +++ b/lib/vtls/vtls_scache.h @@ -202,9 +202,9 @@ void Curl_ssl_scache_remove_all(struct Curl_cfilter *cf, struct Curl_easy *data, const char *ssl_peer_key); -#ifdef USE_SSLS_EXPORT +bool Curl_ssl_scache_is_locked_by_current_thread(struct Curl_easy *data); -bool Curl_ssl_scache_is_locked(struct Curl_easy *data); +#ifdef USE_SSLS_EXPORT CURLcode Curl_ssl_session_import(struct Curl_easy *data, const char *ssl_peer_key,