Re: [PATCH v1 1/2] random-bits: Factor out entropy generating function

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 29/03/2022 17:14, H.J. Lu wrote:
> On Tue, Mar 29, 2022 at 12:56 PM Noah Goldstein via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> On Tue, Mar 29, 2022 at 2:51 PM Adhemerval Zanella
>> <adhemerval.zanella@linaro.org> wrote:
>>>
>>>
>>>
>>> On 28/03/2022 19:09, Noah Goldstein via Libc-alpha wrote:
>>>> On some architectures `clock_gettime` is undesirable as
>>>> it may use a syscall or there may be a faster alternative.
>>>> Future architecture specific functions can be added in
>>>> sysdeps/<arch>/random-bits-entropy.h to provide a version of
>>>> 'random_bits_entropy' that doesn't use 'clock_gettime'.
>>>> ---
>>>>  include/random-bits.h                 | 16 ++++++--------
>>>>  sysdeps/generic/random-bits-entropy.h | 31 +++++++++++++++++++++++++++
>>>>  2 files changed, 37 insertions(+), 10 deletions(-)
>>>>  create mode 100644 sysdeps/generic/random-bits-entropy.h
>>>>
>>>> diff --git a/include/random-bits.h b/include/random-bits.h
>>>> index 17665b479a..016b87576c 100644
>>>> --- a/include/random-bits.h
>>>> +++ b/include/random-bits.h
>>>> @@ -19,21 +19,17 @@
>>>>  #ifndef _RANDOM_BITS_H
>>>>  # define _RANDOM_BITS_H
>>>>
>>>> -#include <time.h>
>>>> -#include <stdint.h>
>>>> +# include <random-bits-entropy.h>
>>>> +# include <stdint.h>
>>>>
>>>> -/* Provides fast pseudo-random bits through clock_gettime.  It has unspecified
>>>> -   starting time, nano-second accuracy, its randomness is significantly better
>>>> -   than gettimeofday, and for mostly architectures it is implemented through
>>>> -   vDSO instead of a syscall.  Since the source is a system clock, the upper
>>>> -   bits will have less entropy. */
>>>> +/* Provides fast pseudo-random bits through architecture specific
>>>> +   random_bits_entropy.  Expectation is source is some timing function so
>>>> +   the upper bits have less entropy.  */
>>>>  static inline uint32_t
>>>>  random_bits (void)
>>>>  {
>>>> -  struct __timespec64 tv;
>>>> -  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>>>> +  uint32_t ret = random_bits_entropy ();
>>>>    /* Shuffle the lower bits to minimize the clock bias.  */
>>>> -  uint32_t ret = tv.tv_nsec ^ tv.tv_sec;
>>>>    ret ^= (ret << 24) | (ret >> 8);
>>>>    return ret;
>>>>  }
>>>
>>> We already provide hp-timing.h, which uses rdtsc on x86 and clock_gettime on
>>> generic interface (and other high precision timing on other architectures).
>>> So I think a better way would be to:
>>
>> For x86/generic that works but other architectures also have hp-timing
>> implementations that might not be suitable for this (i.e there might be
>> an entropy regression).
> 
> The default hp-timing.h has
> 
> # define HP_TIMING_NOW(var) \
> ({ \
>   struct __timespec64 tv; \
>   __clock_gettime64 (CLOCK_MONOTONIC, &tv); \
>   (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec); \
> })
> 
> It isn't the same as the current include/random-bits.h.

Maybe refactor hp-timing.h to add a routine to get the system clock without
any adjustments?  I don't have a strong preference here, but I take that
you are not aiming to use RDRAND or similar instruction, since you are
optimizing to latency.  So I see that using hp-timing.h seems the best
approach, since it should work similar on different architectures (and
each one might disable if the entropy is not large enough).