[PATCH 2/2] Use getrandom on try_tempname_len [BZ #15813]

[Adhemerval Zanella]

On 10/09/2020 18:21, Paul Eggert wrote:
>>> As I wrote in bugzilla, I think it would be better to use clock_gettime64 ^
>>> pid based "random" source for the initial randomness value, so that it
>>> wouldn't deplete the random entropy pool, and use it only for the retries
>>> (so only in the unlikely case the file exists already).
> 
> Isn't part of the goal to avoid collisions even in the first try, to avoid attacks by name-guessers on not-so-well-written callers? If so, we should use getrandom even for the first try (with GRND_NONBLOCK of course).
> 
> Generating a file name ought to be a reasonably-rare action, and I wouldn't worry too much about entropy pool exhaustion from such a small request.

I don't have a strong opinion here, but I see that using GRND_NONBLOCK
with current glibc code results in a slight simples code with better
guaranties specially on recent kernels.  I will send an updated version
using gnulib code with GRND_NONBLOCK change.

> 
>> +  *var = ((uintptr_t) var) / alignof (max_align_t);
>> +#if _LIBC || (defined CLOCK_MONOTONIC && HAVE_CLOCK_GETTIME)
>> +  struct __timespec64 tv;
>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>> +  *var ^= tv.tv_nsec;
>>   #endif
>> +  *var += (uintptr_t) tmpl;
> 
> This should also use ^=.> 
>> +#if _LIBC || (defined CLOCK_MONOTONIC && HAVE_CLOCK_GETTIME)
>> +  /* Add entropy if getrandom is not supported.  */
>> +  struct __timespec64 tv;
>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>> +  var ^= tv.tv_nsec;
>> +#endif
> 
> This is duplicate code and should be refactored out, assuming you don't follow my suggestion above (which should remove the code duplication anyway).> 
>> +  /* A random variable.  Try to get some entropy to avoid call random_bits
>> +     (which might be expensive).  */
> 
> I don't quite follow the comment, which has grammar problems with that "avoid call"