linux-ext4 - Re: Linux 5.3-rc8

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, 10 Sep 2019 19:21:54 +0100
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Ahmed S. Darwish" <darwish.07@...il.com>
Cc:     "Theodore Ts'o" <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Jan Kara <jack@...e.cz>, Ray Strode <rstrode@...hat.com>,
        William Jon McCann <mccann@....edu>,
        zhangjs <zachary@...shancloud.com>, linux-ext4@...r.kernel.org,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: Linux 5.3-rc8

On Tue, Sep 10, 2019 at 6:33 PM Ahmed S. Darwish <darwish.07@...il.com> wrote:
>
> While gnome-session is obviously at fault here by requiring
> *blocking* randomness at the boot path, it's still not requesting
> much, just (5 * 16) bytes to be exact.
>
> I guess an x86 laptop should be able to provide that, even without
> RDRAND / random.trust_cpu=on (TSC jitter, etc.) ?

Yeah, the problem is partly because we can't trust "get_cycles()"
because not all architectures have it. So we use "jiffies" for the
entropy estimation, and my guess is that it just ends up estimating
you have little to no entropy from your disk IO.

So the timestamp counter value is added to the randomness pool, but
the jitter in the TSC values isn't then used to estimate the entropy
at all.

Just out of curiosity, what happens if you apply a patch like this
(intentionally whitespace-damaged, I don't want anybody to pick it up
without thinking about it) thing:

   diff --git a/drivers/char/random.c b/drivers/char/random.c
   index 5d5ea4ce1442..60709a7b4af1 100644
   --- a/drivers/char/random.c
   +++ b/drivers/char/random.c
   @@ -1223,6 +1223,7 @@ static void add_timer_randomness(struct
timer_rand_state *state, unsigned $
         * We take into account the first, second and third-order deltas
         * in order to make our estimate.
         */
   +    sample.jiffies += sample.cycles;
        delta = sample.jiffies - state->last_time;
        state->last_time = sample.jiffies;

which just makes the entropy estimation use the _sum_ of jiffies and
cycles as the base. On architectures that don't have a cycle counter,
it ends up being the same it used to be (just jiffies), and on
architectures that do have a timestamp counter the TSC differences
will overwhelm the jiffies differences, so you end up effectively
using the third-order TSC difference as the entropy estimation.

Which I think is what the code really wants - it's only using jiffies
because that is the only thing _guaranteed_ to change at all. But with
the sum, you get the best of both worlds, and should basically make
the entropy estimation use the "better of two counters".

Ted, comments? I'd hate to revert the ext4 thing just because it
happens to expose a bad thing in user space.

              Linus