From cd694f60dc975e9fe41e8643ca6f0629283d102e Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 8 Nov 2023 13:30:55 +0200 Subject: [PATCH] Change pgcrypto to use the new ResourceOwner mechanism. This is a nice example of how extensions can now use ResourceOwners to track extension-specific resource kinds Reviewed-by: Peter Eisentraut, Andres Freund Discussion: https://www.postgresql.org/message-id/d746cead-a1ef-7efe-fb47-933311e876a3%40iki.fi --- contrib/pgcrypto/openssl.c | 181 ++++++++++++++++--------------------- 1 file changed, 78 insertions(+), 103 deletions(-) diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c index cf315517e0..4a913bd04f 100644 --- a/contrib/pgcrypto/openssl.c +++ b/contrib/pgcrypto/openssl.c @@ -50,9 +50,8 @@ */ /* - * To make sure we don't leak OpenSSL handles on abort, we keep OSSLDigest - * objects in a linked list, allocated in TopMemoryContext. We use the - * ResourceOwner mechanism to free them on abort. + * To make sure we don't leak OpenSSL handles, we use the ResourceOwner + * mechanism to free them on abort. */ typedef struct OSSLDigest { @@ -60,56 +59,41 @@ typedef struct OSSLDigest EVP_MD_CTX *ctx; ResourceOwner owner; - struct OSSLDigest *next; - struct OSSLDigest *prev; } OSSLDigest; -static OSSLDigest *open_digests = NULL; -static bool digest_resowner_callback_registered = false; +/* ResourceOwner callbacks to hold OpenSSL digest handles */ +static void ResOwnerReleaseOSSLDigest(Datum res); + +static const ResourceOwnerDesc ossldigest_resowner_desc = +{ + .name = "pgcrypto OpenSSL digest handle", + .release_phase = RESOURCE_RELEASE_BEFORE_LOCKS, + .release_priority = RELEASE_PRIO_FIRST, + .ReleaseResource = ResOwnerReleaseOSSLDigest, + .DebugPrint = NULL, /* default message is fine */ +}; + +/* Convenience wrappers over ResourceOwnerRemember/Forget */ +static inline void +ResourceOwnerRememberOSSLDigest(ResourceOwner owner, OSSLDigest *digest) +{ + ResourceOwnerRemember(owner, PointerGetDatum(digest), &ossldigest_resowner_desc); +} +static inline void +ResourceOwnerForgetOSSLDigest(ResourceOwner owner, OSSLDigest *digest) +{ + ResourceOwnerForget(owner, PointerGetDatum(digest), &ossldigest_resowner_desc); +} static void free_openssl_digest(OSSLDigest *digest) { EVP_MD_CTX_destroy(digest->ctx); - if (digest->prev) - digest->prev->next = digest->next; - else - open_digests = digest->next; - if (digest->next) - digest->next->prev = digest->prev; + if (digest->owner != NULL) + ResourceOwnerForgetOSSLDigest(digest->owner, digest); pfree(digest); } -/* - * Close any open OpenSSL handles on abort. - */ -static void -digest_free_callback(ResourceReleasePhase phase, - bool isCommit, - bool isTopLevel, - void *arg) -{ - OSSLDigest *curr; - OSSLDigest *next; - - if (phase != RESOURCE_RELEASE_AFTER_LOCKS) - return; - - next = open_digests; - while (next) - { - curr = next; - next = curr->next; - - if (curr->owner == CurrentResourceOwner) - { - if (isCommit) - elog(WARNING, "pgcrypto digest reference leak: digest %p still referenced", curr); - free_openssl_digest(curr); - } - } -} - static unsigned digest_result_size(PX_MD *h) { @@ -188,16 +172,12 @@ px_find_digest(const char *name, PX_MD **res) OpenSSL_add_all_algorithms(); } - if (!digest_resowner_callback_registered) - { - RegisterResourceReleaseCallback(digest_free_callback, NULL); - digest_resowner_callback_registered = true; - } - md = EVP_get_digestbyname(name); if (md == NULL) return PXE_NO_HASH; + ResourceOwnerEnlarge(CurrentResourceOwner); + /* * Create an OSSLDigest object, an OpenSSL MD object, and a PX_MD object. * The order is crucial, to make sure we don't leak anything on @@ -221,9 +201,7 @@ px_find_digest(const char *name, PX_MD **res) digest->algo = md; digest->ctx = ctx; digest->owner = CurrentResourceOwner; - digest->next = open_digests; - digest->prev = NULL; - open_digests = digest; + ResourceOwnerRememberOSSLDigest(digest->owner, digest); /* The PX_MD object is allocated in the current memory context. */ h = palloc(sizeof(*h)); @@ -239,6 +217,17 @@ px_find_digest(const char *name, PX_MD **res) return 0; } +/* ResourceOwner callbacks for OSSLDigest */ + +static void +ResOwnerReleaseOSSLDigest(Datum res) +{ + OSSLDigest *digest = (OSSLDigest *) DatumGetPointer(res); + + digest->owner = NULL; + free_openssl_digest(digest); +} + /* * Ciphers * @@ -266,9 +255,8 @@ struct ossl_cipher * OSSLCipher contains the state for using a cipher. A separate OSSLCipher * object is allocated in each px_find_cipher() call. * - * To make sure we don't leak OpenSSL handles on abort, we keep OSSLCipher - * objects in a linked list, allocated in TopMemoryContext. We use the - * ResourceOwner mechanism to free them on abort. + * To make sure we don't leak OpenSSL handles, we use the ResourceOwner + * mechanism to free them on abort. */ typedef struct OSSLCipher { @@ -281,56 +269,41 @@ typedef struct OSSLCipher const struct ossl_cipher *ciph; ResourceOwner owner; - struct OSSLCipher *next; - struct OSSLCipher *prev; } OSSLCipher; -static OSSLCipher *open_ciphers = NULL; -static bool cipher_resowner_callback_registered = false; +/* ResourceOwner callbacks to hold OpenSSL cipher state */ +static void ResOwnerReleaseOSSLCipher(Datum res); + +static const ResourceOwnerDesc osslcipher_resowner_desc = +{ + .name = "pgcrypto OpenSSL cipher handle", + .release_phase = RESOURCE_RELEASE_BEFORE_LOCKS, + .release_priority = RELEASE_PRIO_FIRST, + .ReleaseResource = ResOwnerReleaseOSSLCipher, + .DebugPrint = NULL, /* default message is fine */ +}; + +/* Convenience wrappers over ResourceOwnerRemember/Forget */ +static inline void +ResourceOwnerRememberOSSLCipher(ResourceOwner owner, OSSLCipher *od) +{ + ResourceOwnerRemember(owner, PointerGetDatum(od), &osslcipher_resowner_desc); +} +static inline void +ResourceOwnerForgetOSSLCipher(ResourceOwner owner, OSSLCipher *od) +{ + ResourceOwnerForget(owner, PointerGetDatum(od), &osslcipher_resowner_desc); +} static void free_openssl_cipher(OSSLCipher *od) { EVP_CIPHER_CTX_free(od->evp_ctx); - if (od->prev) - od->prev->next = od->next; - else - open_ciphers = od->next; - if (od->next) - od->next->prev = od->prev; + if (od->owner != NULL) + ResourceOwnerForgetOSSLCipher(od->owner, od); pfree(od); } -/* - * Close any open OpenSSL cipher handles on abort. - */ -static void -cipher_free_callback(ResourceReleasePhase phase, - bool isCommit, - bool isTopLevel, - void *arg) -{ - OSSLCipher *curr; - OSSLCipher *next; - - if (phase != RESOURCE_RELEASE_AFTER_LOCKS) - return; - - next = open_ciphers; - while (next) - { - curr = next; - next = curr->next; - - if (curr->owner == CurrentResourceOwner) - { - if (isCommit) - elog(WARNING, "pgcrypto cipher reference leak: cipher %p still referenced", curr); - free_openssl_cipher(curr); - } - } -} - /* Common routines for all algorithms */ static unsigned @@ -782,11 +755,7 @@ px_find_cipher(const char *name, PX_Cipher **res) if (i->name == NULL) return PXE_NO_CIPHER; - if (!cipher_resowner_callback_registered) - { - RegisterResourceReleaseCallback(cipher_free_callback, NULL); - cipher_resowner_callback_registered = true; - } + ResourceOwnerEnlarge(CurrentResourceOwner); /* * Create an OSSLCipher object, an EVP_CIPHER_CTX object and a PX_Cipher. @@ -806,9 +775,7 @@ px_find_cipher(const char *name, PX_Cipher **res) od->evp_ctx = ctx; od->owner = CurrentResourceOwner; - od->next = open_ciphers; - od->prev = NULL; - open_ciphers = od; + ResourceOwnerRememberOSSLCipher(od->owner, od); if (i->ciph->cipher_func) od->evp_ciph = i->ciph->cipher_func(); @@ -827,3 +794,11 @@ px_find_cipher(const char *name, PX_Cipher **res) *res = c; return 0; } + +/* ResourceOwner callbacks for OSSLCipher */ + +static void +ResOwnerReleaseOSSLCipher(Datum res) +{ + free_openssl_cipher((OSSLCipher *) DatumGetPointer(res)); +}