On Thu, Jul 21, 2022 at 11:50:27AM +0800, Guozihua (Scott) wrote:
> On 2022/7/19 19:01, Jason A. Donenfeld wrote:
> > Hi,
> >
> > On Thu, Jul 14, 2022 at 03:33:47PM +0800, Guozihua (Scott) wrote:
> > > Recently we noticed the removal of flag O_NONBLOCK on /dev/random by
> > > commit 30c08efec888 ("random: make /dev/random be almost like
> > > /dev/urandom"), it seems that some of the open_source packages e.g.
> > > random_get_fd() of util-linux and __getrandom() of glibc. The man page
> > > for random() is not updated either.
> > >
> > > Would anyone please kindly provide some background knowledge of this
> > > flag and it's removal? Thanks!
> >
> > I didn't write that code, but I assume it was done this way because it
> > doesn't really matter that much now, as /dev/random never blocks after
> > the RNG is seeded. And now a days, the RNG gets seeded with jitter
> > fairly quickly as a last resort, so almost nobody waits a long time.
> >
> > Looking at the two examples you mentioned, the one in util-linux does
> > that if /dev/urandom fails, which means it's mostly unused code, and the
> > one in glibc is for GNU Hurd, not Linux. I did a global code search and
> > found a bunch of other instances pretty similar to the util-linux case,
> > where /dev/random in O_NONBLOCK mode is used as a fallback to
> > /dev/urandom, which means it's basically never used. (Amusingly one such
> > user of this pattern is Ted's pwgen code from way back at the turn of
> > the millennium.)
> >
> > All together, I couldn't really find anywhere that the removal of
> > O_NONBLOCK semantics would actually pose a problem for, especially since
> > /dev/random doesn't block at all after being initialized. So I'm
> > slightly leaning toward the "doesn't matter, do nothing" course of
> > action.
> >
> > But on the other hand, you did somehow notice this, so that's important
> > perhaps. How did you run into it? *Does* it actually pose a problem? Or
> > was this a mostly theoretical finding from perusing source code?
> > Something like the below diff would probably work and isn't too
> > invasive, but I think I'd prefer to leave it be unless this really did
> > break some userspace of yours. So please let me know.
> >
> > Regards,
> > Jason
> >
> > diff --git a/drivers/char/random.c b/drivers/char/random.c
> > index 70d8d1d7e2d7..6f232ac258bf 100644
> > --- a/drivers/char/random.c
> > +++ b/drivers/char/random.c
> > @@ -1347,6 +1347,10 @@ static ssize_t random_read_iter(struct kiocb *kiocb, struct iov_iter *iter)
> > {
> > int ret;
> > + if (!crng_ready() &&
> > + ((kiocb->ki_flags & IOCB_NOWAIT) || (kiocb->ki_filp->f_flags & O_NONBLOCK)))
> > + return -EAGAIN;
> > +
> > ret = wait_for_random_bytes();
> > if (ret != 0)
> > return ret;
> >
> > .
>
> Hi Jason, Thanks for the respond.
>
> The reason this comes to me is that we have an environment that is super
> clean with very limited random events and with very limited random hardware
> access. It would take up to 80 minutes before /dev/random is fully
> initialized. I think it would be great if we can restore the O_NONBLOCK
> flag.
>
> Would you mind merge this change into mainstream or may I have the honor?
>
Can you elaborate on how this change would actually solve a problem for you? Do
you actually have a program that is using /dev/random with O_NONBLOCK, and that
handles the EAGAIN error correctly? Just because you're seeing a program wait
for the RNG to be initialized doesn't necessarily mean that this change would
make a difference, as the program could just be reading from /dev/random without
O_NONBLOCK or calling getrandom() without GRND_NONBLOCK. The behavior of those
(more common) cases would be unchanged by Jason's proposed change.
- Eric
