This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v7] getrandom system call wrapper [BZ #17252]


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.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]