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 2/4] nptl: Handle EPIPE on tst-cancel2


* Rich Felker:

> 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.)

This seems to be a backwards interpretation to me.  I think POSIX wants
to describe what can happen during cancellation, and not say that any
cancellation point can be skipped if some side effect happens that
cannot happen before EINTR.

> 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.

Agreed.  And I think not acting on the cancellation and returning
success is also not conforming.

The main problem is that POSIX would need to specify a memory model in
order to make the current language (“side-effects occur before any
cancellation cleanup handlers are called”, “a thread is canceled while
executing the function”, “a cancellation request has been made”, etc.).

>> 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.

The problem is that the implementation reorders cancellation requests
fairly arbitrarily.  I don't think that's what the authors of POSIX had
in mind when they said that the cancellation itself progresses
asynchronously.  I don't think that can be fixed without kernel support,
though.  We'll see eventually how this impacts existing applications.

> 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.

And this is true for open as well.  For FIFOs, there is an observable
side effect, likewise for rewinding tape devices, and even for regular
files, the close call affects POSIX advisory locks on the file.  Oh
well.  So it's just not possible to unwind directly after file
descriptor allocation.

>> 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.

Can we even make sure we act on the cancellation only if the descriptor
has been closed?

Thanks,
Florian


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