Re: [PATCH] random: fix rdrand mix-in

[Arnd Bergmann]

On Tue, Jul 17, 2018 at 6:26 PM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Jul 17, 2018 at 6:54 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>>
>> The newly added arch_get_random_int() call was done incorrectly,
>> using the output only if rdrand hardware was /not/ available. The
>> compiler points out that the data is uninitialized in this case:
>
> Ack.
>
> Except:
>
>>                 for (b = bytes ; b > 0 ; b -= sizeof(__u32), i++) {
>> -                       if (arch_get_random_int(&t))
>> +                       if (!arch_get_random_int(&t))
>>                                 continue;
>>                         buf[i] ^= t;
>>                 }
>
> Why not just make that "continue" be a "break"? If you fail once, you
> will fail the next time too (whether the arch just doesn't support it
> at all, or whether the HW entropy is just temporarily exhausted).
>
> So no point in "continue". Just give up. Maybe it will work much
> later, but not _immediately_.

Makes sense. I found that a bit odd, but didn't think much of it:
on all architectures other than x86, arch_get_random_int()
will return a hardcoded 0 from an inline function, so the compiler
should be able to drop the entire loop either way.

On x86 however, it will keep evaluating arch_has_random()
pointlessly.

> (I don't actually see the commit in question - it's not in my pile of
> emails only in linux-next, maybe there's some reason further down
> prefers "continue" and the whole loop be finished).

I didn't see one.

      Arnd