Re: [PATCH 2/4] nptl: Handle EPIPE on tst-cancel2

[Rich Felker <dalias at libc dot org>]

On Fri, Aug 23, 2019 at 02:47:39PM +0200, Florian Weimer wrote:
> I looked at this some more.  It seems that in theory, we could also see
> EPIPE in the old implementation or at least observe execution of a
> signal handler, and the asynchronous cancel hides that.
> 
> In the new implementation, if there is a signal handler and a signal for
> it is delivered before SIGCANCEL (even if the signal was sent *after*
> pthread_cancel, from the same thread), I do not think there is any
> chance whatsoever that we can hide the behavior difference.  After all,
> SIGCANCEL may not trigger an asynchronous cancellation from the signal
> handler.
> 
> System calls that are cancellation points appear to fall into these
> categories:
> 
> (A) Those that do not perform any resource allocation (write, read).
> 
> (B) Those that perform resource allocation, but the allocation can be
>     easily reverted (openat, accept4).
> 
> (C) Those that perform resource allocation, but the allocation is
>     difficult to undo (recvmsg with descriptor passing).
> 
> (D) close.
> 
> For (A), I think POSIX allows sufficient wiggle room so that we can
> exhibit a partial side effect and still act on the cancellation (without
> reporting a partial write first to the application).

No. POSIX is very clear that cancellation is only permitted to be
acted on when the side effects are the same as EINTR, which can only
happen for read/write when no data has been transferred yet. If data
has already been transferred, the only option not ruled out by one or
more constraints is to return successfully with a short read or write;
pending cancellation will then be handled on a subsequent call to a
cancellation point. (If you're doing a retry loop until the total
length n is transferred, then in the next iteration of the loop.)

Of course this case of non-conformance would be less severe than UAF
or resource leak errors in most cases, but it's still not good.

> For (B), maybe we should undo the resource allocation and then proceed
> to act on the cancellation.

No, why would you do that? In general it's not possible (think of
things like O_TRUNC), and even if it were it's more work. You just
don't act on the cancellation if success has already happened.

I don't see how/why you're characterizing "accept4" as "can easily be
reverted". Closing the accepted socket would *drop a connection*
rather than leaving the connection pending for another thread calling
accept to receive.

> For (C), the complexity may not be worth it.
> 
> For (A), (B), (C), we can act on the cancellation in the error case,
> after we observe the cancellation flag in the signal handler trampoline.
> (I think dropping the EINTR restriction from there achieves that.)  If
> we do that, we do not need to change the test case.
> 
> (D) is very special.  Ideally, we would specify what happens with the
> descriptor if the close call is canceled.  POSIX does not even specify
> what the state of file descriptor is after an EINTR error, so it doesn't
> say what happens with cancellation, either.  Maybe we have to leave that
> undefined.

Failure to specify EINTR was an error in POSIX that's been fixed for
the next issue. Leaving it undefined is atrociously bad and results in
UAF type errors. Even if glibc doesn't want to apply the fix for
EINTR, cancellation of close absolutely needs to behave *as if* by the
newly-specified behavior for close with EINTR.

Rich