Re: Inquiry about the removal of flag O_NONBLOCK on /dev/random

["Guozihua (Scott)" <guozihua@huawei.com>]

On 2022/7/21 12:07, Eric Biggers wrote:
> 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
> .

Hi Eric

We have a userspace program that starts pretty early in the boot process 
and it tries to fetch random bits from /dev/random with O_NONBLOCK, if 
that returns -EAGAIN, it turns to /dev/urandom. Is this a correct 
handling of -EAGAIN? Or this is not one of the intended use case of 
O_NONBLOCK?

-- 
Best
GUO Zihua