lkml.org 
[lkml]   [2022]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] random: allow writes to /dev/urandom to influence fast init
    On Tue, Mar 22, 2022 at 12:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
    >
    > @@ -1507,6 +1507,8 @@ static int write_pool(const char __user *ubuf, size_t count)
    > }
    > count -= len;
    > ubuf += len;
    > + if (unlikely(crng_init == 0 && !will_credit))
    > + crng_pre_init_inject(block, len, false);
    > mix_pool_bytes(block, len);
    > cond_resched();
    > }

    Ugh. I hate that whole crng_pre_init_inject() dance.

    We already mix the data into the input_pool with that 'mix_pool_bytes()' call.

    So what I think the real fix is, is to just make urandom_read() use
    the input_pool data directly for initializing the state.

    IOW, why isn't the patch along the lines of just making
    crng_make_state() take the data from the input pool instead, when
    crng_ready() isn't set?

    As a broken example patch, something like the appended (except that
    doesn't build, because 'input_pool' is declared later)?

    So take this purely as a conceptual patch, not a real patch.

    (Yeah, I think this also means that code that currently does that

    crng_pre_init_inject(pool, sizeof(pool), true);
    mix_pool_bytes(pool, sizeof(pool));

    should do those two operations in the reverse order, so that the input
    pool is always updated before that crng_pre_init_inject() dance).

    Maybe I'm missing something. But it seems kind of silly to use
    base_crng AT ALL before crng_ready(). Why not use the pool we have
    that *is* actually updated (that 'input_pool')?

    Linus

    @@ -374,19 +374,14 @@ static void crng_make_state(u32
    chacha_state[CHACHA_STATE_WORDS],
    /*
    * For the fast path, we check whether we're ready, unlocked first, and
    * then re-check once locked later. In the case where we're really not
    - * ready, we do fast key erasure with the base_crng directly, because
    - * this is what crng_pre_init_inject() mutates during early init.
    + * ready, we do fast key erasure with the input pool directly.
    */
    if (!crng_ready()) {
    - bool ready;
    -
    - spin_lock_irqsave(&base_crng.lock, flags);
    - ready = crng_ready();
    - if (!ready)
    - crng_fast_key_erasure(base_crng.key, chacha_state,
    - random_data, random_data_len);
    - spin_unlock_irqrestore(&base_crng.lock, flags);
    - if (!ready)
    + spin_lock_irqsave(&input_pool.lock, flags);
    + crng_fast_key_erasure(input_pool.key, chacha_state,
    + random_data, random_data_len);
    + spin_unlock_irqrestore(&input_pool.lock, flags);
    + if (!crng_ready())
    return;
    }

    \
     
     \ /
      Last update: 2022-03-22 21:43    [W:4.123 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site