From: Matt Caswell <matt@openssl.org>
Date: Fri, 12 May 2023 15:15:21 +0000 (+0100)
Subject: Don't take a write lock to retrieve a value from a stack
X-Git-Tag: openssl-3.2.0-alpha1~698
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=80935bf5ad309bf6c03591acf1d48fe1db57b78f;p=thirdparty%2Fopenssl.git

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 <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20952)
---

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 21a8260a44b..247698583cc 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)