From dc10ffc2834e0d2f5ebc1c3e29bd97f1f43a0ead Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Wed, 16 Oct 2024 14:34:08 -0400 Subject: [PATCH] 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 Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/25664) --- crypto/bio/bio_lib.c | 4 ++-- crypto/dh/dh_lib.c | 4 ++-- crypto/dsa/dsa_lib.c | 4 ++-- crypto/dso/dso_lib.c | 4 ++-- crypto/ec/ec_key.c | 4 ++-- crypto/ec/ec_mult.c | 2 +- crypto/ec/ecp_nistp224.c | 2 +- crypto/ec/ecp_nistp256.c | 2 +- crypto/ec/ecp_nistp384.c | 2 +- crypto/ec/ecp_nistp521.c | 2 +- crypto/ec/ecp_nistz256.c | 2 +- crypto/ec/ecx_key.c | 4 ++-- crypto/evp/p_lib.c | 4 ++-- crypto/rsa/rsa_lib.c | 4 ++-- crypto/x509/x509_lu.c | 4 ++-- crypto/x509/x509_set.c | 2 +- crypto/x509/x509cset.c | 2 +- include/internal/refcount.h | 4 ++-- ssl/ssl_cert.c | 2 +- ssl/ssl_cert_comp.c | 4 ++-- ssl/ssl_lib.c | 8 ++++---- ssl/ssl_sess.c | 4 ++-- 22 files changed, 37 insertions(+), 37 deletions(-) diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c index 272189a9a6..85ab4afe18 100644 --- 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 9d5a6b0b6c..93e08b3f8c 100644 --- 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 7997c2ac25..db6e3b059b 100644 --- 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 8f3387e9b8..65579cb8b3 100644 --- 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 05224b31a4..681488e3f3 100644 --- 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 9eb007cdf9..e9092a6c9d 100644 --- 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 debfdb3dc9..6485f46f71 100644 --- 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 325ace67bc..eaf9dddbc8 100644 --- 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 e2a0e36488..44ac1cea3d 100644 --- 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 db5a9dd5de..fe6836a147 100644 --- 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 55ae2651ac..36b1d164f4 100644 --- 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 98f68cd013..7b09494912 100644 --- 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 09bd185a25..2eb142fa76 100644 --- 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 6dfb02f5f2..d9ceb80880 100644 --- 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 b76cb08209..7b0979b5b8 100644 --- 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 2aba0e8c14..0c9df51b3c 100644 --- 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 205fe3d6e5..e5dd4d5c3a 100644 --- 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 a90fa2453a..8de230f343 100644 --- 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 0303d07578..4aef149520 100644 --- 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 ba9bfb480c..7bf49d435f 100644 --- 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 6f6adf8963..5afd89c3e9 100644 --- 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 f35fcc7748..69149de050 100644 --- 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); }