| Message ID | 3dd5d3b8-c196-98fb-1671-90cd90ab90c7@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11/14/2016 12:44 PM, Florian Weimer wrote: > This patch switches back to the ssize_t return time. This goes against > Theodore Ts'o preference, but seems to reflect the consensus from the > largery community. I still don't think this function should be a cancellation point. > I weakened the protection against accidental interposition somewhat. > Since the declaration is a new header file, I do not use a function-like > macro to prevent a definition of a function named “getrandom”, and > non-GNU compilers do not get any redirection. We don't normally do this at all. I don't understand why this function should be treated differently. Can you please explain what concrete situations, involving real, existing code, you're trying to defend against here? zw
On 11/14/2016 07:29 PM, Zack Weinberg wrote: > On 11/14/2016 12:44 PM, Florian Weimer wrote: >> I weakened the protection against accidental interposition somewhat. >> Since the declaration is a new header file, I do not use a function-like >> macro to prevent a definition of a function named “getrandom”, and >> non-GNU compilers do not get any redirection. > > We don't normally do this at all. I don't understand why this function > should be treated differently. Can you please explain what concrete > situations, involving real, existing code, you're trying to defend > against here? It has been proposed that we do this for all new non-standardized symbols. Which does make sense from a certain point of view. As far as I can see it can't hurt, and can only help when it comes to inter-operating with user binaries that could have legitimately used the symbols until now. r~
On 11/14/2016 07:29 PM, Zack Weinberg wrote: > On 11/14/2016 12:44 PM, Florian Weimer wrote: >> This patch switches back to the ssize_t return time. This goes against >> Theodore Ts'o preference, but seems to reflect the consensus from the >> largery community. > > I still don't think this function should be a cancellation point. I guess we'll have to agree to disagree on this matter. >> I weakened the protection against accidental interposition somewhat. >> Since the declaration is a new header file, I do not use a function-like >> macro to prevent a definition of a function named “getrandom”, and >> non-GNU compilers do not get any redirection. > > We don't normally do this at all. See the “Evolution of ELF symbol management” thread. We do it all the time, for the benefit of non-libc DSOs in the glibc conglomerate. I think there is broad consensus that we need to extend this to libstdc++ at least (in addition to changes needed to enable C++ compilation without _GNU_SOURCE). And once we are at C++, why stop there? Even dynamic languages with a C extension framework would use this. Thanks, Florian
On 11/16/2016 10:11 AM, Florian Weimer wrote: > On 11/14/2016 07:29 PM, Zack Weinberg wrote: >> On 11/14/2016 12:44 PM, Florian Weimer wrote: >>> This patch switches back to the ssize_t return time. This goes against >>> Theodore Ts'o preference, but seems to reflect the consensus from the >>> largery community. >> >> I still don't think this function should be a cancellation point. > > I guess we'll have to agree to disagree on this matter. I am seriously considering escalating my disagreement here to a formal objection. I would like to know why you think it is NECESSARY - not merely convenient or consistent with other stuff - for this function to be a cancellation point. (My basic attitude is that adding new cancellation points is always the Wrong Thing, and should only be done when _unavoidable_; and in this particular case it is especially bad since applications are probably going to assume that this function never fails, blocks, or even writes fewer bytes than requested to the buffer, no matter how clearly we say that it might.) >> We don't normally do this at all. > > See the “Evolution of ELF symbol management” thread. We do it all the > time, for the benefit of non-libc DSOs in the glibc conglomerate. I > think there is broad consensus that we need to extend this to libstdc++ > at least (in addition to changes needed to enable C++ compilation > without _GNU_SOURCE). And once we are at C++, why stop there? Even > dynamic languages with a C extension framework would use this. See the reply I'm about to post in that thread. zw
On 11/16/2016 04:20 PM, Zack Weinberg wrote: > On 11/16/2016 10:11 AM, Florian Weimer wrote: >> On 11/14/2016 07:29 PM, Zack Weinberg wrote: >>> On 11/14/2016 12:44 PM, Florian Weimer wrote: >>>> This patch switches back to the ssize_t return time. This goes against >>>> Theodore Ts'o preference, but seems to reflect the consensus from the >>>> largery community. >>> >>> I still don't think this function should be a cancellation point. >> >> I guess we'll have to agree to disagree on this matter. > > I am seriously considering escalating my disagreement here to a formal > objection. I would like to know why you think it is NECESSARY - not > merely convenient or consistent with other stuff - for this function to > be a cancellation point. It's necessary if you ever want to cancel a hanging getrandom in a context where you cannot install a signal handler (so that you can trigger EINTR when getrandom is stuck). I really don't understand why cancellation points are widely considered as evil. Most code does not use cancellation in a correct way, and it will not improve if we simply stop adding new cancellation points. Furthermore, it's easy to make a cancellation point go away (just switch the cancel state around the call), but at least in library code, it is impossible to introduce cancellation into a system call where the wrapper does not support it (because you cannot fake your own version of cancellation with a do-nothing signal handler). Thanks, Florian
On Wed, Nov 16, 2016 at 10:52 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 11/16/2016 04:20 PM, Zack Weinberg wrote: >> I am seriously considering escalating my disagreement here to a >> formal objection. I would like to know why you think it is >> NECESSARY - not merely convenient or consistent with other stuff - >> for this function to be a cancellation point. > > It's necessary if you ever want to cancel a hanging getrandom in a > context where you cannot install a signal handler (so that you can > trigger EINTR when getrandom is stuck). That only pushes the question back a level. When would it ever be necessary to do that? Be as concrete as you possibly can. Actual code from a real program, if possible. > I really don't understand why cancellation points are widely > considered as evil. Most code does not use cancellation in a > correct way, and it will not improve if we simply stop adding new > cancellation points. I would argue that most code does not use cancellation correctly in large part _because_ the set of cancellation points is so large and amorphous. It is the same problem that people have with exceptions in C++; because practically everything you might do exposes you to cancellation, it's so difficult to know how to write cancel-safe code that people just apply the big hammer of not using it at all. Another useful example is multithreading versus coroutines with explicit yield points---the latter can be less efficient and/or more verbose, but it's so much easier to debug that it's worth it. There's not a lot we can do about the cancellation-point set in POSIX being so large, but we can at least not make things worse, by not adding additional cancellation points. > at least in library code, it is impossible to introduce cancellation > into a system call where the wrapper does not support it (because > you cannot fake your own version of cancellation with a do-nothing > signal handler). From the perspective that as few operations as possible should be cancellation points, this is a Good Thing. And I don't see why it would be a problem for getrandom in particular. zw
On Wed, 2016-11-16 at 16:52 +0100, Florian Weimer wrote: > On 11/16/2016 04:20 PM, Zack Weinberg wrote: > > On 11/16/2016 10:11 AM, Florian Weimer wrote: > >> On 11/14/2016 07:29 PM, Zack Weinberg wrote: > >>> On 11/14/2016 12:44 PM, Florian Weimer wrote: > >>>> This patch switches back to the ssize_t return time. This goes against > >>>> Theodore Ts'o preference, but seems to reflect the consensus from the > >>>> largery community. > >>> > >>> I still don't think this function should be a cancellation point. > >> > >> I guess we'll have to agree to disagree on this matter. > > > > I am seriously considering escalating my disagreement here to a formal > > objection. I would like to know why you think it is NECESSARY - not > > merely convenient or consistent with other stuff - for this function to > > be a cancellation point. > > It's necessary if you ever want to cancel a hanging getrandom in a > context where you cannot install a signal handler (so that you can > trigger EINTR when getrandom is stuck). I think it would be better to split the getrandom that is a cancellation point into two functions, getrandom (not a cancellation point) and getrandom_cancelable (is a cancellation point). This way, the functionality is available for programs but requires explicit opt-in, while the default is not going to lead to surprises in the expected common case (ie, when randomness is available). I don't think the opt-in is a problem because as you said, cancellation requires careful programming anyway.
On 16/11/2016 16:02, Torvald Riegel wrote: > On Wed, 2016-11-16 at 16:52 +0100, Florian Weimer wrote: >> On 11/16/2016 04:20 PM, Zack Weinberg wrote: >>> On 11/16/2016 10:11 AM, Florian Weimer wrote: >>>> On 11/14/2016 07:29 PM, Zack Weinberg wrote: >>>>> On 11/14/2016 12:44 PM, Florian Weimer wrote: >>>>>> This patch switches back to the ssize_t return time. This goes against >>>>>> Theodore Ts'o preference, but seems to reflect the consensus from the >>>>>> largery community. >>>>> >>>>> I still don't think this function should be a cancellation point. >>>> >>>> I guess we'll have to agree to disagree on this matter. >>> >>> I am seriously considering escalating my disagreement here to a formal >>> objection. I would like to know why you think it is NECESSARY - not >>> merely convenient or consistent with other stuff - for this function to >>> be a cancellation point. >> >> It's necessary if you ever want to cancel a hanging getrandom in a >> context where you cannot install a signal handler (so that you can >> trigger EINTR when getrandom is stuck). > > I think it would be better to split the getrandom that is a cancellation > point into two functions, getrandom (not a cancellation point) and > getrandom_cancelable (is a cancellation point). This way, the > functionality is available for programs but requires explicit opt-in, > while the default is not going to lead to surprises in the expected > common case (ie, when randomness is available). I don't think the > opt-in is a problem because as you said, cancellation requires careful > programming anyway. > I would advise against adding another symbol, it only adds complexity and most likely one interface would be preferable from application point of view. Considering portability, why not following current approach from other OS/libc? User will probably create more highly level interfaces based underlying facilities, so I see trying to follow current interface semantics in a non standard interface seems a better approach. For instance, OpenBSD getentropy seems a to just direct syscall, so not cancellation handling (src/libexec/ld.so/<architecture>/ldasm.S). FreeBSD does not have a similar syscall, but provides a sysctl/KERN_ARND that does basically the same. I am not sure about Solaris.
On 14 Nov 2016 18:44, Florian Weimer wrote: just nits at this point > +/* Flags for use with getrandom. */ > +#define GRND_NONBLOCK 1 > +#define GRND_RANDOM 2 if they're bit flags, should we be doing 0x1/0x2 etc ? otherwise this will turn into 4, 8, 16, 32, 64, etc... which gets ugly. the kernel headers use hex constants. > +/* Test getrandom with a single buffer length. */ > +static void > +test_length (char *buffer, size_t length, unsigned int flags) > +{ > + memset (buffer, 0, length); > + strcpy (buffer + length, "123"); while this works, it seems pointlessly fragile. can't you treat it like a normal "this is the length of the buffer" and carve out space at the end yourself ? i.e. memset (buffer, 0, length); static const char canary[] = "123"; size_t canary_len = sizeof(canary); length -= canary_len; strcpy (buffer + length, canary); ... ssize_t ret = getrandom (buffer, length - , flags); > + if (ret < 0) > + { > + if (!((flags & GRND_RANDOM) > + && (flags & GRND_NONBLOCK) > + && errno != EAGAIN)) seems like it'd be more readable to distribute the ! and to combine the flags check into a single mask ? i have to read these lines a few times to digest what exactly the code is trying to do. > + if (getrandom_full (buffer1, sizeof (buffer1), flags) > + && getrandom_full (buffer2, sizeof (buffer2), flags)) > + { > + if (memcmp (buffer1, buffer2, sizeof (buffer1)) == 0) maybe also add a comment that likelihood of this being the same is extremely rare too. return 77; > + > + for (int use_random = 0; use_random < 2; ++use_random) > + for (int use_nonblock = 0; use_nonblock < 2; ++use_nonblock) > + { > + int flags = 0; unsigned to match the API ? -mike > + if (use_random) > + flags |= GRND_RANDOM; > + if (use_nonblock) > + flags |= GRND_NONBLOCK; > + test_flags (flags); > + } > + return errors; > +} > + > +#define TEST_FUNCTION do_test () > +#include "../test-skeleton.c" > diff --git a/sysdeps/arm/nacl/libc.abilist b/sysdeps/arm/nacl/libc.abilist > index 807e43d..f52e7e7 100644 > --- a/sysdeps/arm/nacl/libc.abilist > +++ b/sysdeps/arm/nacl/libc.abilist > @@ -1843,6 +1843,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 gnu_dev_major F > GLIBC_2.25 gnu_dev_makedev F > GLIBC_2.25 gnu_dev_minor F > diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list > index 2254c76..79483ea 100644 > --- a/sysdeps/unix/syscalls.list > +++ b/sysdeps/unix/syscalls.list > @@ -100,3 +100,4 @@ utimes - utimes i:sp __utimes utimes > vhangup - vhangup i:i vhangup > write - write Ci:ibn __libc_write __write write > writev - writev Ci:ipi __writev writev > +getrandom - getrandom Ci:bni __libc_getrandom getrandom > diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist > index 77accdf..77a2231 100644 > --- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist > @@ -2090,6 +2090,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist > index 659b7fc..922e7c3 100644 > --- a/sysdeps/unix/sysv/linux/alpha/libc.abilist > +++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist > @@ -2001,6 +2001,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/arm/libc.abilist b/sysdeps/unix/sysv/linux/arm/libc.abilist > index 8bc979a..7831eb2 100644 > --- a/sysdeps/unix/sysv/linux/arm/libc.abilist > +++ b/sysdeps/unix/sysv/linux/arm/libc.abilist > @@ -91,6 +91,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist > index 299b705..f6623b7 100644 > --- a/sysdeps/unix/sysv/linux/hppa/libc.abilist > +++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist > @@ -1855,6 +1855,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist > index f00345f..ef04a40 100644 > --- a/sysdeps/unix/sysv/linux/i386/libc.abilist > +++ b/sysdeps/unix/sysv/linux/i386/libc.abilist > @@ -2013,6 +2013,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist > index e5fcf88..37dde9d 100644 > --- a/sysdeps/unix/sysv/linux/ia64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist > @@ -1877,6 +1877,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist > index 8f382f6..b236ba8 100644 > --- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist > +++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist > @@ -92,6 +92,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist > index 320b7fe..0983296 100644 > --- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist > +++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist > @@ -1969,6 +1969,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/microblaze/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/libc.abilist > index 21b1426..6cd5093 100644 > --- a/sysdeps/unix/sysv/linux/microblaze/libc.abilist > +++ b/sysdeps/unix/sysv/linux/microblaze/libc.abilist > @@ -2090,6 +2090,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist > index 5c4b596..67c0ce0 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist > +++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist > @@ -1944,6 +1944,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist > index 001fa6c..88b9f5f 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist > +++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist > @@ -1942,6 +1942,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist > index 2d87001..b2bfc81 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist > @@ -1940,6 +1940,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist > index aa1ee66..2cb3e46 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist > @@ -1935,6 +1935,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist > index 2471d68..bd3db24 100644 > --- a/sysdeps/unix/sysv/linux/nios2/libc.abilist > +++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist > @@ -2131,6 +2131,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist > index 4b0cde8..317c8ba 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist > @@ -1973,6 +1973,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist > index 0557c16..0774b80 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist > @@ -1978,6 +1978,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist > index 821384e..174f8b2 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist > @@ -2178,6 +2178,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist > index c40a3f1..b364cae 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist > @@ -92,6 +92,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist > index 5b39a60..66168b9 100644 > --- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist > @@ -1973,6 +1973,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist > index a9db32f..0e99054 100644 > --- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist > @@ -1874,6 +1874,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/sh/libc.abilist b/sysdeps/unix/sysv/linux/sh/libc.abilist > index 294af0a..c47d2ac 100644 > --- a/sysdeps/unix/sysv/linux/sh/libc.abilist > +++ b/sysdeps/unix/sysv/linux/sh/libc.abilist > @@ -1859,6 +1859,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist > index 32747bd..923e598 100644 > --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist > @@ -1965,6 +1965,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist > index b0ac4d4..836dabb 100644 > --- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist > @@ -1903,6 +1903,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist > index 4d92d81..2f7d425 100644 > --- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist > @@ -2097,6 +2097,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist > index a68aef7..5a240a4 100644 > --- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist > @@ -2097,6 +2097,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist > index 4d92d81..2f7d425 100644 > --- a/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist > +++ b/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist > @@ -2097,6 +2097,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist > index b8623fc..aa57670 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist > @@ -1854,6 +1854,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist > index a61d874..1a7bcea 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist > @@ -2097,6 +2097,8 @@ GLIBC_2.23 fts64_set F > GLIBC_2.24 GLIBC_2.24 A > GLIBC_2.24 quick_exit F > GLIBC_2.25 GLIBC_2.25 A > +GLIBC_2.25 __libc_getrandom F > +GLIBC_2.25 getrandom F > GLIBC_2.25 strfromd F > GLIBC_2.25 strfromf F > GLIBC_2.25 strfroml F
On Wed, 2016-11-16 at 17:52 -0200, Adhemerval Zanella wrote: > > On 16/11/2016 16:02, Torvald Riegel wrote: > > On Wed, 2016-11-16 at 16:52 +0100, Florian Weimer wrote: > >> On 11/16/2016 04:20 PM, Zack Weinberg wrote: > >>> On 11/16/2016 10:11 AM, Florian Weimer wrote: > >>>> On 11/14/2016 07:29 PM, Zack Weinberg wrote: > >>>>> On 11/14/2016 12:44 PM, Florian Weimer wrote: > >>>>>> This patch switches back to the ssize_t return time. This goes against > >>>>>> Theodore Ts'o preference, but seems to reflect the consensus from the > >>>>>> largery community. > >>>>> > >>>>> I still don't think this function should be a cancellation point. > >>>> > >>>> I guess we'll have to agree to disagree on this matter. > >>> > >>> I am seriously considering escalating my disagreement here to a formal > >>> objection. I would like to know why you think it is NECESSARY - not > >>> merely convenient or consistent with other stuff - for this function to > >>> be a cancellation point. > >> > >> It's necessary if you ever want to cancel a hanging getrandom in a > >> context where you cannot install a signal handler (so that you can > >> trigger EINTR when getrandom is stuck). > > > > I think it would be better to split the getrandom that is a cancellation > > point into two functions, getrandom (not a cancellation point) and > > getrandom_cancelable (is a cancellation point). This way, the > > functionality is available for programs but requires explicit opt-in, > > while the default is not going to lead to surprises in the expected > > common case (ie, when randomness is available). I don't think the > > opt-in is a problem because as you said, cancellation requires careful > > programming anyway. > > > > I would advise against adding another symbol, it only adds complexity > and most likely one interface would be preferable from application > point of view. We could add a parameter too that determines whether it is a cancellation point. That avoids your concern about adding another symbol, and programmers will still have to make a conscious decision about cancellation. I don't see how this adds complexity; the complexity in there is having to choose between the different semantics, or be aware that it's just one of them when we know that just one of them is not a one-size-fits-all for all use cases.
On 11/16/2016 05:41 PM, Zack Weinberg wrote: > On Wed, Nov 16, 2016 at 10:52 AM, Florian Weimer <fweimer@redhat.com> wrote: >> On 11/16/2016 04:20 PM, Zack Weinberg wrote: >>> I am seriously considering escalating my disagreement here to a >>> formal objection. I would like to know why you think it is >>> NECESSARY - not merely convenient or consistent with other stuff - >>> for this function to be a cancellation point. >> >> It's necessary if you ever want to cancel a hanging getrandom in a >> context where you cannot install a signal handler (so that you can >> trigger EINTR when getrandom is stuck). > > That only pushes the question back a level. When would it ever be > necessary to do that? Be as concrete as you possibly can. Actual > code from a real program, if possible. It's not clear to me what you are asking here. Do you mean cancellation in general, or cancellation in conjunction with getrandom specifically? Thanks, Florian
On 11/17/2016 08:02 AM, Florian Weimer wrote: > On 11/16/2016 05:41 PM, Zack Weinberg wrote: >> On Wed, Nov 16, 2016 at 10:52 AM, Florian Weimer <fweimer@redhat.com> >> wrote: >>> On 11/16/2016 04:20 PM, Zack Weinberg wrote: >>>> I am seriously considering escalating my disagreement here to a >>>> formal objection. I would like to know why you think it is >>>> NECESSARY - not merely convenient or consistent with other stuff - >>>> for this function to be a cancellation point. >>> >>> It's necessary if you ever want to cancel a hanging getrandom in a >>> context where you cannot install a signal handler (so that you can >>> trigger EINTR when getrandom is stuck). >> >> That only pushes the question back a level. When would it ever be >> necessary to do that? Be as concrete as you possibly can. Actual >> code from a real program, if possible. > > It's not clear to me what you are asking here. > > Do you mean cancellation in general, or cancellation in conjunction with > getrandom specifically? Sorry. I meant cancellation specifically of a thread hanging in getrandom. zw
On 11/17/2016 02:45 PM, Zack Weinberg wrote: > On 11/17/2016 08:02 AM, Florian Weimer wrote: >> On 11/16/2016 05:41 PM, Zack Weinberg wrote: >>> On Wed, Nov 16, 2016 at 10:52 AM, Florian Weimer <fweimer@redhat.com> >>> wrote: >>>> On 11/16/2016 04:20 PM, Zack Weinberg wrote: >>>>> I am seriously considering escalating my disagreement here to a >>>>> formal objection. I would like to know why you think it is >>>>> NECESSARY - not merely convenient or consistent with other stuff - >>>>> for this function to be a cancellation point. >>>> >>>> It's necessary if you ever want to cancel a hanging getrandom in a >>>> context where you cannot install a signal handler (so that you can >>>> trigger EINTR when getrandom is stuck). >>> >>> That only pushes the question back a level. When would it ever be >>> necessary to do that? Be as concrete as you possibly can. Actual >>> code from a real program, if possible. >> >> It's not clear to me what you are asking here. >> >> Do you mean cancellation in general, or cancellation in conjunction with >> getrandom specifically? > > Sorry. I meant cancellation specifically of a thread hanging in getrandom. I'm not sure how I can provide that, considering that there is currently no way to cancel a thread which hangs in getrandom because we do not provide a way for applications to implement system calls as cancellation points (unless we provide a wrapper for the specific system call, of course). Florian
On 11/17/2016 08:50 AM, Florian Weimer wrote: > On 11/17/2016 02:45 PM, Zack Weinberg wrote: >> On 11/17/2016 08:02 AM, Florian Weimer wrote: >>> On 11/16/2016 05:41 PM, Zack Weinberg wrote: >>>> On Wed, Nov 16, 2016 at 10:52 AM, Florian Weimer <fweimer@redhat.com> >>>> wrote: >>>>> On 11/16/2016 04:20 PM, Zack Weinberg wrote: >>>>>> I am seriously considering escalating my disagreement here to a >>>>>> formal objection. I would like to know why you think it is >>>>>> NECESSARY - not merely convenient or consistent with other stuff - >>>>>> for this function to be a cancellation point. >>>>> >>>>> It's necessary if you ever want to cancel a hanging getrandom in a >>>>> context where you cannot install a signal handler (so that you can >>>>> trigger EINTR when getrandom is stuck). >>>> >>>> That only pushes the question back a level. When would it ever be >>>> necessary to do that? Be as concrete as you possibly can. Actual >>>> code from a real program, if possible. >>> >>> It's not clear to me what you are asking here. >>> >>> Do you mean cancellation in general, or cancellation in conjunction with >>> getrandom specifically? >> >> Sorry. I meant cancellation specifically of a thread hanging in >> getrandom. > > I'm not sure how I can provide that, considering that there is currently > no way to cancel a thread which hangs in getrandom because we do not > provide a way for applications to implement system calls as cancellation > points (unless we provide a wrapper for the specific system call, of > course). What I'm asking for is evidence that that is actually a problem for at least one real application. Also evidence that making getrandom a cancellation point _won't_ break programs that naively assume it can never fail. zw
On 11/17/2016 02:56 PM, Zack Weinberg wrote: > On 11/17/2016 08:50 AM, Florian Weimer wrote: >> On 11/17/2016 02:45 PM, Zack Weinberg wrote: >>> On 11/17/2016 08:02 AM, Florian Weimer wrote: >>>> On 11/16/2016 05:41 PM, Zack Weinberg wrote: >>>>> On Wed, Nov 16, 2016 at 10:52 AM, Florian Weimer <fweimer@redhat.com> >>>>> wrote: >>>>>> On 11/16/2016 04:20 PM, Zack Weinberg wrote: >>>>>>> I am seriously considering escalating my disagreement here to a >>>>>>> formal objection. I would like to know why you think it is >>>>>>> NECESSARY - not merely convenient or consistent with other stuff - >>>>>>> for this function to be a cancellation point. >>>>>> >>>>>> It's necessary if you ever want to cancel a hanging getrandom in a >>>>>> context where you cannot install a signal handler (so that you can >>>>>> trigger EINTR when getrandom is stuck). >>>>> >>>>> That only pushes the question back a level. When would it ever be >>>>> necessary to do that? Be as concrete as you possibly can. Actual >>>>> code from a real program, if possible. >>>> >>>> It's not clear to me what you are asking here. >>>> >>>> Do you mean cancellation in general, or cancellation in conjunction with >>>> getrandom specifically? >>> >>> Sorry. I meant cancellation specifically of a thread hanging in >>> getrandom. >> >> I'm not sure how I can provide that, considering that there is currently >> no way to cancel a thread which hangs in getrandom because we do not >> provide a way for applications to implement system calls as cancellation >> points (unless we provide a wrapper for the specific system call, of >> course). > > What I'm asking for is evidence that that is actually a problem for at > least one real application. I found this: <https://opensource.apple.com/source/ppp/ppp-412.3/Helpers/vpnd/ipsecoptions.c> ipsec_resolver_thread reads from /dev/random, which can block for a very long time. The thread for it is spawned in ipsec_process_prefs, which cancels it when got_terminate returns true. The intent seems to be that is terminated on graceful process termination: read in ipsec_process_prefs returns EINTR after a signal handler has run which makes got_terminate (defined in main.c in the parent directory) return true. If the read from /dev/random is replaced with a call to getrandom which is not a cancellation point, this would like not work as intended. Is this the kind of stuff you are looking for? > Also evidence that making getrandom a > cancellation point _won't_ break programs that naively assume it can > never fail. Cancellation does not add additional error return cases. Thanks, Florian
On 11/17/2016 10:24 AM, Florian Weimer wrote: > On 11/17/2016 02:56 PM, Zack Weinberg wrote: >> >> What I'm asking for is evidence that that is actually a problem for at >> least one real application. > > I found this: > > <https://opensource.apple.com/source/ppp/ppp-412.3/Helpers/vpnd/ipsecoptions.c> > > ipsec_resolver_thread reads from /dev/random, which can block for a very > long time. The thread for it is spawned in ipsec_process_prefs, which > cancels it when got_terminate returns true. The intent seems to be that > is terminated on graceful process termination: read in > ipsec_process_prefs returns EINTR after a signal handler has run which > makes got_terminate (defined in main.c in the parent directory) return > true. It seems to me that this code does not *need* to cancel the ipsec_resolver_thread when it does, as it is about to call exit() [I think] which will blow away all threads anyway. >> Also evidence that making getrandom a >> cancellation point _won't_ break programs that naively assume it can >> never fail. > > Cancellation does not add additional error return cases. It does add additional _failure_ cases. Suppose a program that expects threads only to be at risk of cancellation at points where they do network I/O, and does all the necessary dancing to make that reliable. These threads are _already_ using getrandom() where available, via a portability wrapper that will call into the C library if possible, or make a direct syscall otherwise. Being a wrapper, it's not a cancellation point, and the surrounding code relies on that. Now you upgrade glibc, and suddenly getrandom() _is_ a cancellation point, and the threads can now be cancelled in places where their data structures are internally inconsistent -- and it doesn't matter that getrandom() doesn't block under normal conditions, because the generic cancel-testing code will fire anyway. [This is just the general argument that adding new cancellation points to the C library can render existing code buggy without notice.] zw
On 17/11/16 12:52, Torvald Riegel wrote: > On Wed, 2016-11-16 at 17:52 -0200, Adhemerval Zanella wrote: >> >> On 16/11/2016 16:02, Torvald Riegel wrote: >>> On Wed, 2016-11-16 at 16:52 +0100, Florian Weimer wrote: >>>> On 11/16/2016 04:20 PM, Zack Weinberg wrote: >>>>> On 11/16/2016 10:11 AM, Florian Weimer wrote: >>>>>> On 11/14/2016 07:29 PM, Zack Weinberg wrote: >>>>>>> On 11/14/2016 12:44 PM, Florian Weimer wrote: >>>>>>>> This patch switches back to the ssize_t return time. This goes against >>>>>>>> Theodore Ts'o preference, but seems to reflect the consensus from the >>>>>>>> largery community. >>>>>>> >>>>>>> I still don't think this function should be a cancellation point. >>>>>> >>>>>> I guess we'll have to agree to disagree on this matter. >>>>> >>>>> I am seriously considering escalating my disagreement here to a formal >>>>> objection. I would like to know why you think it is NECESSARY - not >>>>> merely convenient or consistent with other stuff - for this function to >>>>> be a cancellation point. >>>> >>>> It's necessary if you ever want to cancel a hanging getrandom in a >>>> context where you cannot install a signal handler (so that you can >>>> trigger EINTR when getrandom is stuck). >>> >>> I think it would be better to split the getrandom that is a cancellation >>> point into two functions, getrandom (not a cancellation point) and >>> getrandom_cancelable (is a cancellation point). This way, the >>> functionality is available for programs but requires explicit opt-in, >>> while the default is not going to lead to surprises in the expected >>> common case (ie, when randomness is available). I don't think the >>> opt-in is a problem because as you said, cancellation requires careful >>> programming anyway. >>> >> >> I would advise against adding another symbol, it only adds complexity >> and most likely one interface would be preferable from application >> point of view. > > We could add a parameter too that determines whether it is a > cancellation point. That avoids your concern about adding another why? use pthread_setcancelstate. works with all cancellation points not just getrandom. > symbol, and programmers will still have to make a conscious decision > about cancellation. I don't see how this adds complexity; the > complexity in there is having to choose between the different semantics, > or be aware that it's just one of them when we know that just one of > them is not a one-size-fits-all for all use cases. >
On 17/11/16 17:16, Zack Weinberg wrote: > On 11/17/2016 10:24 AM, Florian Weimer wrote: >> On 11/17/2016 02:56 PM, Zack Weinberg wrote: >>> Also evidence that making getrandom a >>> cancellation point _won't_ break programs that naively assume it can >>> never fail. >> >> Cancellation does not add additional error return cases. > > It does add additional _failure_ cases. Suppose a program that expects > threads only to be at risk of cancellation at points where they do > network I/O, and does all the necessary dancing to make that reliable. > These threads are _already_ using getrandom() where available, via a > portability wrapper that will call into the C library if possible, or > make a direct syscall otherwise. Being a wrapper, it's not a > cancellation point, and the surrounding code relies on that. Now you > upgrade glibc, and suddenly getrandom() _is_ a cancellation point, and > the threads can now be cancelled in places where their data structures > are internally inconsistent -- and it doesn't matter that getrandom() > doesn't block under normal conditions, because the generic > cancel-testing code will fire anyway. > > [This is just the general argument that adding new cancellation points > to the C library can render existing code buggy without notice.] there is no existing code that uses glibc getrandom. a user can easily turn a cancellation point into a non-cancellation one if desired, but the other way is not possible. blocking syscalls have to be cancellation points otherwise they cannot be called safely from a long running process that has to remain responsive: blocked threads can keep piling up and there is no way to reuse the resources they hold. [this is the general argument for adding new blocking syscalls as cancellation points].
On 11/17/2016 07:21 AM, Mike Frysinger wrote: > On 14 Nov 2016 18:44, Florian Weimer wrote: > > just nits at this point > >> +/* Flags for use with getrandom. */ >> +#define GRND_NONBLOCK 1 >> +#define GRND_RANDOM 2 > > if they're bit flags, should we be doing 0x1/0x2 etc ? otherwise this > will turn into 4, 8, 16, 32, 64, etc... which gets ugly. the kernel > headers use hex constants. Okay, I turned this into: #define GRND_NONBLOCK 0x01 #define GRND_RANDOM 0x02 Do you want more zero padding? (I'm not a fan of column alignment because it means that future patches will have to make whitespace-only changes to change column alignment, which slightly obfuscates the actual change.) >> +/* Test getrandom with a single buffer length. */ >> +static void >> +test_length (char *buffer, size_t length, unsigned int flags) >> +{ >> + memset (buffer, 0, length); >> + strcpy (buffer + length, "123"); > > while this works, it seems pointlessly fragile. can't you treat it like > a normal "this is the length of the buffer" and carve out space at the > end yourself ? i.e. > memset (buffer, 0, length); > static const char canary[] = "123"; > size_t canary_len = sizeof(canary); > length -= canary_len; > strcpy (buffer + length, canary); > ... > > ssize_t ret = getrandom (buffer, length - , flags); I don't think this is much clearer. I'm going to add a comment: /* Test getrandom with a single buffer length. NB: The passed-in buffer must have room for four extra bytes after the specified length, which are used to test that getrandom leaves those bytes unchanged. */ Hopefully this is clear enough. > >> + if (ret < 0) >> + { >> + if (!((flags & GRND_RANDOM) >> + && (flags & GRND_NONBLOCK) >> + && errno != EAGAIN)) > > seems like it'd be more readable to distribute the ! and to combine the > flags check into a single mask ? i have to read these lines a few times > to digest what exactly the code is trying to do. What about this? /* EAGAIN is an expected error with GRND_RANDOM and GRND_NONBLOCK. */ if ((flags & GRND_RANDOM) && (flags & GRND_NONBLOCK) && errno == EAGAIN) return; printf ("error: getrandom (%zu, 0x%x): %m\n", length, flags); errors = true; return; The second return was missing before, the old condition was actually wrong. >> + if (getrandom_full (buffer1, sizeof (buffer1), flags) >> + && getrandom_full (buffer2, sizeof (buffer2), flags)) >> + { >> + if (memcmp (buffer1, buffer2, sizeof (buffer1)) == 0) > > maybe also add a comment that likelihood of this being the same is > extremely rare too. This should do it: /* The probability that these two 8-byte buffers are equal is very small (assuming that two subsequent calls to getrandom result are independent, uniformly distributed random variables). */ >> + for (int use_random = 0; use_random < 2; ++use_random) >> + for (int use_nonblock = 0; use_nonblock < 2; ++use_nonblock) >> + { >> + int flags = 0; > > unsigned to match the API ? Right, thanks. Do you have any comments about the matter of the cancellation point and the redirect to __libc_getrandom? Thanks, florian
On Fri, 2016-11-18 at 08:28 +0000, Szabolcs Nagy wrote: > On 17/11/16 12:52, Torvald Riegel wrote: > > On Wed, 2016-11-16 at 17:52 -0200, Adhemerval Zanella wrote: > >> > >> On 16/11/2016 16:02, Torvald Riegel wrote: > >>> On Wed, 2016-11-16 at 16:52 +0100, Florian Weimer wrote: > >>>> On 11/16/2016 04:20 PM, Zack Weinberg wrote: > >>>>> On 11/16/2016 10:11 AM, Florian Weimer wrote: > >>>>>> On 11/14/2016 07:29 PM, Zack Weinberg wrote: > >>>>>>> On 11/14/2016 12:44 PM, Florian Weimer wrote: > >>>>>>>> This patch switches back to the ssize_t return time. This goes against > >>>>>>>> Theodore Ts'o preference, but seems to reflect the consensus from the > >>>>>>>> largery community. > >>>>>>> > >>>>>>> I still don't think this function should be a cancellation point. > >>>>>> > >>>>>> I guess we'll have to agree to disagree on this matter. > >>>>> > >>>>> I am seriously considering escalating my disagreement here to a formal > >>>>> objection. I would like to know why you think it is NECESSARY - not > >>>>> merely convenient or consistent with other stuff - for this function to > >>>>> be a cancellation point. > >>>> > >>>> It's necessary if you ever want to cancel a hanging getrandom in a > >>>> context where you cannot install a signal handler (so that you can > >>>> trigger EINTR when getrandom is stuck). > >>> > >>> I think it would be better to split the getrandom that is a cancellation > >>> point into two functions, getrandom (not a cancellation point) and > >>> getrandom_cancelable (is a cancellation point). This way, the > >>> functionality is available for programs but requires explicit opt-in, > >>> while the default is not going to lead to surprises in the expected > >>> common case (ie, when randomness is available). I don't think the > >>> opt-in is a problem because as you said, cancellation requires careful > >>> programming anyway. > >>> > >> > >> I would advise against adding another symbol, it only adds complexity > >> and most likely one interface would be preferable from application > >> point of view. > > > > We could add a parameter too that determines whether it is a > > cancellation point. That avoids your concern about adding another > > why? > > use pthread_setcancelstate. > > works with all cancellation points not just getrandom. As discussed in the thread, there are different opinions about what the default should be. There are reasonable arguments for both options. In such a case, it seems better to make the choice explicit, simply from an ease-of-use and interface design perspective. This is not about whether it's possible for users to do (they could build their own syscall wrappers after all too, right? ;) ) but about clean interfaces. With your proposal, one could argue that, for example, every library has to be aware of cancellation and how it works if one of the clients of the library could want to use cancellation; the library either has to disable it explicitly, or it has to document which additional cancellation points exist and has to implement the cleanup handlers. This is error-prone and adds complexity to those use cases. Therefore, it seems better to avoid that potential source of bugs and either make the default to not support cancellation (ie, an opt-in for the cancellation facility), or make at least make the choice explicit for users of getrandom (ie, two functions or an additional parameter).
On 11/18/2016 03:21 PM, Torvald Riegel wrote: > As discussed in the thread, there are different opinions about what the > default should be. There are reasonable arguments for both options. In > such a case, it seems better to make the choice explicit, simply from an > ease-of-use and interface design perspective. Unfortunately, this is not the approach that POSIX has chosen. But there is precedent for doing our own thing in this area: the "c" flag for fopen. We cannot use the existing flags argument in getrandom for this purpose because its layout is controlled by the kernel. > This is not about whether > it's possible for users to do (they could build their own syscall > wrappers after all too, right? ;) ) but about clean interfaces. It's not possible to opt in to cancellation at system calls using only public glibc interfaces. The best you can do is to implement cancellation from scratch, using a different signal. > With your proposal, one could argue that, for example, every library has > to be aware of cancellation and how it works if one of the clients of > the library could want to use cancellation; But that is true for most libraries today, no matter what we do about getrandom. getrandom just does not add significant additional exposure in this area. > the library either has to > disable it explicitly, or it has to document which additional > cancellation points exist and has to implement the cleanup handlers. > This is error-prone and adds complexity to those use cases. Therefore, > it seems better to avoid that potential source of bugs and either make > the default to not support cancellation (ie, an opt-in for the > cancellation facility), or make at least make the choice explicit for > users of getrandom (ie, two functions or an additional parameter). I'm increasingly worried that this discussion is actually about thread cancellation in general and only peripherally about getrandom. Considering the fringe nature of getrandom (it is intended for the implementation of PRNG seeding, after all), it seems to be a strange choice to attempt to settle this debate. Does POSIX even say that cancellation has to be enabled for new threads by default, like we do? It's probably too late to change this. In the end, this is very similar to the ongoing debate about exception handling in C++ and other languages. For most such languages, there are large code bases which ban exceptions for various reasons, and others which use them with success. I don't think we can reach agreement which one application developers should prefer. The only thing we can do is try to be as consistent as possible (and for getrandom, I think the model should be the read function, and not rand). This discussion is also blocking further work on my part for additional randomness generation functions. More user-oriented interfaces we could add, such as getentropy or arc4random, should *not* be cancellation points, I think. Thanks, Florian
On Fri, 2016-11-18 at 10:27 +0000, Szabolcs Nagy wrote: > blocking syscalls have to be cancellation points > otherwise they cannot be called safely from a long > running process that has to remain responsive: > blocked threads can keep piling up and there is no > way to reuse the resources they hold. > > [this is the general argument for adding new > blocking syscalls as cancellation points]. You are describing just *one* use case. You also don't define what "remain responsive" is supposed to mean; there's a difference between being able to cancel any time vs. ensuring that a syscall will not block indefinitely (eg, I think the common case for getrandom should be that it does not block indefinitely, and scenarios in which no randomness can be found nor "generated" are corner cases). Related to that, you don't distinguish between what can be done to recover when a particular syscall does not complete; for example, is there likely to be a workaround for a getrandom that does complete, or is calling exit() the most likely response. You also don't discuss who the expected audience for a syscall wrapper is. How likely it is that they are aware of cancellation, might make use of it, etc.? How likely is the syscall going to be used directly vs. being hidden by another wrapper that is produced by developers that understand cancellation? Just the fact that there are all these different considerations suggests that there's no clean one-size-fits-all interface. Hence my suggestion to at least make the choice explicit (and thus obvious to users), for example by either an additional parameter or by providing two wrappers. If there is in fact no good default or one-size-fits-all, we should not pretend that there is. Finally, pthreads cancellation is a pretty crude means to build asynchronous / cancellable operations because you cannot cancel just a particular call; you always cancel a thread, which means that in general, you need additional synchronization or other constraints to make sure that you do not cancel at another cancellation point in some other unrelated operation executed by the same thread.
On Fri, 2016-11-18 at 16:13 +0100, Florian Weimer wrote: > On 11/18/2016 03:21 PM, Torvald Riegel wrote: > > > As discussed in the thread, there are different opinions about what the > > default should be. There are reasonable arguments for both options. In > > such a case, it seems better to make the choice explicit, simply from an > > ease-of-use and interface design perspective. > > Unfortunately, this is not the approach that POSIX has chosen. But > there is precedent for doing our own thing in this area: the "c" flag > for fopen. We cannot use the existing flags argument in getrandom for > this purpose because its layout is controlled by the kernel. It seems a separate argument would be better than using up space in the existing flags. Cancellation is something we add, so we should add to the underlying interface too, instead of messing with it. > > This is not about whether > > it's possible for users to do (they could build their own syscall > > wrappers after all too, right? ;) ) but about clean interfaces. > > It's not possible to opt in to cancellation at system calls using only > public glibc interfaces. The best you can do is to implement > cancellation from scratch, using a different signal. I agree with that, but I was replying specifically to Scabolcs' comment that users would have to just disable cancellation around a cancelable syscall wrapper. > > With your proposal, one could argue that, for example, every library has > > to be aware of cancellation and how it works if one of the clients of > > the library could want to use cancellation; > > But that is true for most libraries today, no matter what we do about > getrandom. getrandom just does not add significant additional exposure > in this area. That's not true for a library that just wraps getrandom functionality. I think that we should care about that independently of whether the same problem exists elsewhere, unless it's clear that users will always have to face the problem (eg, if getrandom would only be usable when also doing file I/O at the same time). > > the library either has to > > disable it explicitly, or it has to document which additional > > cancellation points exist and has to implement the cleanup handlers. > > This is error-prone and adds complexity to those use cases. Therefore, > > it seems better to avoid that potential source of bugs and either make > > the default to not support cancellation (ie, an opt-in for the > > cancellation facility), or make at least make the choice explicit for > > users of getrandom (ie, two functions or an additional parameter). > > I'm increasingly worried that this discussion is actually about thread > cancellation in general and only peripherally about getrandom. > Considering the fringe nature of getrandom (it is intended for the > implementation of PRNG seeding, after all), it seems to be a strange > choice to attempt to settle this debate. I can understand that you feel burdened by the wider debate, but I'm also not convinced that we can ignore it just because it does something that will be executed rarely. I think the fringe nature might even be an argument for having to be more careful about cancellation: If getrandom in that corner over there, far away from other uses of cancellation, how likely is it that users of it are aware of cancellation at all? > Does POSIX even say that cancellation has to be enabled for new threads > by default, like we do? It's probably too late to change this. > > In the end, this is very similar to the ongoing debate about exception > handling in C++ and other languages. For most such languages, there are > large code bases which ban exceptions for various reasons, and others > which use them with success. I don't think we can reach agreement which > one application developers should prefer. The only thing we can do is > try to be as consistent as possible (and for getrandom, I think the > model should be the read function, and not rand). I don't think this is a clean analogy. > This discussion is also blocking further work on my part for additional > randomness generation functions. More user-oriented interfaces we could > add, such as getentropy or arc4random, should *not* be cancellation > points, I think. That's unfortunate, I agree. What I haven't seen so far is a convincing argument why we should not make the choice of cancellation point or not explicit in the wrapper. Some have voiced broad concerns about the number of symbols (but is this really such a big deal?), but I haven't seen an argument yet which expressed detailed concerns about adding a separate argument that determines whether there is cancellation point or not. If we can't decide on one solution, mostly because we expect there to be different use cases and users, why should we try to force a decision instead of exposing the choice? -- especially given the point you make about this being a very specialized syscall.
On Fri, Nov 18, 2016 at 5:27 AM, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > On 17/11/16 17:16, Zack Weinberg wrote: >> [This is just the general argument that adding new cancellation points >> to the C library can render existing code buggy without notice.] > > there is no existing code that uses glibc getrandom. > > a user can easily turn a cancellation point into > a non-cancellation one if desired, but the other > way is not possible. I specifically said there was a wrapper that anticipates the availability of glibc getrandom, and uses a direct syscall in the meantime. AC_REPLACE_FUNCS(getrandom) or dlsym("getrandom") or like that. > blocking syscalls have to be cancellation points > otherwise they cannot be called safely from a long > running process that has to remain responsive: > blocked threads can keep piling up and there is no > way to reuse the resources they hold. This is a valid point - I personally read it as an argument against adding any new blocking syscalls, but that's not the world we live in. I don't want to derail this into a general debate over adding new cancellation points, and I especially don't want to hold up work on a user-space CSPRNG on something unrelated, because arc4random() is the top item on _my_ todo list after explicit_bzero(). So here is a counterproposal. Most of my concerns about getrandom() being a cancellation point go away if it is only a cancellation point when it is actually going to block -- note that I very much do _not_ mean "when it is called with arguments that permit it to block." How about we have the public getrandom do like this: ssize_t getrandom (void *buffer, size_t length, unsigned int flags) { ssize_t rv = __getrandom_nocancel (buffer, length, flags | GRND_NONBLOCK); if (rv == length) return rv; if (rv < 0 && ((flags & GRND_NONBLOCK) || errno != EAGAIN) return rv; rv = max(rv, 0); return __getrandom_maycancel (buffer + rv, length - rv, flags); } I could live with that. zw
On Fri, 2016-11-18 at 13:50 -0500, Zack Weinberg wrote: > I don't want to derail this into a general debate over adding new > cancellation points, and I especially don't want to hold up work on a > user-space CSPRNG on something unrelated, because arc4random() is the > top item on _my_ todo list after explicit_bzero(). So here is a > counterproposal. Most of my concerns about getrandom() being a > cancellation point go away if it is only a cancellation point when it > is actually going to block Is there a way for the caller to ensure that a subsequent call to getrandom is not going to block? If there isn't, then the caller has to conservatively assume that the call may be a cancellation point. I still think something like ssize_t getrandom (/*getrandom parameters*/, bool cancellation_point); would probably be the best option because it requires a caller to make a conscious decision about cancellation (and thus also makes the caller aware of the whole cancellation thing).
On Mon, Nov 21, 2016 at 11:57 AM, Torvald Riegel <triegel@redhat.com> wrote: > On Fri, 2016-11-18 at 13:50 -0500, Zack Weinberg wrote: >> I don't want to derail this into a general debate over adding new >> cancellation points, and I especially don't want to hold up work on a >> user-space CSPRNG on something unrelated, because arc4random() is the >> top item on _my_ todo list after explicit_bzero(). So here is a >> counterproposal. Most of my concerns about getrandom() being a >> cancellation point go away if it is only a cancellation point when it >> is actually going to block > > Is there a way for the caller to ensure that a subsequent call to > getrandom is not going to block? If there isn't, then the caller has to > conservatively assume that the call may be a cancellation point. The whole reason I can live with my proposal is that most code should be able to assume getrandom *never* blocks. Specifically, if getrandom ever blocks once we're past a fairly early stage of boot-up, that's a bug in either the kernel or the setup procedure. (I'd *like* to get to where the kernel wouldn't even start process 1 until it could guarantee that the RNG would not block, but that's a long way off.) So the only code that has to worry about it is early-stage boot code that has to be written hyper-defensively anyway. zw
On Mon, 2016-11-21 at 12:12 -0500, Zack Weinberg wrote: > On Mon, Nov 21, 2016 at 11:57 AM, Torvald Riegel <triegel@redhat.com> wrote: > > On Fri, 2016-11-18 at 13:50 -0500, Zack Weinberg wrote: > >> I don't want to derail this into a general debate over adding new > >> cancellation points, and I especially don't want to hold up work on a > >> user-space CSPRNG on something unrelated, because arc4random() is the > >> top item on _my_ todo list after explicit_bzero(). So here is a > >> counterproposal. Most of my concerns about getrandom() being a > >> cancellation point go away if it is only a cancellation point when it > >> is actually going to block > > > > Is there a way for the caller to ensure that a subsequent call to > > getrandom is not going to block? If there isn't, then the caller has to > > conservatively assume that the call may be a cancellation point. > > The whole reason I can live with my proposal is that most code should > be able to assume getrandom *never* blocks. Specifically, if > getrandom ever blocks once we're past a fairly early stage of boot-up, > that's a bug in either the kernel or the setup procedure. Does the kernel guarantee this, meaning that it's specifically documented as part of the interface? Or similarly, does it give a guarantee under some precondition (which systemd or similar guarantees to establish)? I understand why you think this would be useful behavior, but the "should be able" part in your statement is too vague for us to rely on. Also, what do you mean specifically by "block"? If it is just taking a relatively long time, but will always eventually finish irrespective of whether other processes do or do not do something, then I wouldn't classify the behavior as blocking.
On 11/21/2016 06:30 PM, Torvald Riegel wrote: > Does the kernel guarantee this, meaning that it's specifically > documented as part of the interface? Yes, if the flags argument is 0, the call will only block if the kernel randomness pool has not been seeded yet. And this is the reason why people want getrandom: they want to stop reading potentially predictable bits from /dev/urandom. This is documented in the manual page. Florian
On 11/18/2016 05:04 PM, Torvald Riegel wrote: > On Fri, 2016-11-18 at 16:13 +0100, Florian Weimer wrote: >> On 11/18/2016 03:21 PM, Torvald Riegel wrote: >> >>> As discussed in the thread, there are different opinions about what the >>> default should be. There are reasonable arguments for both options. In >>> such a case, it seems better to make the choice explicit, simply from an >>> ease-of-use and interface design perspective. >> >> Unfortunately, this is not the approach that POSIX has chosen. But >> there is precedent for doing our own thing in this area: the "c" flag >> for fopen. We cannot use the existing flags argument in getrandom for >> this purpose because its layout is controlled by the kernel. > > It seems a separate argument would be better than using up space in the > existing flags. Cancellation is something we add, so we should add to > the underlying interface too, instead of messing with it. Is this separate argument your personal preference, or are you just trying to find common ground and reconcile different positions? Thanks, Florian
On 11/18/2016 07:50 PM, Zack Weinberg wrote: > I don't want to derail this into a general debate over adding new > cancellation points, and I especially don't want to hold up work on a > user-space CSPRNG on something unrelated, because arc4random() is the > top item on _my_ todo list after explicit_bzero(). So here is a > counterproposal. Most of my concerns about getrandom() being a > cancellation point go away if it is only a cancellation point when it > is actually going to block -- note that I very much do _not_ mean > "when it is called with arguments that permit it to block." How about > we have the public getrandom do like this: > > ssize_t getrandom (void *buffer, size_t length, unsigned int flags) > { > ssize_t rv = __getrandom_nocancel (buffer, length, flags | GRND_NONBLOCK); > if (rv == length) > return rv; > if (rv < 0 && ((flags & GRND_NONBLOCK) || errno != EAGAIN) > return rv; > rv = max(rv, 0); > return __getrandom_maycancel (buffer + rv, length - rv, flags); > } This is an intriguing idea, but it actually makes reasoning about cancellation more difficult, particularly with GRND_RANDOM. I haven't seen a rationale for the POSIX design (in which cancellation points always check for cancellation, even if they don't block). I suspect it goes something like this: If thread cancellation happens on only blocking, then whether cancellation occurs depends on a race: the cancellation has to occur while being blocked. This might never happen. It's hard to tell whether you actually can cancel threads doing this (when cancellation is desired), or that they do not get canceled accidentally. Thanks, Florian
On Tue, 2016-11-29 at 09:16 +0100, Florian Weimer wrote: > On 11/18/2016 05:04 PM, Torvald Riegel wrote: > > On Fri, 2016-11-18 at 16:13 +0100, Florian Weimer wrote: > >> On 11/18/2016 03:21 PM, Torvald Riegel wrote: > >> > >>> As discussed in the thread, there are different opinions about what the > >>> default should be. There are reasonable arguments for both options. In > >>> such a case, it seems better to make the choice explicit, simply from an > >>> ease-of-use and interface design perspective. > >> > >> Unfortunately, this is not the approach that POSIX has chosen. But > >> there is precedent for doing our own thing in this area: the "c" flag > >> for fopen. We cannot use the existing flags argument in getrandom for > >> this purpose because its layout is controlled by the kernel. > > > > It seems a separate argument would be better than using up space in the > > existing flags. Cancellation is something we add, so we should add to > > the underlying interface too, instead of messing with it. > > Is this separate argument your personal preference, or are you just > trying to find common ground and reconcile different positions? It's my personal preference. Which is partially motivated by trying to find common ground between the different use cases (and not finding obvious common group, so therefore make the choice explicit).
On 11/29/2016 02:56 PM, Torvald Riegel wrote: > On Tue, 2016-11-29 at 09:16 +0100, Florian Weimer wrote: >> On 11/18/2016 05:04 PM, Torvald Riegel wrote: >>> On Fri, 2016-11-18 at 16:13 +0100, Florian Weimer wrote: >>>> On 11/18/2016 03:21 PM, Torvald Riegel wrote: >>>> >>>>> As discussed in the thread, there are different opinions about what the >>>>> default should be. There are reasonable arguments for both options. In >>>>> such a case, it seems better to make the choice explicit, simply from an >>>>> ease-of-use and interface design perspective. >>>> >>>> Unfortunately, this is not the approach that POSIX has chosen. But >>>> there is precedent for doing our own thing in this area: the "c" flag >>>> for fopen. We cannot use the existing flags argument in getrandom for >>>> this purpose because its layout is controlled by the kernel. >>> >>> It seems a separate argument would be better than using up space in the >>> existing flags. Cancellation is something we add, so we should add to >>> the underlying interface too, instead of messing with it. >> >> Is this separate argument your personal preference, or are you just >> trying to find common ground and reconcile different positions? > > It's my personal preference. Which is partially motivated by trying to > find common ground between the different use cases (and not finding > obvious common group, so therefore make the choice explicit). Hmph. I was about to propose a new patch, with two functions: getrandom, as I posted it the last time (an unadorned system call which is also a cancellation point). getentropy, a thin wrapper which avoids returning EINTR (to match OpenBSD and Solaris) and is not a cancellation point. It would return EIO on short reads, too. The documentation would have said that getrandom is a lower-level function for those which need GRND_RANDOM or cancellation, and everyone else should call getrandom. Would this work for you? Thanks, Florian
On Tue, 2016-11-29 at 15:40 +0100, Florian Weimer wrote: > On 11/29/2016 02:56 PM, Torvald Riegel wrote: > > On Tue, 2016-11-29 at 09:16 +0100, Florian Weimer wrote: > >> On 11/18/2016 05:04 PM, Torvald Riegel wrote: > >>> On Fri, 2016-11-18 at 16:13 +0100, Florian Weimer wrote: > >>>> On 11/18/2016 03:21 PM, Torvald Riegel wrote: > >>>> > >>>>> As discussed in the thread, there are different opinions about what the > >>>>> default should be. There are reasonable arguments for both options. In > >>>>> such a case, it seems better to make the choice explicit, simply from an > >>>>> ease-of-use and interface design perspective. > >>>> > >>>> Unfortunately, this is not the approach that POSIX has chosen. But > >>>> there is precedent for doing our own thing in this area: the "c" flag > >>>> for fopen. We cannot use the existing flags argument in getrandom for > >>>> this purpose because its layout is controlled by the kernel. > >>> > >>> It seems a separate argument would be better than using up space in the > >>> existing flags. Cancellation is something we add, so we should add to > >>> the underlying interface too, instead of messing with it. > >> > >> Is this separate argument your personal preference, or are you just > >> trying to find common ground and reconcile different positions? > > > > It's my personal preference. Which is partially motivated by trying to > > find common ground between the different use cases (and not finding > > obvious common group, so therefore make the choice explicit). > > Hmph. Note that I mean that this is my preference specifically in the scenario of adding some sort of flag to getrandom() and offering only getrandom(). > I was about to propose a new patch, with two functions: > > getrandom, as I posted it the last time (an unadorned system call which > is also a cancellation point). > > getentropy, a thin wrapper which avoids returning EINTR (to match > OpenBSD and Solaris) and is not a cancellation point. It would return > EIO on short reads, too. > > The documentation would have said that getrandom is a lower-level > function for those which need GRND_RANDOM or cancellation, and everyone > else should call getrandom. I guess one of these should be getentropy (the latter?). > Would this work for you? Yeah, I guess so. I can't really estimate how obvious it would be for users to have to consider the pair of these (and thus make a conscious choice and understand what cancellation means) -- but if we can make this fairly obvious (eg, through proper documentation) and thus are likely to prevent users from making a mistake, and we have an alternative for the non-cancellation use case, then I guess this could be a meaningful solution.
On 11/29/2016 04:23 PM, Torvald Riegel wrote: > On Tue, 2016-11-29 at 15:40 +0100, Florian Weimer wrote: >> On 11/29/2016 02:56 PM, Torvald Riegel wrote: >>> On Tue, 2016-11-29 at 09:16 +0100, Florian Weimer wrote: >>>> On 11/18/2016 05:04 PM, Torvald Riegel wrote: >>>>> On Fri, 2016-11-18 at 16:13 +0100, Florian Weimer wrote: >>>>>> On 11/18/2016 03:21 PM, Torvald Riegel wrote: >>>>>> >>>>>>> As discussed in the thread, there are different opinions about what the >>>>>>> default should be. There are reasonable arguments for both options. In >>>>>>> such a case, it seems better to make the choice explicit, simply from an >>>>>>> ease-of-use and interface design perspective. >>>>>> >>>>>> Unfortunately, this is not the approach that POSIX has chosen. But >>>>>> there is precedent for doing our own thing in this area: the "c" flag >>>>>> for fopen. We cannot use the existing flags argument in getrandom for >>>>>> this purpose because its layout is controlled by the kernel. >>>>> >>>>> It seems a separate argument would be better than using up space in the >>>>> existing flags. Cancellation is something we add, so we should add to >>>>> the underlying interface too, instead of messing with it. >>>> >>>> Is this separate argument your personal preference, or are you just >>>> trying to find common ground and reconcile different positions? >>> >>> It's my personal preference. Which is partially motivated by trying to >>> find common ground between the different use cases (and not finding >>> obvious common group, so therefore make the choice explicit). >> >> Hmph. > > Note that I mean that this is my preference specifically in the scenario > of adding some sort of flag to getrandom() and offering only > getrandom(). > >> I was about to propose a new patch, with two functions: >> >> getrandom, as I posted it the last time (an unadorned system call which >> is also a cancellation point). >> >> getentropy, a thin wrapper which avoids returning EINTR (to match >> OpenBSD and Solaris) and is not a cancellation point. It would return >> EIO on short reads, too. >> >> The documentation would have said that getrandom is a lower-level >> function for those which need GRND_RANDOM or cancellation, and everyone >> else should call getrandom. > > I guess one of these should be getentropy (the latter?). Yes, developers should prefer getentropy or a PRNG seeded from getentropy. >> Would this work for you? > > Yeah, I guess so. I can't really estimate how obvious it would be for > users to have to consider the pair of these (and thus make a conscious > choice and understand what cancellation means) -- but if we can make > this fairly obvious (eg, through proper documentation) and thus are > likely to prevent users from making a mistake, and we have an > alternative for the non-cancellation use case, then I guess this could > be a meaningful solution. Great. Zack, what do you think? Thanks, Florian
On Tue, Nov 29, 2016 at 10:32 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 11/29/2016 04:23 PM, Torvald Riegel wrote: >> >> On Tue, 2016-11-29 at 15:40 +0100, Florian Weimer wrote: >>> >>> I was about to propose a new patch, with two functions: >>> >>> getrandom, as I posted it the last time (an unadorned system call which >>> is also a cancellation point). >>> >>> getentropy, a thin wrapper which avoids returning EINTR (to match >>> OpenBSD and Solaris) and is not a cancellation point. It would return >>> EIO on short reads, too. >>> >>> The documentation would have said that getrandom is a lower-level >>> function for those which need GRND_RANDOM or cancellation, and everyone >>> else should call getrandom. >> >> I guess one of these should be getentropy (the latter?). > > Yes, developers should prefer getentropy or a PRNG seeded from getentropy. > >>> Would this work for you? >> >> >> Yeah, I guess so. I can't really estimate how obvious it would be for >> users to have to consider the pair of these (and thus make a conscious >> choice and understand what cancellation means) -- but if we can make >> this fairly obvious (eg, through proper documentation) and thus are >> likely to prevent users from making a mistake, and we have an >> alternative for the non-cancellation use case, then I guess this could >> be a meaningful solution. > > Great. > > Zack, what do you think? Yeah, this would also work for me. Would it make sense for getentropy to loop internally rather than returning a short read? I am really worried about people forgetting to handle short reads. zw
On 11/29/2016 07:54 AM, Zack Weinberg wrote: > Would it make sense for getentropy > to loop internally rather than returning a short read? FWIW, that is what I was planning when implementing a more-portable version of getentropy (I guess that's its name now?) for Gnulib, after the API settles down. Gnulib-using applications typically need a small number of random bytes. I don't want to worry about modifying these apps to take into account the possibility of waiting indefinitely due to a bug in the hardware or operating system. If such a wait occurs, it's OK with me to just say "fix your platform".
On 11/29/2016 06:53 PM, Paul Eggert wrote: > On 11/29/2016 07:54 AM, Zack Weinberg wrote: >> Would it make sense for getentropy >> to loop internally rather than returning a short read? > > FWIW, that is what I was planning when implementing a more-portable > version of getentropy (I guess that's its name now?) for Gnulib, How are you going to avoid interposing the implementation in glibc? Thanks, Florian
On 11/29/2016 10:11 AM, Florian Weimer wrote:
> How are you going to avoid interposing the implementation in glibc?
That will depend on how glibc is implemented. Simplest might be to use
compile-time interposition, which is common in Gnulib. E.g., Gnulib
would supply its own getentropy.h which does "#define getentropy
rpl_getentropy", so that Gnulib-using applications always call
rpl_getentropy from the linker's point of view. This sort of thing is OK
for Gnulib, as it's not intended to be a separate library.
On 11/29/2016 08:37 PM, Paul Eggert wrote: > On 11/29/2016 10:11 AM, Florian Weimer wrote: >> How are you going to avoid interposing the implementation in glibc? > > That will depend on how glibc is implemented. Simplest might be to use > compile-time interposition, which is common in Gnulib. E.g., Gnulib > would supply its own getentropy.h which does "#define getentropy > rpl_getentropy", so that Gnulib-using applications always call > rpl_getentropy from the linker's point of view. This sort of thing is OK > for Gnulib, as it's not intended to be a separate library. This sounds reasonable, thanks. Florian
Add getrandom system call and <sys/random.h> header file [BZ #17252] 2016-11-14 Florian Weimer <fweimer@redhat.com> [BZ #17252] * stdlib/sys/random.h: New file. (headers): Add it. * stdlib/Makefile (routines): Add getrandom. (tests): Add tst-getrandom. * stdlib/Versions (GLIBC_2.25): Add getrandom, __libc_getrandom. * stdlib/getrandom.c: New file. * stdlib/tst-getrandom.c: Likewise. * manual/crypt.texi (Unpredictable Bytes): New section. * manual/math.texi (Pseudo-Random Numbers): Add cross-reference. * sysdeps/arm/nacl/libc.abilist: Add __libc_getrandom, getrandom. * sysdeps/unix/sysv/linux/aarch64/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/alpha/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/arm/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/hppa/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/i386/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/ia64/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/microblaze/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/nios2/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist: Likewise. * sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/sh/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/x86_64/64/libc.abilist: Likewise. * sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist: Likewise. diff --git a/NEWS b/NEWS index 65184b1..40f3ce4 100644 --- a/NEWS +++ b/NEWS @@ -67,6 +67,8 @@ Version 2.25 * The functions strfromd, strfromf, and strfroml, from ISO/IEC TS 18661-1:2014, are added to libc. They convert a floating-point number into string. +* The getrandom function and the <sys/random.h> header file have been added. + * The <sys/quota.h> header now includes the <linux/quota.h> header. Support for the Linux quota interface which predates kernel version 2.4.22 has been removed. diff --git a/manual/crypt.texi b/manual/crypt.texi index 9f44740..f99b8be 100644 --- a/manual/crypt.texi +++ b/manual/crypt.texi @@ -45,6 +45,7 @@ encrypted authentication use normal DES. * getpass:: Prompting the user for a password. * crypt:: A one-way function for passwords. * DES Encryption:: Routines for DES encryption. +* Unpredictable Bytes:: Randomness for cryptography purposes. @end menu @node Legal Problems @@ -428,3 +429,69 @@ each byte. The @code{ecb_crypt}, @code{cbc_crypt}, and @code{des_setparity} functions and their accompanying macros are all defined in the header @file{rpc/des_crypt.h}. + +@node Unpredictable Bytes +@section Generating Unpredictable Bytes + +Some cryptographic applications (such as session key generation) need +unpredictable bytes. + +@comment sys/random.h +@comment GNU +@deftypefun ssize_t getrandom (void *@var{buffer}, size_t @var{length}, unsigned int @var{flags}) +@safety{@mtsafe{}@assafe{}@acsafe{}} + +This function writes @var{length} bytes of random data to the array +starting at @var{buffer}. On success, this function returns the number +of bytes which have been written to the buffer (which can be less than +@var{length}). On error, @code{-1} is returned, and @code{errno} is +updated accordingly. + +The @code{getrandom} function is declared in the header file +@file{sys/random.h}. It is a GNU extension. + +The following flags are defined for the @var{flags} argument: + +@table @code +@item GRND_RANDOM +Use the blocking pool instead of the non-blocking pool to obtain +randomness. By default, the non-blocking pool is used. The blocking +pool corresponds to @file{/dev/random}, and the non-blocking pool to +@file{/dev/urandom}. + +@item GRND_NONBLOCK +Instead of blocking, return to the caller immediately if no data is +available. +@end table + +The @code{getrandom} function is a cancellation point. + +Even access to the non-blocking pool can block if the system has just +booted and the pool has not yet been initialized. + +The @code{getrandom} function can fail with several errors, some of +which are listed below. In addition, the function may not fill the +buffer completely and return a value less than @var{length}. + +@table @code +@item ENOSYS +The kernel does not implement the @code{getrandom} system call. + +@item EAGAIN +No random data was available and @code{GRND_NONBLOCK} was specified in +@var{flags}. + +@item EFAULT +The combination of @var{buffer} and @var{length} arguments specifies +an invalid memory range. + +@item EINTR +The system call was interrupted. During the system boot process, before +the kernel randomness pool is initialized, this can happen even if +@var{flags} is zero. + +@item EINVAL +The @var{flags} argument contains an invalid combination of flags. +@end table + +@end deftypefun diff --git a/manual/math.texi b/manual/math.texi index b4bb323..6c0d460 100644 --- a/manual/math.texi +++ b/manual/math.texi @@ -1414,7 +1414,8 @@ is convenient when you are debugging a program, but it is unhelpful if you want the program to behave unpredictably. If you want a different pseudo-random series each time your program runs, you must specify a different seed each time. For ordinary purposes, basing the seed on the -current time works well. +current time works well. For random numbers in cryptography, +@pxref{Unpredictable Bytes}. You can obtain repeatable sequences of numbers on a particular machine type by specifying the same initial seed value for the random number diff --git a/stdlib/Makefile b/stdlib/Makefile index 3cce9d9..6c4ed22 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -28,7 +28,7 @@ headers := stdlib.h bits/stdlib.h bits/stdlib-ldbl.h bits/stdlib-float.h \ errno.h sys/errno.h bits/errno.h \ ucontext.h sys/ucontext.h \ alloca.h fmtmsg.h \ - bits/stdlib-bsearch.h + bits/stdlib-bsearch.h sys/random.h routines := \ atof atoi atol atoll \ @@ -45,7 +45,7 @@ routines := \ srand48 seed48 lcong48 \ drand48_r erand48_r lrand48_r nrand48_r mrand48_r jrand48_r \ srand48_r seed48_r lcong48_r \ - drand48-iter \ + drand48-iter getrandom \ strfromf strfromd strfroml \ strtol strtoul strtoll strtoull \ strtol_l strtoul_l strtoll_l strtoull_l \ @@ -79,7 +79,8 @@ tests := tst-strtol tst-strtod testmb testrand testsort testdiv \ tst-setcontext3 tst-tls-atexit-nodelete \ tst-strtol-locale tst-strtod-nan-locale tst-strfmon_l \ tst-quick_exit tst-thread-quick_exit tst-width \ - tst-width-stdint tst-strfrom tst-strfrom-locale + tst-width-stdint tst-strfrom tst-strfrom-locale \ + tst-getrandom tests-static := tst-secure-getenv ifeq ($(have-cxx-thread_local),yes) CFLAGS-tst-quick_exit.o = -std=c++11 diff --git a/stdlib/Versions b/stdlib/Versions index 54416b7..15905cf 100644 --- a/stdlib/Versions +++ b/stdlib/Versions @@ -115,6 +115,7 @@ libc { GLIBC_2.25 { # s* strfromd; strfromf; strfroml; + getrandom; __libc_getrandom; } GLIBC_PRIVATE { # functions which have an additional interface since they are diff --git a/stdlib/getrandom.c b/stdlib/getrandom.c new file mode 100644 index 0000000..a7288eb --- /dev/null +++ b/stdlib/getrandom.c @@ -0,0 +1,33 @@ +/* Stub for getrandom. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <sys/random.h> + +/* Write LENGTH bytes of randomness starting at BUFFER. Returns the + number of bytes written, or -1 on error. */ +ssize_t +__libc_getrandom (void *buffer, size_t length, unsigned int flags) +{ + __set_errno (ENOSYS); + return -1; +} + +stub_warning (__libc_getrandom) + +weak_alias (__libc_getrandom, getrandom) diff --git a/stdlib/sys/random.h b/stdlib/sys/random.h new file mode 100644 index 0000000..4d2874f --- /dev/null +++ b/stdlib/sys/random.h @@ -0,0 +1,51 @@ +/* Interfaces for obtaining random bytes. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#ifndef _SYS_RANDOM_H +#define _SYS_RANDOM_H 1 + +#include <features.h> +#include <sys/types.h> + +/* Flags for use with getrandom. */ +#define GRND_NONBLOCK 1 +#define GRND_RANDOM 2 + +__BEGIN_DECLS + +/* Write LENGTH bytes of randomness starting at BUFFER. Returns the + number of bytes written, or -1 on error. */ +#ifdef __REDIRECT +/* For GNU compilers: Redirect getrandom to __libc_getrandom, to + protect against accidental interposition. (Application code should + still use the getrandom symbol.) */ +ssize_t __libc_getrandom (void *__buffer, size_t __length, + unsigned int __flags) __wur; +extern ssize_t __REDIRECT (getrandom, + (void *__buffer, size_t __length, + unsigned int __flags), + __libc_getrandom); +#else +/* Non-GNU compilers call getrandom instead. */ +ssize_t getrandom (void *__buffer, size_t __length, unsigned int __flags) + __wur; +#endif /* __REDIRECT */ + +__END_DECLS + +#endif /* _SYS_RANDOM_H */ diff --git a/stdlib/tst-getrandom.c b/stdlib/tst-getrandom.c new file mode 100644 index 0000000..d8816b8 --- /dev/null +++ b/stdlib/tst-getrandom.c @@ -0,0 +1,166 @@ +/* Tests for the getrandom function. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <stdbool.h> +#include <stdio.h> +#include <string.h> +#include <sys/random.h> + +/* Set to true if any errors are encountered. */ +static bool errors; + +/* Test getrandom with a single buffer length. */ +static void +test_length (char *buffer, size_t length, unsigned int flags) +{ + memset (buffer, 0, length); + strcpy (buffer + length, "123"); + ssize_t ret = getrandom (buffer, length, flags); + if (ret < 0) + { + if (!((flags & GRND_RANDOM) + && (flags & GRND_NONBLOCK) + && errno != EAGAIN)) + { + printf ("error: getrandom (%zu, 0x%x): %m\n", length, flags); + errors = true; + } + } + if (ret != length) + { + if (flags & GRND_RANDOM) + { + if (ret == 0 || ret > length) + { + printf ("error: getrandom (%zu, 0x%x) returned %zd\n", + length, flags, ret); + errors = true; + } + } + else + { + printf ("error: getrandom (%zu, 0x%x) returned %zd\n", + length, flags, ret); + errors = true; + } + } + if (length >= 7) + { + /* One spurious test failure in 2**56 is sufficiently + unlikely. */ + int non_null = 0; + for (int i = 0; i < length; ++i) + non_null += buffer[i] != 0; + if (non_null == 0) + { + printf ("error: getrandom (%zu, 0x%x) returned all-zero bytes\n", + length, flags); + errors = true; + } + } + if (memcmp (buffer + length, "123", 4) != 0) + { + printf ("error: getrandom (%zu, 0x%x) wrote spurious bytes\n", + length, flags); + errors = true; + } +} + +/* Call getrandom repeatedly to fill the buffer. */ +static bool +getrandom_full (char *buffer, size_t length, unsigned int flags) +{ + char *end = buffer + length; + while (buffer < end) + { + ssize_t ret = getrandom (buffer, end - buffer, flags); + if (ret < 0) + { + printf ("error: getrandom (%zu, 0x%x): %m\n", length, flags); + errors = true; + return false; + } + buffer += ret; + } + + return true; +} + +static void +test_flags (unsigned int flags) +{ + /* Test various lengths, but only for !GRND_RANDOM, to conserve + entropy. */ + { + enum { max_length = 300 }; + char buffer[max_length + 4]; + if (flags & GRND_RANDOM) + test_length (buffer, 0, flags); + else + { + for (int length = 0; length <= 9; ++length) + test_length (buffer, length, flags); + test_length (buffer, 16, flags); + test_length (buffer, max_length, flags); + } + } + + /* Test that getrandom returns different data. */ + if (!(flags & GRND_NONBLOCK)) + { + char buffer1[8]; + memset (buffer1, 0, sizeof (buffer1)); + + char buffer2[8]; + memset (buffer2, 0, sizeof (buffer2)); + + if (getrandom_full (buffer1, sizeof (buffer1), flags) + && getrandom_full (buffer2, sizeof (buffer2), flags)) + { + if (memcmp (buffer1, buffer2, sizeof (buffer1)) == 0) + { + printf ("error: getrandom returns constant value\n"); + errors = true; + } + } + } +} + +static int +do_test (void) +{ + /* Check if getrandom is not supported by this system. */ + if (getrandom (NULL, 0, 0) == -1 && errno == ENOSYS) + return 77; + + for (int use_random = 0; use_random < 2; ++use_random) + for (int use_nonblock = 0; use_nonblock < 2; ++use_nonblock) + { + int flags = 0; + if (use_random) + flags |= GRND_RANDOM; + if (use_nonblock) + flags |= GRND_NONBLOCK; + test_flags (flags); + } + return errors; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/sysdeps/arm/nacl/libc.abilist b/sysdeps/arm/nacl/libc.abilist index 807e43d..f52e7e7 100644 --- a/sysdeps/arm/nacl/libc.abilist +++ b/sysdeps/arm/nacl/libc.abilist @@ -1843,6 +1843,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 gnu_dev_major F GLIBC_2.25 gnu_dev_makedev F GLIBC_2.25 gnu_dev_minor F diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list index 2254c76..79483ea 100644 --- a/sysdeps/unix/syscalls.list +++ b/sysdeps/unix/syscalls.list @@ -100,3 +100,4 @@ utimes - utimes i:sp __utimes utimes vhangup - vhangup i:i vhangup write - write Ci:ibn __libc_write __write write writev - writev Ci:ipi __writev writev +getrandom - getrandom Ci:bni __libc_getrandom getrandom diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist index 77accdf..77a2231 100644 --- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist +++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist @@ -2090,6 +2090,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist index 659b7fc..922e7c3 100644 --- a/sysdeps/unix/sysv/linux/alpha/libc.abilist +++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist @@ -2001,6 +2001,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/arm/libc.abilist b/sysdeps/unix/sysv/linux/arm/libc.abilist index 8bc979a..7831eb2 100644 --- a/sysdeps/unix/sysv/linux/arm/libc.abilist +++ b/sysdeps/unix/sysv/linux/arm/libc.abilist @@ -91,6 +91,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist index 299b705..f6623b7 100644 --- a/sysdeps/unix/sysv/linux/hppa/libc.abilist +++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist @@ -1855,6 +1855,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist index f00345f..ef04a40 100644 --- a/sysdeps/unix/sysv/linux/i386/libc.abilist +++ b/sysdeps/unix/sysv/linux/i386/libc.abilist @@ -2013,6 +2013,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist index e5fcf88..37dde9d 100644 --- a/sysdeps/unix/sysv/linux/ia64/libc.abilist +++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist @@ -1877,6 +1877,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist index 8f382f6..b236ba8 100644 --- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist +++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist @@ -92,6 +92,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist index 320b7fe..0983296 100644 --- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist +++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist @@ -1969,6 +1969,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/microblaze/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/libc.abilist index 21b1426..6cd5093 100644 --- a/sysdeps/unix/sysv/linux/microblaze/libc.abilist +++ b/sysdeps/unix/sysv/linux/microblaze/libc.abilist @@ -2090,6 +2090,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist index 5c4b596..67c0ce0 100644 --- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist +++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist @@ -1944,6 +1944,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist index 001fa6c..88b9f5f 100644 --- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist +++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist @@ -1942,6 +1942,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist index 2d87001..b2bfc81 100644 --- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist +++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist @@ -1940,6 +1940,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist index aa1ee66..2cb3e46 100644 --- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist +++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist @@ -1935,6 +1935,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist index 2471d68..bd3db24 100644 --- a/sysdeps/unix/sysv/linux/nios2/libc.abilist +++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist @@ -2131,6 +2131,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist index 4b0cde8..317c8ba 100644 --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist @@ -1973,6 +1973,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist index 0557c16..0774b80 100644 --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist @@ -1978,6 +1978,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist index 821384e..174f8b2 100644 --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist @@ -2178,6 +2178,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist index c40a3f1..b364cae 100644 --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist @@ -92,6 +92,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist index 5b39a60..66168b9 100644 --- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist +++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist @@ -1973,6 +1973,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist index a9db32f..0e99054 100644 --- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist +++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist @@ -1874,6 +1874,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/sh/libc.abilist b/sysdeps/unix/sysv/linux/sh/libc.abilist index 294af0a..c47d2ac 100644 --- a/sysdeps/unix/sysv/linux/sh/libc.abilist +++ b/sysdeps/unix/sysv/linux/sh/libc.abilist @@ -1859,6 +1859,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist index 32747bd..923e598 100644 --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist @@ -1965,6 +1965,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist index b0ac4d4..836dabb 100644 --- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist @@ -1903,6 +1903,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist index 4d92d81..2f7d425 100644 --- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist +++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist @@ -2097,6 +2097,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist index a68aef7..5a240a4 100644 --- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist +++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist @@ -2097,6 +2097,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist index 4d92d81..2f7d425 100644 --- a/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist +++ b/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist @@ -2097,6 +2097,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist index b8623fc..aa57670 100644 --- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist +++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist @@ -1854,6 +1854,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist index a61d874..1a7bcea 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist +++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist @@ -2097,6 +2097,8 @@ GLIBC_2.23 fts64_set F GLIBC_2.24 GLIBC_2.24 A GLIBC_2.24 quick_exit F GLIBC_2.25 GLIBC_2.25 A +GLIBC_2.25 __libc_getrandom F +GLIBC_2.25 getrandom F GLIBC_2.25 strfromd F GLIBC_2.25 strfromf F GLIBC_2.25 strfroml F