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

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

[Shamil Abdulaev]

_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>
---
 libio/Makefile             |  3 ++
 libio/fileops.c            |  2 +-
 libio/tst-file-init-race.c | 66 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 libio/tst-file-init-race.c

diff --git a/libio/Makefile b/libio/Makefile
index 93656466df..7e448295e3 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -107,6 +107,7 @@ tests = \
   tst-fgetc-after-eof \
   tst-fgetwc \
   tst-fgetws \
+  tst-file-init-race \
   tst-fopenloc2 \
   tst-fputws \
   tst-freopen \
@@ -160,6 +161,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 8067c0a9cf..9348d7c3a1 100644
--- 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/tst-file-init-race.c b/libio/tst-file-init-race.c
new file mode 100644
index 0000000000..f47691de94
--- /dev/null
+++ b/libio/tst-file-init-race.c
@@ -0,0 +1,66 @@
+/* Test for race during FILE initialization in _IO_new_file_init_internal.
+   Copyright (C) 2026 Free Software Foundation, Inc.
+   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/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)
+        fclose (fp);
+    }
+  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>
-- 
2.54.0

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

[Florian Weimer]

* Shamil Abdulaev:

> _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>
> ---
>  libio/Makefile             |  3 ++
>  libio/fileops.c            |  2 +-
>  libio/tst-file-init-race.c | 66 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 1 deletion(-)
>  create mode 100644 libio/tst-file-init-race.c
>
> diff --git a/libio/Makefile b/libio/Makefile
> index 93656466df..7e448295e3 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -107,6 +107,7 @@ tests = \
>    tst-fgetc-after-eof \
>    tst-fgetwc \
>    tst-fgetws \
> +  tst-file-init-race \
>    tst-fopenloc2 \
>    tst-fputws \
>    tst-freopen \
> @@ -160,6 +161,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 8067c0a9cf..9348d7c3a1 100644
> --- 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);
>  }

Thanks for investigating this and fixing it.

Would you please apply the parallel change to libio/oldfileops.c?  It
looks like it has the same bug.

Florian

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

[Florian Weimer]

* Shamil Abdulaev:

> +  while (!atomic_load_explicit (&stop, memory_order_acquire))
> +    {
> +      FILE *fp = fopen ("/dev/null", "r");
> +      if (fp != NULL)
> +        fclose (fp);
> +    }

Sorry, missed this: Is there any reason not to treat fopen failure as a
test failure?  You could perhaps use xfopen/xfclose, but some of us
prefer not to use error-checking wrappers for functions under test.

Thanks,
Florian

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

[Shamil Abdulaev]

_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>
---
Changes since v1:
 - Apply the same _fileno/_IO_link_in reordering to libio/oldfileops.c
   (suggested by Florian Weimer).
 - tst-file-init-race: treat fopen/fclose failures as test failures via
   FAIL_EXIT1 instead of silently ignoring them (suggested by Florian
   Weimer).

 libio/Makefile             |  3 ++
 libio/fileops.c            |  2 +-
 libio/oldfileops.c         |  2 +-
 libio/tst-file-init-race.c | 69 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 libio/tst-file-init-race.c

diff --git a/libio/Makefile b/libio/Makefile
index 93656466df..7e448295e3 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -107,6 +107,7 @@ tests = \
   tst-fgetc-after-eof \
   tst-fgetwc \
   tst-fgetws \
+  tst-file-init-race \
   tst-fopenloc2 \
   tst-fputws \
   tst-freopen \
@@ -160,6 +161,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 8067c0a9cf..9348d7c3a1 100644
--- 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 41b15491cb..0b92afbdb1 100644
--- 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
index 0000000000..7adaba272c
--- /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 (C) 2026 Free Software Foundation, Inc.
+   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>
-- 
2.54.0

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

[Florian Weimer]

* Shamil Abdulaev:

> _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>
> ---
> Changes since v1:
>  - Apply the same _fileno/_IO_link_in reordering to libio/oldfileops.c
>    (suggested by Florian Weimer).
>  - tst-file-init-race: treat fopen/fclose failures as test failures via
>    FAIL_EXIT1 instead of silently ignoring them (suggested by Florian
>    Weimer).

This version looks okay to me.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

I'll do a final round of testing and will push it for you afterwards.

Thanks,
Florian

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

[Florian Weimer]

* Florian Weimer:

> * Shamil Abdulaev:
>
>> _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>
>> ---
>> Changes since v1:
>>  - Apply the same _fileno/_IO_link_in reordering to libio/oldfileops.c
>>    (suggested by Florian Weimer).
>>  - tst-file-init-race: treat fopen/fclose failures as test failures via
>>    FAIL_EXIT1 instead of silently ignoring them (suggested by Florian
>>    Weimer).
>
> This version looks okay to me.
>
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
>
> I'll do a final round of testing and will push it for you afterwards.

Sorry, something else has come up.  You submitted this under DCO:

> Signed-off-by: Shamil Abdulaev <ashamil435@gmail.com>

And yet the new file you added says:

> +   Copyright (C) 2026 Free Software Foundation, Inc.

For DCO submissions, it should be

+   Copyright The GNU Toolchain Authors.

Would you please submit a v3 with this fixed?  I don't feel quite
comfortable editing copyright notices before pushing, but I suppose I
could do it for you if explicitly told. 8-)

Thanks,
Florian