commit bca5875eb3c25348ec07758cde66ebec27031ce4
author David Benjamin <davidben@google.com> Sun Feb 04 10:24:18 2024 -0500
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Feb 23 16:20:22 2024 +0000
tree b89251a411a364a407905fd148979e3b35f816b7
parent d4faa8d63a06a5feff2e5b68695adf9bf8fd1f81

Use a more fine-grained lock in by_dir.c

Different BY_DIR_ENTRYs don't need to share a lock. Also switch some
code to use OPENSSL_strndup.

Change-Id: I3809e001afb9577bb96aab214e80e173900356fe
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66012
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/x509/by_dir.c

diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c
index 5eca97b..d49220d 100644
--- a/crypto/x509/by_dir.c
+++ b/crypto/x509/by_dir.c
@@ -72,6 +72,7 @@
 } BY_DIR_HASH;
 
 typedef struct lookup_dir_entry_st {
+  CRYPTO_MUTEX lock;
   char *dir;
   int dir_type;
   STACK_OF(BY_DIR_HASH) *hashes;
@@ -156,6 +157,7 @@
 
 static void by_dir_entry_free(BY_DIR_ENTRY *ent) {
   if (ent != NULL) {
+    CRYPTO_MUTEX_cleanup(&ent->lock);
     OPENSSL_free(ent->dir);
     sk_BY_DIR_HASH_pop_free(ent->hashes, by_dir_hash_free);
     OPENSSL_free(ent);
@@ -215,15 +217,12 @@
       if (!ent) {
         return 0;
       }
+      CRYPTO_MUTEX_init(&ent->lock);
       ent->dir_type = type;
       ent->hashes = sk_BY_DIR_HASH_new(by_dir_hash_cmp);
-      ent->dir = OPENSSL_malloc(len + 1);
-      if (!ent->dir || !ent->hashes) {
-        by_dir_entry_free(ent);
-        return 0;
-      }
-      OPENSSL_strlcpy(ent->dir, ss, len + 1);
-      if (!sk_BY_DIR_ENTRY_push(ctx->dirs, ent)) {
+      ent->dir = OPENSSL_strndup(ss, len);
+      if (ent->dir == NULL || ent->hashes == NULL ||
+          !sk_BY_DIR_ENTRY_push(ctx->dirs, ent)) {
         by_dir_entry_free(ent);
         return 0;
       }
@@ -232,10 +231,6 @@
   return 1;
 }
 
-// g_ent_hashes_lock protects the |hashes| member of all |BY_DIR_ENTRY|
-// objects.
-static CRYPTO_MUTEX g_ent_hashes_lock = CRYPTO_MUTEX_INIT;
-
 static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name,
                                X509_OBJECT *ret) {
   union {
@@ -300,7 +295,7 @@
       }
       if (type == X509_LU_CRL && ent->hashes) {
         htmp.hash = h;
-        CRYPTO_MUTEX_lock_read(&g_ent_hashes_lock);
+        CRYPTO_MUTEX_lock_read(&ent->lock);
         if (sk_BY_DIR_HASH_find(ent->hashes, &idx, &htmp)) {
           hent = sk_BY_DIR_HASH_value(ent->hashes, idx);
           k = hent->suffix;
@@ -308,7 +303,7 @@
           hent = NULL;
           k = 0;
         }
-        CRYPTO_MUTEX_unlock_read(&g_ent_hashes_lock);
+        CRYPTO_MUTEX_unlock_read(&ent->lock);
       } else {
         k = 0;
         hent = NULL;
@@ -341,7 +336,7 @@
       // If a CRL, update the last file suffix added for this
 
       if (type == X509_LU_CRL) {
-        CRYPTO_MUTEX_lock_write(&g_ent_hashes_lock);
+        CRYPTO_MUTEX_lock_write(&ent->lock);
         // Look for entry again in case another thread added an entry
         // first.
         if (!hent) {
@@ -354,14 +349,14 @@
         if (!hent) {
           hent = OPENSSL_malloc(sizeof(BY_DIR_HASH));
           if (hent == NULL) {
-            CRYPTO_MUTEX_unlock_write(&g_ent_hashes_lock);
+            CRYPTO_MUTEX_unlock_write(&ent->lock);
             ok = 0;
             goto finish;
           }
           hent->hash = h;
           hent->suffix = k;
           if (!sk_BY_DIR_HASH_push(ent->hashes, hent)) {
-            CRYPTO_MUTEX_unlock_write(&g_ent_hashes_lock);
+            CRYPTO_MUTEX_unlock_write(&ent->lock);
             OPENSSL_free(hent);
             ok = 0;
             goto finish;
@@ -371,7 +366,7 @@
           hent->suffix = k;
         }
 
-        CRYPTO_MUTEX_unlock_write(&g_ent_hashes_lock);
+        CRYPTO_MUTEX_unlock_write(&ent->lock);
       }
 
       if (tmp != NULL) {