v16-0004-Change-pgcrypto-to-use-the-new-ResourceOwner-mec.patch

From f9b1c8e56847abf7deab1e8b0eb81d2dc14d2bcc Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed, 25 Oct 2023 14:46:01 +0300
Subject: [PATCH v16 4/4] Change pgcrypto to use the new ResourceOwner
 mechanism.

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 cf315517e0c..4a913bd04f7 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,54 +59,39 @@ 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 void
-free_openssl_digest(OSSLDigest *digest)
+static const ResourceOwnerDesc ossldigest_resowner_desc =
 {
-	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;
-	pfree(digest);
+	.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);
 }
 
-/*
- * Close any open OpenSSL handles on abort.
- */
 static void
-digest_free_callback(ResourceReleasePhase phase,
-					 bool isCommit,
-					 bool isTopLevel,
-					 void *arg)
+free_openssl_digest(OSSLDigest *digest)
 {
-	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);
-		}
-	}
+	EVP_MD_CTX_destroy(digest->ctx);
+	if (digest->owner != NULL)
+		ResourceOwnerForgetOSSLDigest(digest->owner, digest);
+	pfree(digest);
 }
 
 static unsigned
@@ -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,54 +269,39 @@ 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 void
-free_openssl_cipher(OSSLCipher *od)
+static const ResourceOwnerDesc osslcipher_resowner_desc =
 {
-	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;
-	pfree(od);
+	.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);
 }
 
-/*
- * Close any open OpenSSL cipher handles on abort.
- */
 static void
-cipher_free_callback(ResourceReleasePhase phase,
-					 bool isCommit,
-					 bool isTopLevel,
-					 void *arg)
+free_openssl_cipher(OSSLCipher *od)
 {
-	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);
-		}
-	}
+	EVP_CIPHER_CTX_free(od->evp_ctx);
+	if (od->owner != NULL)
+		ResourceOwnerForgetOSSLCipher(od->owner, od);
+	pfree(od);
 }
 
 /* Common routines for all algorithms */
@@ -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));
+}
-- 
2.39.2