Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140805214827.GO1674@brightrain.aerifal.cx>
Date: Tue, 5 Aug 2014 17:48:27 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: PATCH: don't call cleanup handlers after a regular return
 from the thread start function

On Tue, Aug 05, 2014 at 11:05:41PM +0200, Jens Gustedt wrote:
> Hello,
> 
> Am Dienstag, den 05.08.2014, 15:41 -0400 schrieb Rich Felker:
> > On Tue, Aug 05, 2014 at 09:06:16PM +0200, Jens Gustedt wrote:
> > > But I have another reason for wanting that, future compatibility with
> > > C threads. Programs that are written for C threads will not be aware
> > > of such interdictions. Concretely in our case of my C thread v3 patch
> > > a user can longjmp from a once-init-handler (written by her or him)
> > > through pthread_once (in libc, for musl with pthread_cleanup_push) to
> > > the thread start function (again user code) and then return from
> > > there. (All of this seems to be allowed by POSIX)
> > 
> > Such a longjmp is UB unless it's explicitly permitted by the standard
> > -- same as longjmp out of qsort.
> 
> Which standard are you refering to? I don't find any mention of such a
> thing in the C standard. In the contrary, I find a lot of contexts
> where longjmp is explicitly forbidden, such as at_exit and
> similar. bsearch, qsort and call_once are not among these, and there
> is no general mention of longjmp and "callback" contexts.
> 
> (I just find your second message. I don't find the "shall return" for
> the comparison function as binding to exclude longjmp, for bsearch
> this might even be tempting. For call_once, I agree, it is probably
> lack of consideration.)

How is a "shall return" not binding? It does not say "if the function
returns, the return value shall be..."; it says "shall return". I
would say this means infinite loops in a comparison function are also
invalid, and a compiler doing optimization of qsort inline could
rightfully assume no infinite loops occur in the comparison function.

> > There's no guarantee to a function
> > called as a callback from the standard library that the calling
> > function does not have internal state which would be left in an
> > inconsistent state by longjmp'ing out.
> 
> Don't get me wrong, it isn't a brilliant idea to do longjmp from such
> callbacks, but I don't see that you have an argument here. The
> definition of setjmp/longjmp clearly states which circumstance make it
> UB to use them. For the rest, this *defines* the behavior for all
> other contexts, including callbacks, unless explicitly forbidden.

//

> 
> > If call_once is explicitly required to support longjmp,
> 
> It is not explicitly forbidden and I think this is sufficient to allow
> for it.

Actually I don't think it's possible to implement a call_once that
supports longjmp out without a longjmp which does unwinding, and I
don't think it's the intent of WG14 to require unwinding. The
fundamental problem is the lack of any way to synchronize "all
subsequent calls to the call_once function with the same value of
flag" when the code to update flag to indicate completion of the call
is skipped.

Do you see any solution to this problem? Perhaps a conforming
implementation can just deadlock all subsequent calls, since (1)
calling the function more than once conflicts with the "exactly once"
requirement, and (2) the completion of the first call, which
subsequent calls have to synchronize with, never happens.

> I don't know about the equivalent for the pthread tools, though.

I can't immediately find any information on the subject, so this may
call for a request for interpretation.

> > However I doubt this usage is supported, since
> > the state of the once_flag would have to be specified for such
> > interrupted calls, and I see no such specification.
> > 
> > > I consider to not execute the cleanup handlers a little bit more
> > > friendly than executing them.
> > 
> > musl does not do either. It's simply UB. At the point where you're
> > claiming the cleanup handler is executed, the pointer being
> > dereferenced is to an object whose lifetime has ended, so it's not
> > even making a call to the cleanup handler but simply invoking UB.
> 
> Aren't you playing a bit with words, here? Fact is, that the handler
> might be called in real life. We are not obliged to be nasty when (and
> if!) the user is invoking UB.

No, I'm not. The only way an attempt to make the call happens is if
you leave the block context of a pthread_cleanup_push illegally. It's
explicitly stated that doing so is UB, and there's no easy way to do
it accidentally:

- Except for longjmp, the statement that leaves the block illegally
  would be local to the function that called pthread_cleanup_push, so
  the invalidity of the code is immediately apparent, and happens in a
  context where the programmer knows pthread_cleanup_push is in
  effect.

- In the case of longjmp, it's already dangerous to longjmp across
  arbitrary code. Unless you know what code you're jumping over is
  doing or have a contract that it's valid to jump over it, you can't
  use longjmp.

> For the case of pthread_once or call_once, we could get away with it
> by having the `struct __ptcb` static at that place. The caller of the
> callback holds a lock at the point of call, so there would be no race
> on such a static object.
> 
> I will prepare a patch for call_once to do so, just to be on the safe
> side. This would incur 0 cost.

The cost is encoding knowledge of the cancellation implementation at
the point of the call, and synchronizing all calls to call_once with
respect to each other, even when they don't use the same once_flag
object.

call_once does not have specified behavior if the callback is
cancelled (because, as far as C11 is concerned, cancellation does not
even exist). Thus there is no need for a cleanup function at all. I
think we should wait to do anything unneccessary, ugly, and/or complex
here unless the next issue of POSIX specifies a behavior for this
case. And even then, only if longjmp is allowed, which is unlikely
(see my above analysis).

> The same could be used for pthread_once.

Above analysis about longjmp being impossible to support without
deadlock or unwinding in longjmp also applies to pthread_once. I
suspect, if longjmp is implicitly allowed, this is just an oversight
and one which will be corrected.

> > > Another possibility would be to split the behavior
> > > 
> > >  - abort for pthreads (this is nicer than executing the handlers or
> > >    than silently ignore them)
> > > 
> > >  - ignore for C threads
> > 
> > There's no way it can happen for C threads unless they're actually
> > using pthread_cleanup_push, and the relevenat code that might be
> > returning is aware of POSIX and the reules for use of
> > pthread_cleanup_push.
> 
> I don't think it is as simple as that. From C you might use a third
> party library that on a specific platform uses pthread features under
> the hood. E.g gcc's libatomic is such a candidate, they are using
> pthread_mutex_t without telling you. OpenMP also comes in mind. And
> who knows want the future will bring on such mixed platforms.

We're talking about a return statement in between the
pthread_cleanup_push and pthread_cleanup_pop calls in a particular
block context. Third-party library code does not magically insert such
a statement in your code. So I don't understand the case you have in
mind.

Rich

Powered by blists - more mailing lists

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