Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 19 Jul 2022 13:01:56 +0200
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
To:     "Guozihua (Scott)" <guozihua@huawei.com>
Cc:     linux-crypto@vger.kernel.org, luto@kernel.org, tytso@mit.edu,
        ebiggers@kernel.org
Subject: Re: Inquiry about the removal of flag O_NONBLOCK on /dev/random
Message-ID: <YtaPJPkewin5uWdn@zx2c4.com>
References: <13e1fa9d-4df8-1a99-ca22-d9d655f2d023@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <13e1fa9d-4df8-1a99-ca22-d9d655f2d023@huawei.com>
Precedence: bulk

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;