[Dnsmasq-discuss] Is wrapping close() in retry_send() required ?

[Pali Rohár]

On Wednesday 27 February 2019 17:07:21 Simon Kelley wrote:
> On 27/02/2019 15:06, Bogdan Harjoc wrote:
> > There are 50 calls to close() in dnsmasq-2.80, and 10 of them are
> > wrapped in retry_send().
> > 
> > "man close" has this paragraph in the section "Dealing with error
> > returns from close":
> > 
> > "Retrying the close() after a failure return is the wrong thing to do,
> > since this may cause a reused file descriptor from another thread to
> > be closed. This can occur because the Linux kernel always releases the
> > file descriptor early in the close operation, freeing it for reuse;
> > the steps that may return an error, such as flushing data to the
> > filesystem or device, occur only later in the close operation".
> > 
> > Dnsmasq is single-threaded so the retry_send() call is probably
> > harmless. Are there other OSes that may return an error before the fd
> > is released, in other words is there an OS where wrapping close in
> > retry_send is required ?
> 
> 
> Good questions.
> 
> Note that retry_send() only deals with EINTR return codes, ie
> interrupted system calls. (Ok there are other return codes in there, but
> nothing which might be returned by close())
> 
> So the use of retry_send(close(...)) is simply to restart the close()
> syscall if a signal arrives during the syscall.
> 
> 
> HOWEVER, whilst the man page for close() on my Linux machine states that
> EINTR is a possible error return, man (7) signal does NOT include
> close() in the set of syscalls which can be interrupted.
> 
> Clearly I was reading the close() man page when I used the wrapper, and
> signal man page when I didn't :)
> 
> 
> You're probably right that it doesn't matter, but it would be nice to
> make this at least consistent.
> 
> Anyone know the answer?

See manpage: http://man7.org/linux/man-pages/man2/close.2.html


    The EINTR error is a somewhat special case.  Regarding the EINTR
    error, POSIX.1-2013 says:

           If close() is interrupted by a signal that is to be caught, it
           shall return -1 with errno set to EINTR and the state of
           fildes is unspecified.

    This permits the behavior that occurs on Linux and many other
    implementations, where, as with other errors that may be reported by
    close(), the file descriptor is guaranteed to be closed.  However, it
    also permits another possibility: that the implementation returns an
    EINTR error and keeps the file descriptor open.  (According to its
    documentation, HP-UX's close() does this.)  The caller must then once
    more use close() to close the file descriptor, to avoid file
    descriptor leaks.  This divergence in implementation behaviors
    provides a difficult hurdle for portable applications, since on many
    implementations, close() must not be called again after an EINTR
    error, and on at least one, close() must be called again.  There are
    plans to address this conundrum for the next major release of the
    POSIX.1 standard.


So retrying close() on EINTR on Linux is an application bug. On the
other hand it is required on HP-UX.

-- 
Pali Rohár
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20190326/b0fcf0ea/attachment.sig>