Fix premature reuse of qp's in rcu locks

An intermittent failure was noted on our new ppc64le CI runner, in which
what appeared to be a corrupted or invalid value getting returned from a
shared pointer under rcu protection

Investigation showed that the problem was with our small number of qp's
in a lock, and slightly incorrect accounting of the number of qp's
available we were prematurely recycling qp's, which led in turn to
premature completion of synchronization states, resulting in readers
reading memory that may have already been freed.

Fix it by:
a) Ensuring that we account for the fact that the first qp in an rcu
lock is allocated at the time the lock is created

and

b) Ensuring that we have a minimum number of 3 qp's:
1 that is free for write side allocation
1 that is in use by the write side currently
1 "next" qp that the read side can update while the prior qp is being
retired

With this change, the rcu threadstest runs indefinately in my testing

Fixes #26356

Reviewed-by: Paul Dale <ppzgs1@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26384)

(cherry picked from commit 25f8e2c15b701514b7b2fe652634289b6fb8581f)

diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index e33f21b9eaadae04bb6e140d1225d1531d786b92..d97b7f87da76b9cadd571afdd97b0d7fa17b623f 100644 (file)
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -666,8 +666,11 @@ CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx)
 {
     struct rcu_lock_st *new;
 
-    if (num_writers < 1)
-        num_writers = 1;
+    /*
+     * We need a minimum of 3 qp's
+     */
+    if (num_writers < 3)
+        num_writers = 3;
 
     ctx = ossl_lib_ctx_get_concrete(ctx);
     if (ctx == NULL)
@@ -683,11 +686,15 @@ CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx)
     pthread_mutex_init(&new->alloc_lock, NULL);
     pthread_cond_init(&new->prior_signal, NULL);
     pthread_cond_init(&new->alloc_signal, NULL);
-    new->qp_group = allocate_new_qp_group(new, num_writers + 1);
+    /* By default our first writer is already alloced */
+    new->writers_alloced = 1;
+
+    new->qp_group = allocate_new_qp_group(new, num_writers);
     if (new->qp_group == NULL) {
         OPENSSL_free(new);
         new = NULL;
     }
+
     return new;
 }
 

diff --git a/crypto/threads_win.c b/crypto/threads_win.c
index 83e2a7c7742fcdb5fbfa77f14aa74821f4a7b7bb..04a62d490a1f6490a80551e150d5ab8b83ab50f8 100644 (file)
--- a/crypto/threads_win.c
+++ b/crypto/threads_win.c
@@ -159,8 +159,11 @@ CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx)
 {
     struct rcu_lock_st *new;
 
-    if (num_writers < 1)
-        num_writers = 1;
+    /*
+     * We need a minimum of 3 qps
+     */
+    if (num_writers < 3)
+        num_writers = 3;
 
     ctx = ossl_lib_ctx_get_concrete(ctx);
     if (ctx == NULL)
@@ -178,7 +181,9 @@ CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx)
     new->prior_signal = ossl_crypto_condvar_new();
     new->alloc_lock = ossl_crypto_mutex_new();
     new->prior_lock = ossl_crypto_mutex_new();
-    new->qp_group = allocate_new_qp_group(new, num_writers + 1);
+    new->qp_group = allocate_new_qp_group(new, num_writers);
+    /* By default the first qp is already alloced */
+    new->writers_alloced = 1;
     if (new->qp_group == NULL
         || new->alloc_signal == NULL
         || new->prior_signal == NULL
@@ -196,6 +201,7 @@ CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx)
         OPENSSL_free(new);
         new = NULL;
     }
+
     return new;
 
 }