Re: [RFC] Should writes to /dev/urandom immediately affect reads?

From: Eric Biggers
Date: Wed Sep 20 2023 - 15:32:52 EST


Hi Linus,

On Wed, Sep 20, 2023 at 11:48:26AM -0700, Linus Torvalds wrote:
> On Tue, 19 Sept 2023 at 23:06, Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> >
> > This would be the potential change, BTW:
>
> Entirely regardless of your fundamental question, no, that's not the
> potential change.
>
> That causes a crng_reseed() even if the write fails completely and
> returns -EFAULT.
>
> So at a *minimum*, I'd expect the patch to be be something like
>
> memzero_explicit(block, sizeof(block));
> - return ret ? ret : -EFAULT;
> + if (!ret)
> + return -EFAULT;
> + crng_reseed(NULL);
> + return ret;
>
> but even then I'd ask
>
> - wouldn't we want some kind of minimum check?
>
> - do we really trust writes to add any actual entropy at all and at what point?
>
> which are admittedly likely the same question just in different guises.

Whether to credit entropy for writes to /dev/{u,}random is an unrelated topic,
and the answer is clearly "no, we must not, and we never have" (as I mentioned
in the second paragraph of my email). I understand the last discussion
https://lore.kernel.org/lkml/20220322191436.110963-1-Jason@xxxxxxxxx/T/#u
diverged into both topics, but they're not directly related. Reseeding the CRNG
just makes it up to date with the entropy pool; nothing more than that.

Yes, obviously there's no point in reseeding if nothing actually got added, so
we could skip the reseed in that case if we want to.

>
> Also, are there any relevant architectures where
> "try_to_generate_entropy()" doesn't work? IOW, why do you even care?
>

There are, as shown by the fact that the full unification of /dev/urandom and
/dev/random failed yet again. But similarly, that's unrelated. The actual
question, which I'm attempting to start a discussion about without getting
sidetracked into questions that may seem related but actually aren't, is simply
whether writes to /dev/{u,}random should immediately affect reads.

- Eric