Re: Support getrandom() for pg_strong_random() source

Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> writes:

> On Tue, Jul 29, 2025 at 8:55 AM Jacob Champion
> <jacob(dot)champion(at)enterprisedb(dot)com> wrote:
>>
>> On Mon, Jul 28, 2025 at 6:30 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> > My understanding of the problem is that it is a choice of efficiency
>> > vs entropy, and that it's not really possible to have both parts of
>> > the cake.
>
> Agreed. I think the optimal choice would depend on the specific use
> case. For instance, since UUIDs are not intended for security
> purposes, they don't require particularly high entropy. In UUID
> generation, the efficiency of random data generation tends to be
> prioritized over the quality of randomness.
>
>>
>> > Could getentropy() be more efficient at the end on most platforms,
>> > meaning that this could limit the meaning of having a GUC switch?
>>
>> I don't know. [2] implies that the performance comparison depends on
>> several factors, and falls in favor of OpenSSL when the number of
>> bytes per call is large -- but our use of pg_strong_random() is
>> generally on small buffers. We would need to do a _lot_ more research
>> before, say, switching any defaults.
>
> The performance issue with getentropy, particularly when len=1024,
> likely stems from the need for multiple getentropy() calls due to its
> 256-byte length restriction.
>
> Analysis of RAND_bytes() through strace reveals that it internally
> makes calls to getrandom() with a fixed length of 32 bytes. While I'm
> uncertain of the exact purpose, it's logical that a single
> getentropy() call would be more efficient than RAND_bytes(), which
> involves additional overhead beyond just calling getrandom(),
> especially when dealing with smaller byte sizes.
>
> I've updated the patch to support getentropy() instead of getrandom().

Thanks, just a few comments:

The blog post at
https://dotat.at/@/2024-10-01-getentropy.html#portability-of-getentropy-
points out a couple of caveats:

* Originally getentropy() was declared in <sys/random.h> but POSIX
declares it in <unistd.h>. You need to include both headers to be
sure.

So the probes need to include both <sys/random.h> (if avaliable) and
<unistd.h>, and in the code <sys/random.h> should only be included if
available.

* POSIX specifies a GETENTROPY_MAX macro in <limits.h> for the largest
buffer getentropy() will fill. Most systems don’t yet have this
macro; if it isn’t defined the limit is 256 bytes.

And this means we should include <limits.h> and only define
GETENTROPY_MAX to 256 if it's not already defined.

> +bool
> +pg_strong_random(void *buf, size_t len)
> +{
> + char *p = buf;
> + ssize_t res;
> +
> + while (len)
> + {
> + size_t l = Min(len, GETENTROPY_MAX_LEN);
> +
> + res = getentropy(buf, l);

This should be getentropy(p, l), otherwise it will will just fill the
first GETENTROPY_MAX_LEN bytes of buf repeatedly. On my machine I got a
warning about that:

../postgresql/src/port/pg_strong_random.c:159:11: warning: variable 'p' set but not used [-Wunused-but-set-variable]
159 | char *p = buf;
| ^

Do we not have any tests for pg_strong_random that make sure it fills
the entire bufffer for various sizes?

I've attached an updated patch for the above, except I don't know enough
autoconf/m4 to make it include <unistd.h> and optionally <sys/random.h>
in the check there.

- ilmari