mirror/glibc
2
0
Fork 0
mirror of git://sourceware.org/git/glibc.git synced 2024-11-27 03:41:23 +08:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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
This commit is contained in:
Adhemerval Zanella 2017-05-05 11:31:38 -03:00
parent d26db8fbb4
commit f1a67a2c78
5 changed files with 114 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
ChangeLog
View File

@ -1,3 +1,14 @@
2017-05-22 Adhemerval Zanella <adhemerval.zanella@linaro.org>
[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.
2017-05-22 Siddhesh Poyarekar <siddhesh@sourceware.org>
* sysdeps/sparc/sparc32/dl-machine.h (elf_machine_matches_host):

22
libio/freopen.c
View File

@ -76,17 +76,31 @@ freopen (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);

22
libio/freopen64.c
View File

@ -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);

109
libio/tst-freopen.c
View File

@ -22,84 +22,91 @@
#include <string.h>
#include <unistd.h>
static int
do_test (void)
#include <support/check.h>
#include <support/temp_file.h>
static int fd;
static char *name;
static void
do_prepare (int argc, char *argv[])
{
fd = create_temp_file ("tst-freopen.", &name);
TEST_VERIFY_EXIT (fd != -1);
}
#define PREPARE do_prepare
/* Basic tests for freopen. */
static void
do_test_basic (void)
{
char name[] = "/tmp/tst-freopen.XXXXXX";
const char * const test = "Let's test freopen.\n";
char temp[strlen (test) + 1];
int fd = mkstemp (name);
FILE *f;
if (fd == -1)
{
printf ("%u: cannot open temporary file: %m\n", __LINE__);
exit (1);
}
f = fdopen (fd, "w");
FILE *f = fdopen (fd, "w");
if (f == NULL)
{
printf ("%u: cannot fdopen temporary file: %m\n", __LINE__);
exit (1);
}
FAIL_EXIT1 ("fdopen: %m");
fputs (test, f);
fclose (f);
f = fopen (name, "r");
if (f == NULL)
{
printf ("%u: cannot fopen temporary file: %m\n", __LINE__);
exit (1);
}
FAIL_EXIT1 ("fopen: %m");
if (fread (temp, 1, strlen (test), f) != strlen (test))
{
printf ("%u: couldn't read the file back: %m\n", __LINE__);
exit (1);
}
FAIL_EXIT1 ("fread: %m");
temp [strlen (test)] = '\0';
if (strcmp (test, temp))
{
printf ("%u: read different string than was written:\n%s%s",
__LINE__, test, temp);
exit (1);
}
FAIL_EXIT1 ("read different string than was written: (%s, %s)",
test, temp);
f = freopen (name, "r+", f);
if (f == NULL)
{
printf ("%u: cannot freopen temporary file: %m\n", __LINE__);
exit (1);
}
FAIL_EXIT1 ("freopen: %m");
if (fseek (f, 0, SEEK_SET) != 0)
{
printf ("%u: couldn't fseek to start: %m\n", __LINE__);
exit (1);
}
FAIL_EXIT1 ("fseek: %m");
if (fread (temp, 1, strlen (test), f) != strlen (test))
{
printf ("%u: couldn't read the file back: %m\n", __LINE__);
exit (1);
}
FAIL_EXIT1 ("fread: %m");
temp [strlen (test)] = '\0';
if (strcmp (test, temp))
{
printf ("%u: read different string than was written:\n%s%s",
__LINE__, test, temp);
exit (1);
}
FAIL_EXIT1 ("read different string than was written: (%s, %s)",
test, temp);
fclose (f);
unlink (name);
exit (0);
}
#define TEST_FUNCTION do_test ()
#include "../test-skeleton.c"
/* Test for BZ#21398, where it tries to freopen stdio after the close
of its file descriptor. */
static void
do_test_bz21398 (void)
{
(void) close (STDIN_FILENO);
FILE *f = freopen (name, "r", stdin);
if (f == NULL)
FAIL_EXIT1 ("freopen: %m");
TEST_VERIFY_EXIT (ferror (f) == 0);
char buf[128];
char *ret = fgets (buf, sizeof (buf), stdin);
TEST_VERIFY_EXIT (ret != NULL);
TEST_VERIFY_EXIT (ferror (f) == 0);
}
static int
do_test (void)
{
do_test_basic ();
do_test_bz21398 ();
return 0;
}
#include <support/test-driver.c>

10
manual/stdio.texi
View File

@ -316,7 +316,15 @@ actually done any output using the stream.) Then the file named by
and associated with the same stream object @var{stream}.
If the operation fails, a null pointer is returned; otherwise,
@code{freopen} returns @var{stream}.
@code{freopen} returns @var{stream}. On Linux, @code{freopen} may also
fail and set @code{errno} to @code{EBUSY} when the kernel structure for
the old file descriptor was not initialized completely before @code{freopen}
was called. This can only happen in multi-threaded programs, when two
threads race to allocate the same file descriptor number. To avoid the
possibility of this race, do not use @code{close} to close the underlying
file descriptor for a @code{FILE}; either use @code{freopen} while the
file is still open, or use @code{open} and then @code{dup2} to install
the new file descriptor.
@code{freopen} has traditionally been used to connect a standard stream
such as @code{stdin} with a file of your own choice. This is useful in