From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
Cc: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Refactoring base64 encoding and decoding into a safer interface |
Date: | 2019-07-02 05:41:08 |
Message-ID: | 20190702054108.GG1388@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jul 01, 2019 at 11:11:43PM +0200, Daniel Gustafsson wrote:
> I very much agree that functions operating on a buffer like this should have
> the size of the buffer in order to safeguard against overflow, so +1 on the
> general concept.
Thanks for the review!
> A few small comments:
>
> In src/common/scram-common.c there are a few instances like this. Shouldn’t we
> also free the result buffer in these cases?
>
> +#ifdef FRONTEND
> + return NULL;
> +#else
Fixed.
> In the below passage, we leave the input buffer with a non-complete
> encoded string. Should we memset the buffer to zero to avoid the
> risk that code which fails to check the return value believes it has
> an encoded string?
Hmm. Good point. I have not thought of that, and your suggestion
makes sense.
Another question is if we'd want to actually use explicit_bzero()
here, but that could be a discussion on this other thread, except if
the patch discussed there is merged first:
https://www.postgresql.org/message-id/42d26bde-5d5b-c90d-87ae-6cab875f73be@2ndquadrant.com
Attached is an updated patch.
--
Michael
Attachment | Content-Type | Size |
---|---|---|
base64-refactor-safe-v2.patch | text/x-diff | 17.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Paul Guo | 2019-07-02 05:46:21 | Re: Two pg_rewind patches (auto generate recovery conf and ensure clean shutdown) |
Previous Message | Julien Rouhaud | 2019-07-02 05:12:38 | Re: cleanup & refactoring on reindexdb.c |