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

[Rich Felker <dalias at aerifal dot cx>]

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:
> >> >> Hi,
> >> >>
> >> >> On Mon, Dec 5, 2016 at 5:11 PM, Rich Felker <dalias@aerifal.cx> wrote:
> >> >> > On Mon, Dec 05, 2016 at 03:48:56PM +0100, Michael Kerrisk wrote:
> >> >> >> For what it's worth, well after the fact...
> >> >> >>
> >> >> >> On Wed, Dec 4, 2013 at 1:08 AM, Ondřej Bílka <neleai@seznam..cz> wrote:
> >> >> >> > On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
> >> >> >> >> When the close system call is interrupted by a signal, the state of its
> >> >> >> >> file descriptor argument is not specified[1]. On Linux, a close that
> >> >> >> >> fails with EINTR must not be restarted because the file descriptor is
> >> >> >> >> guaranteed to have been closed at that point[2]. Note that the kernel
> >> >> >> >> itself never restarts a close when SA_RESTART is set or when no
> >> >> >> >> user-space signal handler is present[3].
> >> >> >> >>
> >> >> >> > This is linux problem, POSIX now says
> >> >> >> >
> >> >> >> > http://austingroupbugs.net/view.php?id=529
> >> >> >> >
> >> >> >> > If close( ) is interrupted by a signal that is to be caught, then it
> >> >> >> > is unspecified whether it returns -1 with errno set to [EINTR] with
> >> >> >> > fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
> >> >> >> > with fildes being closed, or returns 0 to indicate successful
> >> >> >> > completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
> >> >> >> > the option of returning -1 with errno set to [EINTR] and fildes
> >> >> >> > remaining open shall not occur. If close() returns -1 with errno set
> >> >> >> > to [EINTR], it is unspecified whether fildes can subsequently be
> >> >> >> > passed to any function except close( ) or posix_close( ) without error..
> >> >> >> > For all other error situations (except for [EBADF] where fildes was
> >> >> >> > invalid), fildes shall be closed. If fildes was closed even though
> >> >> >> > the close operation is incomplete, the close operation shall continue
> >> >> >> > asynchronously and the process shall have no further ability to track
> >> >> >> > the completion or final status of the close operation.
> >> >> >>
> >> >> >> Yes, but POSIX never said that. That was a proposal that was
> >> >> >> discussed, that subsequently got shot down...
> >> >> >
> >> >> > It was not shot down. It was accepted for inclusion in the next issue
> >> >> > of POSIX. 2016 is not a new issue; it's a TC for POSIX-2008, and
> >> >> > cannot include non-bugfix changes.
> >> >>
> >> >> You're right. I misread the order of entries in bug (was looking at an
> >> >> older proposal that was shot down), and mismatched with what Ondrej
> >> >> quoted.
> >> >>
> >> >> But the point is still the same. Current POSIX.1 permits the Linux
> >> >> behavior, and future POSIX will too. So, the glibc manual still should
> >> >> get Mark's fix, I'd say.
> >> >
> >> > No, POSIX-future does not permit the Linux behavior. If EINTR is
> >> > returned, the fd must still be "open" (not re-assignable, compatible
> >> > with calling close on it again, though it may be in a partially-closed
> >> > and unusable state). If the fd has been deallocated such that it's not
> >> > safe to call close on it again, close must return some value other
> >> > than EINTR; EINPROGRESS is added as the canonical error for the case
> >> > where closing is not complete and happens in the background.
> >>
> >> Yes, you got me. I went away and reread this in the time you wrote the
> >> mail, and came to the same conclusion.
> >>
> >> 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? It was a bug that they ever
allowed ambiguity; 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.

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

Rich