diff options
| -rw-r--r-- | ChangeLog | 11 | ||||
| -rw-r--r-- | libio/freopen.c | 22 | ||||
| -rw-r--r-- | libio/freopen64.c | 22 | ||||
| -rw-r--r-- | libio/tst-freopen.c | 107 | ||||
| -rw-r--r-- | manual/stdio.texi | 10 |
5 files changed, 113 insertions, 59 deletions
diff --git a/ChangeLog b/ChangeLog index 4ce26de40b..dde43e08f5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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): diff --git a/libio/freopen.c b/libio/freopen.c index ad1c848877..980523af48 100644 --- a/libio/freopen.c +++ b/libio/freopen.c @@ -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); 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); diff --git a/libio/tst-freopen.c b/libio/tst-freopen.c index 5f531e3940..5ad589bffa 100644 --- a/libio/tst-freopen.c +++ b/libio/tst-freopen.c @@ -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); +} + +/* 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 (); - unlink (name); - exit (0); + return 0; } -#define TEST_FUNCTION do_test () -#include "../test-skeleton.c" +#include <support/test-driver.c> diff --git a/manual/stdio.texi b/manual/stdio.texi index dbb21ca4a9..29f3fed89b 100644 --- a/manual/stdio.texi +++ b/manual/stdio.texi @@ -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 |