about summary refs log tree commit diff
path: root/libio/freopen64.c
diff options
context:
space:
mode:
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>2017-05-05 11:31:38 -0300
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>2017-05-22 18:13:35 -0300
commitf1a67a2c78601599be51a17250ca02c7d830d79d (patch)
treeac463077dcbc47e15e3b5346c445d9ddd38fcf0d /libio/freopen64.c
parentd26db8fbb4787905590f207d56026e915b3bd5b3 (diff)
downloadglibc-f1a67a2c78601599be51a17250ca02c7d830d79d.tar.gz
glibc-f1a67a2c78601599be51a17250ca02c7d830d79d.tar.xz
glibc-f1a67a2c78601599be51a17250ca02c7d830d79d.zip
libio: Avoid dup already opened file descriptor [BZ#21393]
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.

With this patch the example on BZ#21398 is now actually possible
(I used as the testcase for the bug report).  Checked on
x86_64-linux-gnu.

	[BZ #21393]
	* libio/freopen.c (freopen): Avoid dup already opened file descriptor
	and add a check for dup3 failure.
	* libio/freopen64.c (freopen64): Likewise.
	* libio/tst-freopen.c (do_test): Rename to do_test_basic and use
	libsupport.
	(do_test_bz21398): New test.
	* manual/stdio.texi (freopen): Add documentation of EBUSY failure.

[1] http://stackoverflow.com/questions/23440216/race-condition-when-using-dup2
Diffstat (limited to 'libio/freopen64.c')
-rw-r--r--libio/freopen64.c22
1 files changed, 18 insertions, 4 deletions
diff --git a/libio/freopen64.c b/libio/freopen64.c
index adf749a070..1e56de616c 100644
--- a/libio/freopen64.c
+++ b/libio/freopen64.c
@@ -59,17 +59,31 @@ freopen64 (const char *filename, const char *mode, FILE *fp)
       /* unbound stream orientation */
       result->_mode = 0;
 
-      if (fd != -1)
+      if (fd != -1 && _IO_fileno (result) != fd)
 	{
-	  __dup3 (_IO_fileno (result), fd,
-		  (result->_flags2 & _IO_FLAGS2_CLOEXEC) != 0
-		  ? O_CLOEXEC : 0);
+	  /* At this point we have both file descriptors already allocated,
+	     so __dup3 will not fail with EBADF, EINVAL, or EMFILE.  But
+	     we still need to check for EINVAL and, due Linux internal
+	     implementation, EBUSY.  It is because on how it internally opens
+	     the file by splitting the buffer allocation operation and VFS
+	     opening (a dup operation may run when a file is still pending
+	     'install' on VFS).  */
+	  if (__dup3 (_IO_fileno (result), fd,
+		      (result->_flags2 & _IO_FLAGS2_CLOEXEC) != 0
+		      ? O_CLOEXEC : 0) == -1)
+	    {
+	      _IO_file_close_it (result);
+	      result = NULL;
+	      goto end;
+	    }
 	  __close (_IO_fileno (result));
 	  _IO_fileno (result) = fd;
 	}
     }
   else if (fd != -1)
     __close (fd);
+
+end:
   if (filename == NULL)
     free ((char *) gfilename);
   _IO_release_lock (fp);