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

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

On 29/03/2022 16:56, Noah Goldstein 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).

I would expect that the entropy of the hp-timing.h instruction would be similar
to the ones from system clock (which exception of legacy architecture like alpha),
but I haven't checked yet.

> 
>>
>>   static inline uint32_t
>>   random_bits (void)
>>   {
>>     hp_timing_t hp;
>>     HP_TIMING_NOW (hp);
>>     /* Shuffle the lower bits to minimize the clock bias.  */
>>     uint32_t ret = hp >> 32 ^ (uint32_t) hp;
>>     ret ^= (ret << 24) | (ret >> 8);
>>     return ret;
>>   }
>>
>> And keep the XOR on with higher bits to keep the clock bias.
>>
>>> diff --git a/sysdeps/generic/random-bits-entropy.h b/sysdeps/generic/random-bits-entropy.h
>>> new file mode 100644
>>> index 0000000000..53290c7f7a
>>> --- /dev/null
>>> +++ b/sysdeps/generic/random-bits-entropy.h
>>> @@ -0,0 +1,31 @@
>>> +/* Fast function for generating entropy of random_bits.
>>> +   Copyright (C) 2022 Free Software Foundation, Inc.
>>> +   This file is part of the GNU C Library.
>>> +
>>> +   The GNU C Library is free software; you can redistribute it and/or
>>> +   modify it under the terms of the GNU Lesser General Public
>>> +   License as published by the Free Software Foundation; either
>>> +   version 2.1 of the License, or (at your option) any later version.
>>> +
>>> +   The GNU C Library is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> +   Lesser General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU Lesser General Public
>>> +   License along with the GNU C Library; if not, see
>>> +   <https://www.gnu.org/licenses/>.  */
>>> +
>>> +#include <stdint.h>
>>> +#include <time.h>
>>> +
>>> +/* Generically use 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.  */
>>> +static inline uint32_t
>>> +random_bits_entropy (void)
>>> +{
>>> +  struct __timespec64 tv;
>>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>>> +  return tv.tv_nsec ^ tv.tv_sec;
>>> +}