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)

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 05224b31a4f02a1d03c83eaf17b32e5647c5e08d..681488e3f30efc0cd2390089f8dcd3d44357e059 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 325ace67bc13e28e16a832c827f802ba88afada6..eaf9dddbc8ec63ef9de5c92fdfb1fdf7e0b692f2 100644 (file)
--- a/crypto/ec/ecp_nistp256.c
+++ b/crypto/ec/ecp_nistp256.c
@@ -1876,7 +1876,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 e2a0e36488fa5b55e4828957192fd4c816753819..44ac1cea3d0c0d2b821c39265fa3a37aa26f425d 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 55ae2651acc54f5797b6b12a479550673c3e3b33..36b1d164f4bfeb38118b756e8da1b554bd70c1b1 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 98f68cd013438d234e2471733bb91bfc698f3c1c..7b094949126f601bd7938bdb0daba8be3e8b131c 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);
@@ -95,7 +95,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 09bd185a25b60bc05798674d7bd316202e4e5db1..2eb142fa7660eac302edf096c2f4a3c852848acf 100644 (file)
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1671,7 +1671,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);
 }
@@ -1792,7 +1792,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 6dfb02f5f21c923a0b99b16b07376b2e4d22a634..d9ceb80880056e708cccebdaa297391bbb7f16ae 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);
@@ -193,7 +193,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 b76cb0820984761bc13ab3efffd9fc570c90fc23..7b0979b5b8e015d3a553389335307b62dac8722b 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 2aba0e8c1426307cf92b91ad0ce6ee3c2d818c9e..0c9df51b3c7ea64247b3a1c19c8e480a68066e28 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 a90fa2453aefb3a47d83aca2c3c677e9ec4c2079..8de230f343ac115cef2de3e851d8409dfd93df76 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 0303d075780729b45dd0614c3db33c35b0a6f3dc..4aef14952006bd1c300487aecc0c533584b91a98 100644 (file)
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -269,7 +269,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 ba9bfb480c03c68a265ebc134f28b28b3304db8f..7bf49d435f575cfa95d940c7ccd8428e12430057 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 6f6adf89637d2827a4ecf5c271841bfdb8046ca6..5afd89c3e9c6f911bbabcec70c01cb1a6e17dedc 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -985,7 +985,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);
 }
@@ -1386,7 +1386,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);
@@ -4258,7 +4258,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);
 }
@@ -4272,7 +4272,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 f35fcc77481743950deaa9979807164064f228fc..69149de0507c9888b8c2e3d0f1a7f733464eeeee 100644 (file)
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -844,7 +844,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);
@@ -878,7 +878,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);
 }