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

Attachment: signature.asc
Description: PGP signature

Reply via email to