Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 28 Jan 2015 16:41:28 +0100
From: Daniel Cegiełka <daniel.cegielka@...il.com>
To: musl@...ts.openwall.com
Subject: Re: getrandom syscall

2015-01-28 15:54 GMT+01:00 Rich Felker <dalias@...c.org>:
> On Tue, Jan 27, 2015 at 11:12:46PM +0100, Daniel Cegiełka wrote:
>> best regards,
>> Daniel
>
> Thanks. I've been wanting to get this added as well as a getentropy
> function (the other API for the same thing).

Here is a Theodore Tso version:

https://lkml.org/lkml/2014/7/17/474

+wrapper for getentropy():

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c6e9d6f38894798696f23c8084ca7edbf16ee895

>
>> #include <stddef.h>
>> #include <errno.h>
>> #include "syscall.h"
>>
>> int getrandom(void *buf, size_t len)
>> {
>>       int ret, pre_errno = errno;
>
> There's no need to save/restore errno here. errno is only meaningful
> after a function returns an error code. On success it should not be
> inspected and could contain junk.
>
>>       if (len > 256) {
>>               errno = EIO;
>>               return -1;
>>       }
>
> Could you explain the motivation for this part?

If you request a large number of bytes you’re going to wait while they
are being computed. It make no sense, so getrandom returns an error if
the requested is over 256 bytes (__it's worth it thoroughly
investigate__).

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c6e9d6f38894798696f23c8084ca7edbf16ee895

"""
Theodore Tso 2014-07-23 15:11:06 UTC

The bug should be fixed if you cherry-pick commit
79a8468747c5f95ed3d5ce8376a3e82e0c5857fc into 3.15.4+.   Please let me
know if it doesn't.

Why on *earth* are you using /dev/urandom in this way?   The nbytes
restriction is to prevent an accounting failure since we are now
tracking entropy in fractional bits, so when we convert bytes
requested into fractional bits, we overflow if someone tries to
request more than 32MB.  Given that no sane use of /dev/urandom needs
more than 256 bytes, this was considered acceptable.
"""

https://bugzilla.kernel.org/show_bug.cgi?id=80981

but I don't have certainty about this.


>>       do {
>>               ret = syscall(SYS_getrandom, buf, len, 0);
>>       } while (ret == -1 && errno == EINTR);
>
> This would be more efficient (and avoid your errno issue entirely) if
> you use __syscall which returns -errcode rather than storing errcode
> in errno. It allows the whole loop to be inlined with no function
> call. Something like:
>
>         while ((ret = __syscall(SYS_getrandom, buf, len, 0)) == -EINTR);
>
> Of course there's the question of whether it should loop on EINTR
> anyway; I don't know. Also if this can block there's the question of
> whether it should be cancellable, but that can be decided later.
>
> Finally, I wonder if it would make sense to use other fallbacks in the
> case where the syscall is not supported -- perhaps the aux vector
> AT_RANDOM or even /dev/urandom? (But I'd rather avoid doing anything
> that depends on fds, which would make a function that appears to
> always-work but actually fails on resource exhaustion.)

Thanks,
Daniel

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.