Re: using explicit_bzero

Hi,

On 2019-07-29 11:30:53 +0200, Peter Eisentraut wrote:
> For platforms that don't have explicit_bzero(), provide various
> fallback implementations. (explicit_bzero() itself isn't standard,
> but as Linux/glibc and OpenBSD have it, it's the most common spelling,
> so it makes sense to make that the invocation point.)

I think it's better to have a pg_explicit_bzero or such, and implement
that via the various platform dependant mechanisms. It's considerably
harder to understand code when one is surprised that a function normally
not available is called, the buildsystem part is really hard to
understand (with runtime and code filenames differing etc), and invites
API breakages. And it's not really more work to have our own name.

> +/*-------------------------------------------------------------------------
> + *
> + * explicit_bzero.c
> + *
> + * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
> + * Portions Copyright (c) 1994, Regents of the University of California
> + *
> + *-------------------------------------------------------------------------
> + */
> +
> +#include "c.h"
> +
> +#if defined(HAVE_MEMSET_S)
> +
> +void
> +explicit_bzero(void *buf, size_t len)
> +{
> + (void) memset_s(buf, len, 0, len);
> +}
> +
> +#elif defined(WIN32)
> +
> +#include "c.h"

Hm?

> +/*
> + * Indirect call through a volatile pointer to hopefully avoid dead-store
> + * optimisation eliminating the call. (Idea taken from OpenSSH.) We can't
> + * assume bzero() is present either, so for simplicity we define our own.
> + */
> +
> +static void
> +bzero2(void *buf, size_t len)
> +{
> + memset(buf, 0, len);
> +}
> +
> +static void (* volatile bzero_p)(void *, size_t) = bzero2;

Hm, I'm not really sure that this does that much. Especially when the
call is via a function in the same translation unit.

> +void
> +explicit_bzero(void *buf, size_t len)
> +{
> + bzero_p(buf, len);

I've not followed this discussion. But why isn't the obvious
implementation here memset(...); pg_compiler_barrier()?

A quick web search indicates that that's what a bunch of projects in the
cryptography space also ended up with (well, __asm__ __volatile__("" :::
"memory"), which is what pg_compiler_barrier boils down to for
gcc/clang/compatibles).

Greetings,

Andres Freund