Re: [PATCH] Manual should discourage mixing TEMP_FAILURE_RETRY and close

["Michael Kerrisk (man-pages)" <mtk dot manpages at gmail dot com>]

Hello Rich,

On 5 December 2016 at 17:50, Rich Felker <dalias@aerifal.cx> wrote:
> On Mon, Dec 05, 2016 at 05:40:48PM +0100, Michael Kerrisk (man-pages) wrote:
>> Hello Rich
>>
>> On 5 December 2016 at 17:34, Rich Felker <dalias@aerifal.cx> wrote:
>> > On Mon, Dec 05, 2016 at 05:27:23PM +0100, Michael Kerrisk wrote:
>> >> On Mon, Dec 5, 2016 at 5:22 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> >> > On Mon, Dec 05, 2016 at 05:18:28PM +0100, Michael Kerrisk wrote:

[...]

>> >> So, the summary looks like this, AFAICT. Current POSIX permits the
>> >> Linux behavior. Future POSIX proposes to make a change that
>> >> contradicts the behavior of at least three existing implementations
>> >> (Linux, FreeBSD, and AIX). That doesn't seem right. What have I
>> >> missed?
>> >
>> > The fact that these existing implementations were inconsistent with
>> > what EINTR means everywhere else ("no significant side effect has
>> > occurred yet from the call; if there are significant side effects,
>> > e.g. partial write, semaphore decremented, etc. the call returns the
>> > proper success value") and incompatible with the requirement that the
>> > side effects of a cancellation point on thread cancellation are
>> > required to be the same (almost-nothing) as the side effects on EINTR.
>> > In order to have adopted the Linux kernel behavior of close, POSIX
>> > would have had to special-case cancellation behavior of close() in a
>> > way that differs from every other cancellation point, and would have
>> > had to contradict the historical behavior of other implementations
>> > that actually got this (close/EINTR) right.
>>
>> I didn't miss that ;-). I understand that in most other places, EINTR
>> means something else. But already, POSIX is special casing close().
>> This is not "Linux kernel behavior"; this is "Linux kernel and free
>> BSD kernel and AIX kernel behavior. Surely, POSIX.1 won't just by fiat
>> say that the existing implementations are broken?
>
> Do you have another proposed way out?

Perhaps: leave the current close() specification as is, and mandate
only the new posix_close() with the desired behavior?

> It was a bug that they ever
> allowed ambiguity;

Well, inasmuch as POSIX.1 generally standardizes existing behavior,
the initial bug was the failure to account for existing implementation
behavior of close()...

> before POSIX had threads it didn't matter (well,
> not too much; it could still be an issue with signal handlers!), but
> after threads were added, it became unsafe to have any ambiguity
> whether a file descriptor remains open, because closing it again
> results in an extremely-dangerous double-close race -- these can lead
> to file corruption, file contents disclosure, and even arbitrary code
> execution. The only way to avoid that would be to live with possible
> fd leaks, which result in eventual fd exhaustion.

Well, the other way to do it would be to do what Linux and FreeBSD do:
always close the FD, even when returning an error. But, I take it you
mean what the standard should provide as a guarantee to portable
applications. Ambiguity there is obviously not helpful.

> Fortunately nobody is asking Linux, FreeBSD, and AIX to change a
> feature that people actually want, or in an incompatible way. EINTR in
> close is very rare (only possible with tape drives, other weird
> devices, and perhaps some NFS configurations), and when it does
> happen, it's unwanted anyway. Changing in an incompatible way --
> returning EINTR with the fd still open -- would be one option, but
> it's not a good one. Instead they should just return EINPROGRESS or
> better yet 0.

Of the options you list, 0 does seem the best bet.

> As discussed on the glibc bug tracker issue, 0 is
> probably best to avoid affecting applications that might treat unknown
> results as a write error at close time.
>
> There is perhaps one other course of action that POSIX could have
> taken: defining a macro, or a sysconf() value, to query whether the
> implementation has left the fd open when close returns with EINTR. I
> think this is a much worse solution, but at least it would have made
> it possible to write safe applications.

Not so pretty, I agree.

So, from what I understand, there's a future path for implementations
and applications. But, some existing applications (ones developed on
Linux/FreeBSD/AIX that know not to retry the close() on EINTR) will be
declared broken. That doesn't seem good. Or do I still miss something?

Also, there is a wider problem here, it seems to me. It is not just
EINTR that is being special cased in close(). Linux and FreeBSD [1],
and to some extent, AIX [2] special case *all* errors from close().
For Linux and FreeBSD, the FD must be considered closed for any error
that is returned. AIX has the following text:

    If the FileDescriptor parameter refers to a device and the
    close subroutine actually results in a device close, and the
    device close routine returns an error, the error is returned
    to the application. However, the FileDescriptor parameter is
    considered closed and it may not be used in any subsequent
    calls.

(The AIX man page documents only the errors EBADF and EINTR, but that
text hints to me that other errors are also possible.)

Also, I took a look at the illumos code base, which presumably does
not differ greatly from historical Solaris on this point. It looks to
me (closef(), closeandsetf()) that also on that system there are paths
in close() where errors are returned but the FD is nevertheless
closed. So it really does seem that POSIX.1 was specifying (and plans
to again specify) behavior that is inconsistent with actual
implementations.

Thanks for your explanations, Rich.

Cheers,

Michael

[1] https://www.freebsd.org/cgi/man.cgi?query=close&apropos=0&sektion=0&manpath=FreeBSD+10.3-RELEASE+and+Ports&arch=default&format=html#ERRORS
[2] http://publib16.boulder.ibm.com/doc_link/en_US/a_doc_lib/libs/basetrf1/close.htm

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/