[PATCH v3] getrandom system call wrapper [BZ #17252]

[Torvald Riegel]

On Mon, 2016-10-03 at 13:51 -0400, Carlos O'Donell wrote:
> On 09/09/2016 10:21 AM, Torvald Riegel wrote:
> > On Thu, 2016-09-08 at 13:44 +0200, Florian Weimer wrote:
> >> I have made the system call wrapper a cancellation point.  (If we 
> >> implement the simpler getentropy interface, it would not be a 
> >> cancellation point.)
> > 
> > Why did you do that?  Even though the system call is new, and thus can't
> > have been used in existing code directly, making it a cancellation point
> > will make all callers cancellation points too.  Therefore, for example,
> > we couldn't use it in the implementation of any POSIX functions (that
> > are not cancellation points) in glibc without having to disable and
> > restore the cancellation state around it every time.
> > It might be even more convenient to have one wrapper that is a
> > cancellation point and one that is not.
> > 
> > Can't we just let cancellation rot in its corner?
>  
> I will second what Florian has suggested, I also think getrandom() should
> be a cancellation point.
> 
> * I see getrandom() as more like a read() and read() is cancellation point.
>   The argument that the functions can block indefinitely makes them prime
>   candidates for cancellation.

I agree that cancellation can make sense for them (assuming one wants
cancellation ;) ).

> The fact that rwlock-related, or in fact all
>   pthread-related functions are not cancellation points

Some are cancellation points.

> is not a great
>   comparison because threads are sufficiently complex that their cancellation
>   is generally a combination of explicit cancellation checks and complex cancel
>   handlers rather than pthread routines being cancellation points (too simple a model).

I don't quite understand this part of the sentence.

> * Application authors that do not want to think about cancellation should disable it
>   e.g. pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL), and libraries that
>   enable it should only cancel _their own_ threads which are cancellation aware.
>   This speaks broadly to Torvald's concerns about user code being canceled by
>   accident. I would much rather see glibc set cancellation to disabled by default
>   instead of the POSIX mandated 'enabled and deferred' by default, and document
>   the difference from POSIX (the gun should be unloaded and in a locked box).

That would be good, but I guess we can't do it without potentially
breaking existing programs?

> * Internal use of a non-cancellating version is done easily by calling
>   __getrandom_nocancel() (As noted by Florian).

I'm not that much concerned about internal use.  It was just an example.
The external uses concern me.

> * Making it a cancellation point that is dependent on input flags e.g.
>   non-blocking, will complicate the simple wrapper and make it more
>   difficult to explain to users since it can no longer go on a simple
>   list of cancellation points.

While that is true, I think it rather hides the complexity.  Who really
looks at the list of cancellation points?

>From a birds-eye view, I think it would have been better if users would
explicitly enable cancellation points, instead of vice versa (see the
default of enabled+deferred you mention).  Like we do for timeouts, for
example.
However, POSIX doesn't specify that.  I'd prefer if we would not add
more of that in the form of more cancellation points by default, but I
also can't argue that what you prefer would be inconsistent with POSIX.

Given that, and that a majority seems to share your opinion, I guess we
have consensus.