Fix potential use-after-free in REF_PRINT_COUNT

We use REF_PRINT_COUNT to dump out the value of various reference
counters in our code

However, we commonly use this macro after an increment or decrement.  On
increment its fine, but on decrement its not, because the macro
dereferences the object holding the counter value, which may be freed by
another thread, as we've given up our ref count to it prior to using the
macro.

The rule is that we can't reference memory for an object once we've
released our reference, so lets fix this by altering REF_PRINT_COUNT to
accept the value returned by CRYPTO_[UP|DOWN]_REF instead.  The
eliminates the need to dereference the memory the object points to an
allows us to use the call after we release our reference count

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25664)

(cherry picked from commit dc10ffc2834e0d2f5ebc1c3e29bd97f1f43a0ead)

diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c
index 272189a9a6472f219098f98829a6503be3d25c1a..85ab4afe182a27f2c19262f3d5c2f31f9a3717ae 100644 (file)
--- a/crypto/bio/bio_lib.c
+++ b/crypto/bio/bio_lib.c
@@ -126,7 +126,7 @@ int BIO_free(BIO *a)
     if (CRYPTO_DOWN_REF(&a->references, &ret) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("BIO", a);
+    REF_PRINT_COUNT("BIO", ret, a);
     if (ret > 0)
         return 1;
     REF_ASSERT_ISNT(ret < 0);
@@ -191,7 +191,7 @@ int BIO_up_ref(BIO *a)
     if (CRYPTO_UP_REF(&a->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("BIO", a);
+    REF_PRINT_COUNT("BIO", i, a);
     REF_ASSERT_ISNT(i < 2);
     return i > 1;
 }

diff --git a/crypto/dh/dh_lib.c b/crypto/dh/dh_lib.c
index 9d5a6b0b6c2baa7951fdf770ce70057cdfba023b..93e08b3f8c71bd0a573fefd08e4e15d857222a73 100644 (file)
--- a/crypto/dh/dh_lib.c
+++ b/crypto/dh/dh_lib.c
@@ -141,7 +141,7 @@ void DH_free(DH *r)
         return;
 
     CRYPTO_DOWN_REF(&r->references, &i);
-    REF_PRINT_COUNT("DH", r);
+    REF_PRINT_COUNT("DH", i, r);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -171,7 +171,7 @@ int DH_up_ref(DH *r)
     if (CRYPTO_UP_REF(&r->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("DH", r);
+    REF_PRINT_COUNT("DH", i, r);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/dsa/dsa_lib.c b/crypto/dsa/dsa_lib.c
index 7997c2ac25e459d04acf71828864ed3ac3a5f939..db6e3b059b4deebc68f9bff66c071d87ac04f865 100644 (file)
--- a/crypto/dsa/dsa_lib.c
+++ b/crypto/dsa/dsa_lib.c
@@ -218,7 +218,7 @@ void DSA_free(DSA *r)
         return;
 
     CRYPTO_DOWN_REF(&r->references, &i);
-    REF_PRINT_COUNT("DSA", r);
+    REF_PRINT_COUNT("DSA", i, r);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -249,7 +249,7 @@ int DSA_up_ref(DSA *r)
     if (CRYPTO_UP_REF(&r->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("DSA", r);
+    REF_PRINT_COUNT("DSA", i, r);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/dso/dso_lib.c b/crypto/dso/dso_lib.c
index 8f3387e9b802085420ac49c6c65615226215eda1..65579cb8b360780c8d6d532913cbe65d9ab85541 100644 (file)
--- a/crypto/dso/dso_lib.c
+++ b/crypto/dso/dso_lib.c
@@ -54,7 +54,7 @@ int DSO_free(DSO *dso)
     if (CRYPTO_DOWN_REF(&dso->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("DSO", dso);
+    REF_PRINT_COUNT("DSO", i, dso);
     if (i > 0)
         return 1;
     REF_ASSERT_ISNT(i < 0);
@@ -96,7 +96,7 @@ int DSO_up_ref(DSO *dso)
     if (CRYPTO_UP_REF(&dso->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("DSO", dso);
+    REF_PRINT_COUNT("DSO", i, dso);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/ec/ec_key.c b/crypto/ec/ec_key.c
index 9bc4e032c557171925e732e01217b59af060b31b..823b5c8c8e6c6c13d528b964403a89e36845ff28 100644 (file)
--- a/crypto/ec/ec_key.c
+++ b/crypto/ec/ec_key.c
@@ -76,7 +76,7 @@ void EC_KEY_free(EC_KEY *r)
         return;
 
     CRYPTO_DOWN_REF(&r->references, &i);
-    REF_PRINT_COUNT("EC_KEY", r);
+    REF_PRINT_COUNT("EC_KEY", i, r);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -197,7 +197,7 @@ int EC_KEY_up_ref(EC_KEY *r)
     if (CRYPTO_UP_REF(&r->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("EC_KEY", r);
+    REF_PRINT_COUNT("EC_KEY", i, r);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c
index 9eb007cdf908285b4a26ae0eee1f8a2137a8c394..e9092a6c9db433dd81bf0880fbf935a8bb52efff 100644 (file)
--- a/crypto/ec/ec_mult.c
+++ b/crypto/ec/ec_mult.c
@@ -85,7 +85,7 @@ void EC_ec_pre_comp_free(EC_PRE_COMP *pre)
         return;
 
     CRYPTO_DOWN_REF(&pre->references, &i);
-    REF_PRINT_COUNT("EC_ec", pre);
+    REF_PRINT_COUNT("EC_ec", i, pre);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistp224.c b/crypto/ec/ecp_nistp224.c
index debfdb3dc942e394d933b0b9f48817963f75fcb3..6485f46f717c4d566f4b5a8ce98b494aa7116179 100644 (file)
--- a/crypto/ec/ecp_nistp224.c
+++ b/crypto/ec/ecp_nistp224.c
@@ -1264,7 +1264,7 @@ void EC_nistp224_pre_comp_free(NISTP224_PRE_COMP *p)
         return;
 
     CRYPTO_DOWN_REF(&p->references, &i);
-    REF_PRINT_COUNT("EC_nistp224", p);
+    REF_PRINT_COUNT("EC_nistp224", i, p);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistp256.c b/crypto/ec/ecp_nistp256.c
index d28306a6bd15be35d3950811d0bfb74512141b74..2cf4212685ded2a6d11b4e8947269162374f4370 100644 (file)
--- a/crypto/ec/ecp_nistp256.c
+++ b/crypto/ec/ecp_nistp256.c
@@ -1874,7 +1874,7 @@ void EC_nistp256_pre_comp_free(NISTP256_PRE_COMP *pre)
         return;
 
     CRYPTO_DOWN_REF(&pre->references, &i);
-    REF_PRINT_COUNT("EC_nistp256", pre);
+    REF_PRINT_COUNT("EC_nistp256", i, pre);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistp384.c b/crypto/ec/ecp_nistp384.c
index ff68f9cc7ad023ec33a2ad7be02099a7ae748d06..3fd7a40020d64e3fe0716dd94fdf758b494e48e0 100644 (file)
--- a/crypto/ec/ecp_nistp384.c
+++ b/crypto/ec/ecp_nistp384.c
@@ -1560,7 +1560,7 @@ void ossl_ec_nistp384_pre_comp_free(NISTP384_PRE_COMP *p)
         return;
 
     CRYPTO_DOWN_REF(&p->references, &i);
-    REF_PRINT_COUNT("ossl_ec_nistp384", p);
+    REF_PRINT_COUNT("ossl_ec_nistp384", i, p);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistp521.c b/crypto/ec/ecp_nistp521.c
index db5a9dd5def3691269d9fe769f0755fd1e4f6044..fe6836a147140c5dc00faa6ebf1817b7635c8be1 100644 (file)
--- a/crypto/ec/ecp_nistp521.c
+++ b/crypto/ec/ecp_nistp521.c
@@ -1766,7 +1766,7 @@ void EC_nistp521_pre_comp_free(NISTP521_PRE_COMP *p)
         return;
 
     CRYPTO_DOWN_REF(&p->references, &i);
-    REF_PRINT_COUNT("EC_nistp521", p);
+    REF_PRINT_COUNT("EC_nistp521", i, p);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistz256.c b/crypto/ec/ecp_nistz256.c
index 5760639a2ee2405b2413b672db347e898a426d4d..827c5c43584aa485a4298de65e1b6ded4cf05d7f 100644 (file)
--- a/crypto/ec/ecp_nistz256.c
+++ b/crypto/ec/ecp_nistz256.c
@@ -1238,7 +1238,7 @@ void EC_nistz256_pre_comp_free(NISTZ256_PRE_COMP *pre)
         return;
 
     CRYPTO_DOWN_REF(&pre->references, &i);
-    REF_PRINT_COUNT("EC_nistz256", pre);
+    REF_PRINT_COUNT("EC_nistz256", i, pre);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecx_key.c b/crypto/ec/ecx_key.c
index ba725eb573c28cabf0199f889586e419000087c1..aeaf5783d10c77907704b6c94a646e964a693857 100644 (file)
--- a/crypto/ec/ecx_key.c
+++ b/crypto/ec/ecx_key.c
@@ -69,7 +69,7 @@ void ossl_ecx_key_free(ECX_KEY *key)
         return;
 
     CRYPTO_DOWN_REF(&key->references, &i);
-    REF_PRINT_COUNT("ECX_KEY", key);
+    REF_PRINT_COUNT("ECX_KEY", i, key);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -92,7 +92,7 @@ int ossl_ecx_key_up_ref(ECX_KEY *key)
     if (CRYPTO_UP_REF(&key->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("ECX_KEY", key);
+    REF_PRINT_COUNT("ECX_KEY", i, key);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index b7377751bd483518d83c3580004396ce2e6a4c74..aaba87170586e317447d52c5fa183ccc647a9fce 100644 (file)
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1672,7 +1672,7 @@ int EVP_PKEY_up_ref(EVP_PKEY *pkey)
     if (CRYPTO_UP_REF(&pkey->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("EVP_PKEY", pkey);
+    REF_PRINT_COUNT("EVP_PKEY", i, pkey);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }
@@ -1793,7 +1793,7 @@ void EVP_PKEY_free(EVP_PKEY *x)
         return;
 
     CRYPTO_DOWN_REF(&x->references, &i);
-    REF_PRINT_COUNT("EVP_PKEY", x);
+    REF_PRINT_COUNT("EVP_PKEY", i, x);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c
index 5350a4e659e47cfc79b4ced521f3a3cc8a675284..eb9c44b85a0c6c43cc6e04fb4fa0c6799e95084f 100644 (file)
--- a/crypto/rsa/rsa_lib.c
+++ b/crypto/rsa/rsa_lib.c
@@ -141,7 +141,7 @@ void RSA_free(RSA *r)
         return;
 
     CRYPTO_DOWN_REF(&r->references, &i);
-    REF_PRINT_COUNT("RSA", r);
+    REF_PRINT_COUNT("RSA", i, r);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -188,7 +188,7 @@ int RSA_up_ref(RSA *r)
     if (CRYPTO_UP_REF(&r->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("RSA", r);
+    REF_PRINT_COUNT("RSA", i, r);
     REF_ASSERT_ISNT(i < 2);
     return i > 1 ? 1 : 0;
 }

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index e7fdf3d6ab66747bc7c94a83df67aa6c93ab35e3..09fa2ee1f74108c37f320761146fc2fbc9814b15 100644 (file)
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -232,7 +232,7 @@ void X509_STORE_free(X509_STORE *xs)
     if (xs == NULL)
         return;
     CRYPTO_DOWN_REF(&xs->references, &i);
-    REF_PRINT_COUNT("X509_STORE", xs);
+    REF_PRINT_COUNT("X509_STORE", i, xs);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -260,7 +260,7 @@ int X509_STORE_up_ref(X509_STORE *xs)
     if (CRYPTO_UP_REF(&xs->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("X509_STORE", xs);
+    REF_PRINT_COUNT("X509_STORE", i, xs);
     REF_ASSERT_ISNT(i < 2);
     return i > 1 ? 1 : 0;
 }

diff --git a/crypto/x509/x509_set.c b/crypto/x509/x509_set.c
index 0881be7292b06b6605dbf8df252dfb68fdf63b0b..c05d44097125b58917b46282f850ddf046319814 100644 (file)
--- a/crypto/x509/x509_set.c
+++ b/crypto/x509/x509_set.c
@@ -119,7 +119,7 @@ int X509_up_ref(X509 *x)
     if (CRYPTO_UP_REF(&x->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("X509", x);
+    REF_PRINT_COUNT("X509", i, x);
     REF_ASSERT_ISNT(i < 2);
     return i > 1;
 }

diff --git a/crypto/x509/x509cset.c b/crypto/x509/x509cset.c
index 205fe3d6e5a4d01a72f40520dc7b55a7ded04675..e5dd4d5c3a3dd0cd1872f995bfc67cd2251c8494 100644 (file)
--- a/crypto/x509/x509cset.c
+++ b/crypto/x509/x509cset.c
@@ -78,7 +78,7 @@ int X509_CRL_up_ref(X509_CRL *crl)
     if (CRYPTO_UP_REF(&crl->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("X509_CRL", crl);
+    REF_PRINT_COUNT("X509_CRL", i, crl);
     REF_ASSERT_ISNT(i < 2);
     return i > 1;
 }

diff --git a/include/internal/refcount.h b/include/internal/refcount.h
index 0ccafc7753780afb336ec4733545ae285c5dc0ae..3740a034e146b09e23c830458949ffb669edfb5d 100644 (file)
--- a/include/internal/refcount.h
+++ b/include/internal/refcount.h
@@ -297,7 +297,7 @@ static ossl_unused ossl_inline void CRYPTO_FREE_REF(CRYPTO_REF_COUNT *refcnt)
 
 # define REF_PRINT_EX(text, count, object) \
     OSSL_TRACE3(REF_COUNT, "%p:%4d:%s\n", (object), (count), (text));
-# define REF_PRINT_COUNT(text, object) \
-    REF_PRINT_EX(text, object->references.val, (void *)object)
+# define REF_PRINT_COUNT(text, val, object) \
+    REF_PRINT_EX(text, val, (void *)object)
 
 #endif

diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index c598a8c652034a2e53a9340074528061acfa50fb..1fa8d4518319efdad3f0afac211cb9010ac8e912 100644 (file)
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -267,7 +267,7 @@ void ssl_cert_free(CERT *c)
     if (c == NULL)
         return;
     CRYPTO_DOWN_REF(&c->references, &i);
-    REF_PRINT_COUNT("CERT", c);
+    REF_PRINT_COUNT("CERT", i, c);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/ssl/ssl_cert_comp.c b/ssl/ssl_cert_comp.c
index 639610a5f77e2c9afadfa86f6bf174b790d4a5f0..4c7cdf26826e38b5811a1d568e81b03779085d50 100644 (file)
--- a/ssl/ssl_cert_comp.c
+++ b/ssl/ssl_cert_comp.c
@@ -136,7 +136,7 @@ void OSSL_COMP_CERT_free(OSSL_COMP_CERT *cc)
         return;
 
     CRYPTO_DOWN_REF(&cc->references, &i);
-    REF_PRINT_COUNT("OSSL_COMP_CERT", cc);
+    REF_PRINT_COUNT("OSSL_COMP_CERT", i, cc);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -152,7 +152,7 @@ int OSSL_COMP_CERT_up_ref(OSSL_COMP_CERT *cc)
     if (CRYPTO_UP_REF(&cc->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("OSSL_COMP_CERT", cc);
+    REF_PRINT_COUNT("OSSL_COMP_CERT", i, cc);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index b06173f6ba635e87c283631d74cd2d79683ec707..23215f5b5f7f48aa50d54b1491d046c8219588d1 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -976,7 +976,7 @@ int SSL_up_ref(SSL *s)
     if (CRYPTO_UP_REF(&s->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("SSL", s);
+    REF_PRINT_COUNT("SSL", i, s);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }
@@ -1379,7 +1379,7 @@ void SSL_free(SSL *s)
     if (s == NULL)
         return;
     CRYPTO_DOWN_REF(&s->references, &i);
-    REF_PRINT_COUNT("SSL", s);
+    REF_PRINT_COUNT("SSL", i, s);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -4133,7 +4133,7 @@ int SSL_CTX_up_ref(SSL_CTX *ctx)
     if (CRYPTO_UP_REF(&ctx->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("SSL_CTX", ctx);
+    REF_PRINT_COUNT("SSL_CTX", i, ctx);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }
@@ -4147,7 +4147,7 @@ void SSL_CTX_free(SSL_CTX *a)
         return;
 
     CRYPTO_DOWN_REF(&a->references, &i);
-    REF_PRINT_COUNT("SSL_CTX", a);
+    REF_PRINT_COUNT("SSL_CTX", i, a);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 926dbe650badc568b477a8a3aa3e72da4f4872ce..29fa05cd58b10c3f8f52c2d686e1e61d0586ebbc 100644 (file)
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -843,7 +843,7 @@ void SSL_SESSION_free(SSL_SESSION *ss)
     if (ss == NULL)
         return;
     CRYPTO_DOWN_REF(&ss->references, &i);
-    REF_PRINT_COUNT("SSL_SESSION", ss);
+    REF_PRINT_COUNT("SSL_SESSION", i, ss);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -877,7 +877,7 @@ int SSL_SESSION_up_ref(SSL_SESSION *ss)
     if (CRYPTO_UP_REF(&ss->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("SSL_SESSION", ss);
+    REF_PRINT_COUNT("SSL_SESSION", i, ss);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }