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)
This commit is contained in:
Neil Horman 2024-10-16 14:34:08 -04:00 committed by Tomas Mraz
parent 3bf273b21b
commit dc10ffc283
22 changed files with 37 additions and 37 deletions

View File

@ -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;
}

View File

@ -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);
}

View File

@ -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);
}

View File

@ -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);
}

View File

@ -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);
}

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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);
}

View File

@ -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);

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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

View File

@ -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);

View File

@ -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);
}

View File

@ -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);

View File

@ -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);
}