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

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Tue, Mar 29, 2022 at 3:44 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> 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).

That would work. But we would still need to hardcode the random-bits needs
in some cases.

For example in generic we would still need to combine seconds and nanoseconds
and ideally wouldn't use multiply for that.

At that point it seems we are doing something logically different
enough it would
make more sense to just include <hp-timing.h> in <random-bits-entropy.h> if
it was appropriate for the architecture.