From 80935bf5ad309bf6c03591acf1d48fe1db57b78f Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 12 May 2023 16:15:21 +0100 Subject: [PATCH] Don't take a write lock to retrieve a value from a stack ossl_x509_store_ctx_get_by_subject() was taking a write lock for the store, but was only (usually) retrieving a value from the stack of objects. We take a read lock instead. Partially fixes #20286 Reviewed-by: Tomas Mraz Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/20952) --- crypto/x509/x509_lu.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index 21a8260a44..247698583c 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -44,6 +44,11 @@ int X509_STORE_lock(X509_STORE *xs) return CRYPTO_THREAD_write_lock(xs->lock); } +static int x509_store_read_lock(X509_STORE *xs) +{ + return CRYPTO_THREAD_read_lock(xs->lock); +} + int X509_STORE_unlock(X509_STORE *xs) { return CRYPTO_THREAD_unlock(xs->lock); @@ -324,9 +329,19 @@ static int ossl_x509_store_ctx_get_by_subject(const X509_STORE_CTX *ctx, stmp.type = X509_LU_NONE; stmp.data.ptr = NULL; - if (!X509_STORE_lock(store)) + if (!x509_store_read_lock(store)) return 0; - sk_X509_OBJECT_sort(store->objs); + /* Should already be sorted...but just in case */ + if (!sk_X509_OBJECT_is_sorted(store->objs)) { + X509_STORE_unlock(store); + /* Take a write lock instead of a read lock */ + X509_STORE_lock(store); + /* + * Another thread might have sorted it in the meantime. But if so, + * sk_X509_OBJECT_sort() exits early. + */ + sk_X509_OBJECT_sort(store->objs); + } tmp = X509_OBJECT_retrieve_by_subject(store->objs, type, name); X509_STORE_unlock(store); @@ -535,15 +550,18 @@ static int x509_object_idx_cnt(STACK_OF(X509_OBJECT) *h, X509_LOOKUP_TYPE type, return -1; } + /* Assumes h is locked for read if applicable */ return sk_X509_OBJECT_find_all(h, &stmp, pnmatch); } +/* Assumes h is locked for read if applicable */ int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, X509_LOOKUP_TYPE type, const X509_NAME *name) { return x509_object_idx_cnt(h, type, name, NULL); } +/* Assumes h is locked for read if applicable */ X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h, X509_LOOKUP_TYPE type, const X509_NAME *name)