On Thu, Jun 12, 2014 at 08:53:38PM +0200, Michael Kerrisk (man-pages) wrote: > Another ping.... Sorry for not getting back to you on this! > >>> Here is a proposed alternate text that was recommended to me by a user > >>> on Stack Overflow: > >> > >> It'd be great to add URLs when citing such discussions... With some > >> effort, I found > >> http://stackoverflow.com/questions/23440216/race-condition-when-using-dup > >> > >>> A careful programmer will first dup() the target descriptor, then > >>> use dup2()/dup3() to replace the target descriptor atomically, and > >>> finally close the initially duplicated target descriptor. This > >>> replaces the target descriptor atomically, but also retains a > >>> duplicate for closing so that close-time errors may be checked > >>> for. (In Linux, close() should only be called once, as the > >>> referred to descriptor is always closed, even in case of > >>> errno==EINTR.) > >>> > >>> I'm not sure this is the best wording, since it suggests doing a lot > >>> of work that's likely overkill (and useless in the case where the > >>> target descriptor was read-only, for instance). I might balance such > >>> text with a warning that it's an error to use dup2 or dup3 when the > >>> target descriptor is not known to be open unless you know the code > >>> will only be used in single-threaded programs. And I'm a little bit > >>> hesitant on the parenthetical text about close() since the behavior > >>> it's documenting is contrary to the requirements of the upcoming issue > >>> 8 of POSIX, > >> > >> Again citing the Issue 8 discussion would be helpful. Could > >> you tell me where it is? (It could be useful for the change log.) See the issue that added all the new atomic close-on-exec operations: http://austingroupbugs.net/view.php?id=411 The resolution of 0000149 documented that using close( ) on arbitrary file descriptors is unsafe, and that applications should instead atomically create file descriptors with FD_CLOEXEC already set. [...] And the issue referenced there: http://austingroupbugs.net/view.php?id=149 > >>> and rather irrelevant since EINTR cannot happen in Linux's > >>> close() except with custom device drivers anyway. > >> > >> So, how about something like the following (code not > >> compile-tested...): > >> > >> If newfd was open, any errors that would have been reported at > >> close(2) time are lost. If this is of concern, then—unless the > >> program is single-threaded and does not allocate file descrip‐ > >> tors in signal handlers—the correct approach is not to close > >> newfd before calling dup2(), because of the race condition > >> described above. Instead, code something like the following > >> could be used: > >> > >> /* Obtain a duplicate of 'newfd' that can subsequently > >> be used to check for close() errors; an EBADF error > >> means that 'newfd' was not open. */ > >> > >> tmpfd = dup(newfd); > >> if (tmpfd == -1 && errno != EBADF) { > >> /* Handle unexpected dup() error */ > >> } > >> > >> /* Atomically duplicate 'oldfd' on 'newfd' */ > >> > >> if (dup2(oldfd, newfd) == -1) { > >> /* Handle dup2() error */ > >> } > >> > >> /* Now check for close() errors on the file originally > >> referred to by 'newfd' */ > >> > >> if (tmpfd != -1) { > >> if (close(tmpfd) == -1) { > >> /* Handle errors from close */ > >> } > >> } This code looks like it works, but it's a lot to be recommending without a really good reason to do so. I seem to remember someone claiming that the whole "need" to check for error on close is outdated and not applicable to modern Linux (even with NFS?) but I can't remember where. Anyway I think such code should be accompanied by a discussion of why one might care about checking for close errors so programmers can make an informed decision about whether it's worth the trouble rather than cargo-culting it. Rich -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
