From 12b8786a4525306e78e252340b026531f6d04933 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 5 Nov 2020 15:29:54 +0900 Subject: [PATCH v3 2/3] Switch sha2_openssl.c to use EVP Postgres is two decades late for this switch. --- src/include/utils/resowner_private.h | 7 +++ src/backend/utils/resowner/resowner.c | 65 ++++++++++++++++++++ src/common/sha2_openssl.c | 88 +++++++++++++-------------- 3 files changed, 113 insertions(+), 47 deletions(-) diff --git a/src/include/utils/resowner_private.h b/src/include/utils/resowner_private.h index a781a7a2aa..5ce6fcf882 100644 --- a/src/include/utils/resowner_private.h +++ b/src/include/utils/resowner_private.h @@ -95,4 +95,11 @@ extern void ResourceOwnerRememberJIT(ResourceOwner owner, extern void ResourceOwnerForgetJIT(ResourceOwner owner, Datum handle); +/* support for EVP context management */ +extern void ResourceOwnerEnlargeEVP(ResourceOwner owner); +extern void ResourceOwnerRememberEVP(ResourceOwner owner, + Datum handle); +extern void ResourceOwnerForgetEVP(ResourceOwner owner, + Datum handle); + #endif /* RESOWNER_PRIVATE_H */ diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c index 8bc2c4e9ea..1efb5e98b4 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -20,6 +20,10 @@ */ #include "postgres.h" +#ifdef USE_OPENSSL +#include +#endif + #include "common/hashfn.h" #include "jit/jit.h" #include "storage/bufmgr.h" @@ -128,6 +132,7 @@ typedef struct ResourceOwnerData ResourceArray filearr; /* open temporary files */ ResourceArray dsmarr; /* dynamic shmem segments */ ResourceArray jitarr; /* JIT contexts */ + ResourceArray evparr; /* EVP contexts */ /* We can remember up to MAX_RESOWNER_LOCKS references to local locks. */ int nlocks; /* number of owned locks */ @@ -175,6 +180,7 @@ static void PrintTupleDescLeakWarning(TupleDesc tupdesc); static void PrintSnapshotLeakWarning(Snapshot snapshot); static void PrintFileLeakWarning(File file); static void PrintDSMLeakWarning(dsm_segment *seg); +static void PrintEVPLeakWarning(Datum handle); /***************************************************************************** @@ -444,6 +450,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name) ResourceArrayInit(&(owner->filearr), FileGetDatum(-1)); ResourceArrayInit(&(owner->dsmarr), PointerGetDatum(NULL)); ResourceArrayInit(&(owner->jitarr), PointerGetDatum(NULL)); + ResourceArrayInit(&(owner->evparr), PointerGetDatum(NULL)); return owner; } @@ -553,6 +560,17 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, jit_release_context(context); } + + /* Ditto for EVP contexts */ + while (ResourceArrayGetAny(&(owner->evparr), &foundres)) + { + if (isCommit) + PrintEVPLeakWarning(foundres); +#ifdef USE_OPENSSL + EVP_MD_CTX_destroy((EVP_MD_CTX *) DatumGetPointer(foundres)); +#endif + ResourceOwnerForgetEVP(owner, foundres); + } } else if (phase == RESOURCE_RELEASE_LOCKS) { @@ -725,6 +743,7 @@ ResourceOwnerDelete(ResourceOwner owner) Assert(owner->filearr.nitems == 0); Assert(owner->dsmarr.nitems == 0); Assert(owner->jitarr.nitems == 0); + Assert(owner->evparr.nitems == 0); Assert(owner->nlocks == 0 || owner->nlocks == MAX_RESOWNER_LOCKS + 1); /* @@ -752,6 +771,7 @@ ResourceOwnerDelete(ResourceOwner owner) ResourceArrayFree(&(owner->filearr)); ResourceArrayFree(&(owner->dsmarr)); ResourceArrayFree(&(owner->jitarr)); + ResourceArrayFree(&(owner->evparr)); pfree(owner); } @@ -1370,3 +1390,48 @@ ResourceOwnerForgetJIT(ResourceOwner owner, Datum handle) elog(ERROR, "JIT context %p is not owned by resource owner %s", DatumGetPointer(handle), owner->name); } + +/* + * Make sure there is room for at least one more entry in a ResourceOwner's + * EVP context reference array. + * + * This is separate from actually inserting an entry because if we run out of + * memory, it's critical to do so *before* acquiring the resource. + */ +void +ResourceOwnerEnlargeEVP(ResourceOwner owner) +{ + ResourceArrayEnlarge(&(owner->evparr)); +} + +/* + * Remember that an EVP context is owned by a ResourceOwner + * + * Caller must have previously done ResourceOwnerEnlargeEVP() + */ +void +ResourceOwnerRememberEVP(ResourceOwner owner, Datum handle) +{ + ResourceArrayAdd(&(owner->evparr), handle); +} + +/* + * Forget that an EVP context is owned by a ResourceOwner + */ +void +ResourceOwnerForgetEVP(ResourceOwner owner, Datum handle) +{ + if (!ResourceArrayRemove(&(owner->evparr), handle)) + elog(ERROR, "EVP context %p is not owned by resource owner %s", + DatumGetPointer(handle), owner->name); +} + +/* + * Debugging subroutine + */ +static void +PrintEVPLeakWarning(Datum handle) +{ + elog(WARNING, "EVP context reference leak: context %p still referenced", + DatumGetPointer(handle)); +} diff --git a/src/common/sha2_openssl.c b/src/common/sha2_openssl.c index 57de96df90..8cfe69cd69 100644 --- a/src/common/sha2_openssl.c +++ b/src/common/sha2_openssl.c @@ -20,9 +20,14 @@ #include "postgres_fe.h" #endif -#include +#include #include "common/sha2.h" +#ifndef FRONTEND +#include "utils/memutils.h" +#include "utils/resowner.h" +#include "utils/resowner_private.h" +#endif /* * In backend, use palloc/pfree to ease the error handling. In frontend, @@ -52,25 +57,31 @@ pg_sha2_create(pg_sha2_type type) ctx->type = type; - switch (type) - { - case PG_SHA224: - case PG_SHA256: - ctx->data = ALLOC(sizeof(SHA256_CTX)); - break; - case PG_SHA384: - case PG_SHA512: - ctx->data = ALLOC(sizeof(SHA512_CTX)); - break; - } +#ifndef FRONTEND + ResourceOwnerEnlargeEVP(CurrentResourceOwner); +#endif + + /* + * Initialization takes care of assigning the correct type for OpenSSL. + */ + ctx->data = EVP_MD_CTX_create(); if (ctx->data == NULL) { explicit_bzero(ctx, sizeof(pg_sha2_ctx)); +#ifndef FRONTEND + elog(ERROR, "out of memory"); +#else FREE(ctx); return NULL; +#endif } +#ifndef FRONTEND + ResourceOwnerRememberEVP(CurrentResourceOwner, + PointerGetDatum(ctx->data)); +#endif + return ctx; } @@ -90,16 +101,20 @@ pg_sha2_init(pg_sha2_ctx *ctx) switch (ctx->type) { case PG_SHA224: - status = SHA224_Init((SHA256_CTX *) ctx->data); + status = EVP_DigestInit_ex((EVP_MD_CTX *) ctx->data, + EVP_sha224(), NULL); break; case PG_SHA256: - status = SHA256_Init((SHA256_CTX *) ctx->data); + status = EVP_DigestInit_ex((EVP_MD_CTX *) ctx->data, + EVP_sha256(), NULL); break; case PG_SHA384: - status = SHA384_Init((SHA512_CTX *) ctx->data); + status = EVP_DigestInit_ex((EVP_MD_CTX *) ctx->data, + EVP_sha384(), NULL); break; case PG_SHA512: - status = SHA512_Init((SHA512_CTX *) ctx->data); + status = EVP_DigestInit_ex((EVP_MD_CTX *) ctx->data, + EVP_sha512(), NULL); break; } @@ -122,21 +137,7 @@ pg_sha2_update(pg_sha2_ctx *ctx, const uint8 *data, size_t len) if (ctx == NULL) return 0; - switch (ctx->type) - { - case PG_SHA224: - status = SHA224_Update((SHA256_CTX *) ctx->data, data, len); - break; - case PG_SHA256: - status = SHA256_Update((SHA256_CTX *) ctx->data, data, len); - break; - case PG_SHA384: - status = SHA384_Update((SHA512_CTX *) ctx->data, data, len); - break; - case PG_SHA512: - status = SHA512_Update((SHA512_CTX *) ctx->data, data, len); - break; - } + status = EVP_DigestUpdate((EVP_MD_CTX *) ctx->data, data, len); /* OpenSSL internals return 1 on success, 0 on failure */ if (status <= 0) @@ -157,21 +158,7 @@ pg_sha2_final(pg_sha2_ctx *ctx, uint8 *dest) if (ctx == NULL) return 0; - switch (ctx->type) - { - case PG_SHA224: - status = SHA224_Final(dest, (SHA256_CTX *) ctx->data); - break; - case PG_SHA256: - status = SHA256_Final(dest, (SHA256_CTX *) ctx->data); - break; - case PG_SHA384: - status = SHA384_Final(dest, (SHA512_CTX *) ctx->data); - break; - case PG_SHA512: - status = SHA512_Final(dest, (SHA512_CTX *) ctx->data); - break; - } + status = EVP_DigestFinal_ex((EVP_MD_CTX *) ctx->data, dest, 0); /* OpenSSL internals return 1 on success, 0 on failure */ if (status <= 0) @@ -189,7 +176,14 @@ pg_sha2_free(pg_sha2_ctx *ctx) { if (ctx == NULL) return; - FREE(ctx->data); + + EVP_MD_CTX_destroy((EVP_MD_CTX *) ctx->data); + +#ifndef FRONTEND + ResourceOwnerForgetEVP(CurrentResourceOwner, + PointerGetDatum(ctx->data)); +#endif + explicit_bzero(ctx, sizeof(pg_sha2_ctx)); FREE(ctx); } -- 2.29.1