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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Jul 2018 09:26:00 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     "Theodore Ts'o" <tytso@....edu>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jann Horn <jannh@...gle.com>, tcharding <me@...in.cc>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Eric Biggers <ebiggers@...gle.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] random: fix rdrand mix-in

On Tue, Jul 17, 2018 at 6:54 AM Arnd Bergmann <arnd@...db.de> 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_.

(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).

             Linus