Re: [PATCH] libio: Avoid dup already opened file descriptor [BZ#21393]

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 18/05/2017 16:03, Siddhesh Poyarekar wrote:
> On Friday 05 May 2017 08:39 PM, Adhemerval Zanella wrote:
>> As described in BZ#21398 (close as dup of 21393) report current
>> freopen implementation fails when one tries to freopen STDIN_FILENO,
>> STDOUT_FILENO, or STDERR_FILENO.  Although on bug report the
>> discussion leads to argue if a close followed by a freopen on the
>> standard file is a valid operation, the underlying issue is not
>> really the check for dup3 returned value, but rather calling it
>> if the returned file descriptor is equal as the input one.
>>
>> So for a quality of implementation this patch avoid calling dup3
>> for the aforementioned case.  It also adds a dup3 error case check
>> for the two possible failures, with one being Linux only: EINTR and
>> EBUSY.  The EBUSY issue is better explained on this stackoverflow
>> thread [1], but in a short it is due the internal Linux
>> implementation which allows a race condition window for dup2 due
>> the logic dissociation of file descriptor allocation and actual
>> VFS 'install' operation.  For both outliers failures all allocated
>> memory is freed and a NULL FILE* is returned.
> 
> Carlos and I had a private conversation with Mathew Wilcox months ago
> about the EBUSY issue and how it probably doesn't strictly adhere to the
> current wording in POSIX.  The trouble here is that failing due to an
> EBUSY could possibly cause a false negative in a number of cases where
> the backing store (nfs or whatever) just isn't quick enough and
> applications will then have to specifically look out for the EBUSY and
> retry.
> 
> To make the behaviour strictly conforming and clean (and not have
> applications change), we would have to retry repeatedly (maybe a limited
> number of times) until dup3 either succeeds or fails with a different
> error.  However, in the ideal world the kernel ought to do this for us.
> 
> Since 21398 would be solved by just ensuring that the old and new fd are
> different to warrant closing the old fd before associating the new one,
> I would suggest dropping the error check for dup3 for now and then deal
> with the conformity issue separately.
> 
> Carlos, what do you think?
> 
> Siddhesh
> 

I think for BZ#21393 we won't get any sufficient conforming implementation
with proper kernel help or without relaxing the POSIX definition to also
allow this kind of errno.  So I still think that checking for EBUSY and
returning it is still the correct approach.  Specially because for the cases 
where it fails then application might still seeing spurious issue due freopen
returning a valid return code but without dupping correctly the file
descriptor.

Also I think for proper underlying kernel support it would potentially
require dup3 to indefinitely block (which still be even more troublesome due
underlying FS like NFS that might timeout).  I would require to extend dup3
semantic, possible making it a cancelable entrypoint, and imho I think kernel
won't take this path lightly.