libio: Fix race in _IO_new_file_init_internal initialization order [BZ #33785]

_IO_new_file_init_internal linked the new stream into _IO_list_all
before setting fp->_fileno to -1.  A concurrent thread that walks
_IO_list_all (for example via fflush (NULL)) could observe the stream
with an uninitialized _fileno before initialization completed.

Set _fileno = -1 before _IO_link_in so the stream is fully
initialized when it becomes visible in the global list.

This is the residual concurrency defect noted at the end of commit
b657f72fa3 ("libio: Fix deadlock between freopen, fflush (NULL) and
fclose (bug 24963)").

Add libio/tst-file-init-race exercising concurrent fopen/fclose and
fflush (NULL) to detect regressions.

Signed-off-by: Shamil Abdulaev <ashamil435@gmail.com>
Reviewed-by: Florian Weimer <fweimer@redhat.com>

diff --git a/libio/Makefile b/libio/Makefile
index 6e0627bb88387740ba1cfbc2e41577fc8d129fa9..684697fb9e5349401a9bcb1c24e59123282de995 100644 (file)
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -108,6 +108,7 @@ tests = \
   tst-fgetc-after-eof \
   tst-fgetwc \
   tst-fgetws \
+  tst-file-init-race \
   tst-fopenloc2 \
   tst-fputws \
   tst-freopen \
@@ -161,6 +162,8 @@ tests-static += \
 
 $(objpfx)tst-popen-fork: $(shared-thread-library)
 
+$(objpfx)tst-file-init-race: $(shared-thread-library)
+
 tests-internal = tst-vtables tst-vtables-interposed
 
 ifeq (yes,$(build-shared))

diff --git a/libio/fileops.c b/libio/fileops.c
index 8067c0a9cfef1efc013d772d0549059844a862d6..9348d7c3a1fd2434ab7e5e0b25201df54fd01411 100644 (file)
--- a/libio/fileops.c
+++ b/libio/fileops.c
@@ -111,8 +111,8 @@ _IO_new_file_init_internal (struct _IO_FILE_plus *fp)
   fp->file._offset = _IO_pos_BAD;
   fp->file._flags |= CLOSED_FILEBUF_FLAGS;
 
-  _IO_link_in (fp);
   fp->file._fileno = -1;
+  _IO_link_in (fp);
 }
 
 /* External version of _IO_new_file_init_internal which switches off

diff --git a/libio/oldfileops.c b/libio/oldfileops.c
index 41b15491cb64ba4c0eaa33093b481aa387f25856..0b92afbdb135692b94e2ebf60ceaa258e75eae5b 100644 (file)
--- a/libio/oldfileops.c
+++ b/libio/oldfileops.c
@@ -108,8 +108,8 @@ _IO_old_file_init_internal (struct _IO_FILE_plus *fp)
      _IO_vtable_offset is used to detect the old binaries.  */
   fp->file._vtable_offset = ((int) sizeof (struct _IO_FILE)
                             - (int) sizeof (struct _IO_FILE_complete));
-  _IO_link_in (fp);
   fp->file._fileno = -1;
+  _IO_link_in (fp);
 
   if (&_IO_stdin_used != NULL || !_IO_legacy_file ((FILE *) fp))
     /* The object is dynamically allocated and large enough.  Initialize

diff --git a/libio/tst-file-init-race.c b/libio/tst-file-init-race.c
new file mode 100644 (file)
index 0000000..643075f
--- /dev/null
+++ b/libio/tst-file-init-race.c
@@ -0,0 +1,69 @@
+/* Test for race during FILE initialization in _IO_new_file_init_internal.
+   Copyright The GNU Toolchain Authors.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <stdatomic.h>
+#include <pthread.h>
+#include <time.h>
+
+#include <support/check.h>
+#include <support/xthread.h>
+
+static atomic_bool stop = ATOMIC_VAR_INIT (0);
+
+static void *
+opener_thread (__attribute__ ((unused)) void *arg)
+{
+  while (!atomic_load_explicit (&stop, memory_order_acquire))
+    {
+      FILE *fp = fopen ("/dev/null", "r");
+      if (fp == NULL)
+        FAIL_EXIT1 ("fopen: %m");
+      if (fclose (fp) != 0)
+        FAIL_EXIT1 ("fclose: %m");
+    }
+  return NULL;
+}
+
+static void *
+flusher_thread (__attribute__ ((unused)) void *arg)
+{
+  while (!atomic_load_explicit (&stop, memory_order_acquire))
+    fflush (NULL);
+  return NULL;
+}
+
+static int
+do_test (void)
+{
+  pthread_t t1 = xpthread_create (NULL, opener_thread, NULL);
+  pthread_t t2 = xpthread_create (NULL, flusher_thread, NULL);
+
+  struct timespec ts = { .tv_sec = 3, .tv_nsec = 0 };
+  nanosleep (&ts, NULL);
+
+  atomic_store_explicit (&stop, 1, memory_order_release);
+
+  xpthread_join (t1);
+  xpthread_join (t2);
+
+  return 0;
+}
+
+#define TIMEOUT 30
+#include <support/test-driver.c>