From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Proposed patch for key managment |
Date: | 2020-12-05 12:54:33 |
Message-ID: | X8uDCWWf9BlBFa/B@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Dec 04, 2020 at 10:52:29PM -0500, Bruce Momjian wrote:
> OK, I worked with Sawada-san and added the attached patch. The updated
> full patch is at the same URL: :-)
>
> https://github.com/postgres/postgres/compare/master...bmomjian:key.diff
Oh, I see that you use SHA256 during firstboot, which is why you
bypass the resowner paths in cryptohash_openssl.c. Wouldn't it be
better to use IsBootstrapProcessingMode() then?
> @@ -72,14 +72,15 @@ pg_cryptohash_create(pg_cryptohash_type type)
> ctx = ALLOC(sizeof(pg_cryptohash_ctx));
> if (ctx == NULL)
> return NULL;
> + explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
>
> state = ALLOC(sizeof(pg_cryptohash_state));
> if (state == NULL)
> {
> - explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
> FREE(ctx);
> return NULL;
> }
> + explicit_bzero(state, sizeof(pg_cryptohash_state));
explicit_bzero() is used to give the guarantee that any sensitive data
gets cleaned up after an error or on free. So I think that your use
is incorrect here for an initialization.
> if (state->evpctx == NULL)
> {
> - explicit_bzero(state, sizeof(pg_cryptohash_state));
> - explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
> #ifndef FRONTEND
> ereport(ERROR,
> (errcode(ERRCODE_OUT_OF_MEMORY),
And this original position is IMO more correct.
Anyway, at quick glance, the backtrace of upthread is indicating a
double-free with an attempt to free a resource owner that has been
already free'd.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2020-12-05 14:41:16 | Re: A few new options for CHECKPOINT |
Previous Message | Laurenz Albe | 2020-12-05 12:04:13 | Re: Add session statistics to pg_stat_database |