libio: Fix deadlock between freopen, fflush (NULL) and fclose (bug 24963)

The canonical lock ordering for stream list processing is
locking list_all_lock first, then individual streams as needed.
The fclose implementation reversed that, and the freopen
implementation performed list operations under the reverse order,
too.

Unlinking in fclose was already unconditional, and the early unlinking
looks unnecessary: _IO_file_close_it would call it even for
!_IO_IS_FILEBUF streams.

There is still a remaining concurrency defect because
_IO_new_file_init_internal links in the stream before it is
fully initialized, and it is not locked at this point.

Reviewed-by: Arjun Shankar <arjun@redhat.com>

libio/fileops.c

@@ -124,8 +124,10 @@ _IO_new_file_init (struct _IO_FILE_plus *fp)
   _IO_new_file_init_internal (fp);
 }
 
+/* Close FP, but do not deallocate it.  If UNLINK, call _IO_un_link to
+   remove the file from the global list of files if necessary.  */
 int
-_IO_new_file_close_it (FILE *fp)
+_IO_file_close_maybe_unlink (FILE *fp, bool unlink)
 {
   int flush_status = 0;
   if (!_IO_file_is_open (fp))
@@ -188,13 +190,22 @@ _IO_new_file_close_it (FILE *fp)
   _IO_setg (fp, NULL, NULL, NULL);
   _IO_setp (fp, NULL, NULL);
 
-  _IO_un_link ((struct _IO_FILE_plus *) fp);
-  fp->_flags = _IO_MAGIC|CLOSED_FILEBUF_FLAGS;
+  if (unlink)
+    _IO_un_link ((struct _IO_FILE_plus *) fp);
+  /* Preserve the _IO_LINKED flag, so that _IO_un_link called from
+     fclose still unlinks the stream.  */
+  fp->_flags = _IO_MAGIC | CLOSED_FILEBUF_FLAGS | (fp->_flags & _IO_LINKED);
   fp->_fileno = -1;
   fp->_offset = _IO_pos_BAD;
 
   return close_status ? close_status : flush_status;
 }
+
+int
+_IO_new_file_close_it (FILE *fp)
+{
+  return _IO_file_close_maybe_unlink (fp, true);
+}
 libc_hidden_ver (_IO_new_file_close_it, _IO_file_close_it)
 
 void
@@ -236,7 +247,12 @@ _IO_file_open (FILE *fp, const char *filename, int posix_mode, int prot,
 	  return NULL;
 	}
     }
-  _IO_link_in ((struct _IO_FILE_plus *) fp);
+
+  /* During reopen, do not try to link in the stream.  It is already on
+     the list.  This avoids deadlocks due to lock ordering issues.  */
+  if ((fp->_flags2 & _IO_FLAGS2_NOCLOSE) == 0)
+    _IO_link_in ((struct _IO_FILE_plus *) fp);
+
   return fp;
 }
 libc_hidden_def (_IO_file_open)

libio/freopen.c

@@ -72,7 +72,10 @@ freopen (const char *filename, const char *mode, FILE *fp)
   else
 #endif
     {
-      _IO_file_close_it (fp);
+      /* Do not unlink the stream because it has to stay on the list.
+         Flushing through fflush (NULL) is prevented because the
+         stream is still locked.  */
+      _IO_file_close_maybe_unlink (fp, false);
       _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps;
       if (_IO_vtable_offset (fp) == 0 && fp->_wide_data != NULL)
 	fp->_wide_data->_wide_vtable = &_IO_wfile_jumps;
@@ -112,8 +115,19 @@ freopen (const char *filename, const char *mode, FILE *fp)
     __close (fd);
 
 end:
+  if (result == NULL)
+    /* After the unlock below, _IO_flush_all could run and try to
+       flush the partially closed stream.  Setting the flag prevents that.  */
+    fp->_flags2 |= _IO_FLAGS2_NOCLOSE;
+
   _IO_release_lock (fp);
-  if (result == NULL && (fp->_flags & _IO_IS_FILEBUF) != 0)
-    _IO_deallocate_file (fp);
+
+  /* See fclose for the concurrency impact.  */
+  if (result == NULL)
+    {
+      _IO_un_link ((struct _IO_FILE_plus *) fp);
+      if (fp->_flags & _IO_IS_FILEBUF)
+	_IO_deallocate_file (fp);
+    }
   return result;
 }

libio/freopen64.c

@@ -52,7 +52,10 @@ freopen64 (const char *filename, const char *mode, FILE *fp)
     = filename != NULL ? filename : __fd_to_filename (fd, &fdfilename);
 
   fp->_flags2 |= _IO_FLAGS2_NOCLOSE;
-  _IO_file_close_it (fp);
+  /* Do not unlink the stream because it has to stay on the list.
+     Flushing through fflush (NULL) is prevented because the
+     stream is still locked.  */
+  _IO_file_close_maybe_unlink (fp, false);
   _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps;
   if (_IO_vtable_offset (fp) == 0 && fp->_wide_data != NULL)
     fp->_wide_data->_wide_vtable = &_IO_wfile_jumps;
@@ -91,8 +94,19 @@ freopen64 (const char *filename, const char *mode, FILE *fp)
     __close (fd);
 
 end:
+  if (result == NULL)
+    /* After the unlock below, _IO_flush_all could run and try to
+       flush the partially closed stream.  Setting the flag prevents that.  */
+    fp->_flags2 |= _IO_FLAGS2_NOCLOSE;
+
   _IO_release_lock (fp);
-  if (result == NULL && (fp->_flags & _IO_IS_FILEBUF) != 0)
-    _IO_deallocate_file (fp);
+
+  /* See fclose for the concurrency impact.  */
+  if (result == NULL)
+    {
+      _IO_un_link ((struct _IO_FILE_plus *) fp);
+      if (fp->_flags & _IO_IS_FILEBUF)
+	_IO_deallocate_file (fp);
+    }
   return result;
 }

libio/genops.c

@@ -724,20 +724,25 @@ _IO_flush_all (void)
       run_fp = fp;
       _IO_flockfile (fp);
 
-      if (((fp->_mode <= 0 && fp->_IO_write_ptr > fp->_IO_write_base)
-	   || (_IO_vtable_offset (fp) == 0
-	       && fp->_mode > 0 && (fp->_wide_data->_IO_write_ptr
-				    > fp->_wide_data->_IO_write_base))
-	   )
-	  && _IO_OVERFLOW (fp, EOF) == EOF)
-	result = EOF;
-      if (_IO_fileno (fp) >= 0
-	  && ((fp->_mode <= 0 && fp->_IO_read_ptr < fp->_IO_read_end)
-	      || (_IO_vtable_offset (fp) == 0
-		  && fp->_mode > 0 && (fp->_wide_data->_IO_read_ptr
-				       < fp->_wide_data->_IO_read_end)))
-	  && _IO_SYNC (fp) != 0)
-	result = EOF;
+      /* If fp is in an freopen operation or about to be closed, do
+	 not flush it again.  Flushing is handled by these operations.  */
+      if ((fp->_flags2 & _IO_FLAGS2_NOCLOSE) == 0)
+	{
+	  if (((fp->_mode <= 0 && fp->_IO_write_ptr > fp->_IO_write_base)
+	       || (_IO_vtable_offset (fp) == 0
+		   && fp->_mode > 0 && (fp->_wide_data->_IO_write_ptr
+					> fp->_wide_data->_IO_write_base))
+	       )
+	      && _IO_OVERFLOW (fp, EOF) == EOF)
+	    result = EOF;
+	  if (_IO_fileno (fp) >= 0
+	      && ((fp->_mode <= 0 && fp->_IO_read_ptr < fp->_IO_read_end)
+		  || (_IO_vtable_offset (fp) == 0
+		      && fp->_mode > 0 && (fp->_wide_data->_IO_read_ptr
+					   < fp->_wide_data->_IO_read_end)))
+	      && _IO_SYNC (fp) != 0)
+	    result = EOF;
+	}
 
       _IO_funlockfile (fp);
       run_fp = NULL;
@@ -767,7 +772,11 @@ _IO_flush_all_linebuffered (void)
       run_fp = fp;
       _IO_flockfile (fp);
 
-      if ((fp->_flags & _IO_NO_WRITES) == 0 && fp->_flags & _IO_LINE_BUF)
+      /* Regarding _IO_FLAGS2_NOCLOSE: If fp is in an freopen
+	 operation or about to be closed, do not flush it again.
+	 Flushing is handled by these operations.  */
+      if ((fp->_flags & _IO_NO_WRITES) == 0 && fp->_flags & _IO_LINE_BUF
+	  && (fp->_flags2 & _IO_FLAGS2_NOCLOSE) == 0)
 	_IO_OVERFLOW (fp, EOF);
 
       _IO_funlockfile (fp);

libio/iofclose.c

@@ -44,16 +44,26 @@ _IO_new_fclose (FILE *fp)
     return _IO_old_fclose (fp);
 #endif
 
-  /* First unlink the stream.  */
-  if (fp->_flags & _IO_IS_FILEBUF)
-    _IO_un_link ((struct _IO_FILE_plus *) fp);
-
   _IO_acquire_lock (fp);
   if (fp->_flags & _IO_IS_FILEBUF)
-    status = _IO_file_close_it (fp);
+    {
+      status = _IO_file_close_maybe_unlink (fp, false);
+      /* Skip future flushing.  */
+      fp->_flags2 |= _IO_FLAGS2_NOCLOSE;
+    }
   else
     status = fp->_flags & _IO_ERR_SEEN ? -1 : 0;
   _IO_release_lock (fp);
+
+  /* Unlink after releasing the lock on fp.  This maintains the usual
+     locking order (list_all_lock acquired first, then the fp lock).
+     The only valid reference to fp after a call to fclose is the
+     implicit reference to it as part of fflush (NULL).  The
+     _IO_un_link call here synchronizes with fflush (NULL).  Future
+     interaction with fflush (NULL) is not possible because the stream
+     is no longer on the list.  */
+  _IO_un_link ((struct _IO_FILE_plus *) fp);
+
   _IO_FINISH (fp);
   if (fp->_mode > 0)
     {

libio/libio.h

@@ -86,7 +86,12 @@ typedef struct
 #define _IO_FLAGS2_MMAP 1
 #define _IO_FLAGS2_NOTCANCEL 2
 #define _IO_FLAGS2_USER_WBUF 8
+
+/* The file is in a freopen operation, or it is about to be closed.
+   Closing it does not deallocate its underlying file descriptor, and
+   fflush (NULL) will not flush this file.  */
 #define _IO_FLAGS2_NOCLOSE 32
+
 #define _IO_FLAGS2_CLOEXEC 64
 #define _IO_FLAGS2_NEED_LOCK 128
 

libio/libioP.h

@@ -639,6 +639,7 @@ libc_hidden_proto (_IO_file_finish)
 
 extern FILE* _IO_new_file_attach (FILE *, int);
 extern int _IO_new_file_close_it (FILE *);
+int _IO_file_close_maybe_unlink (FILE *, bool) attribute_hidden;
 extern void _IO_new_file_finish (FILE *, int);
 extern FILE* _IO_new_file_fopen (FILE *, const char *, const char *,
 				     int);

sysdeps/pthread/Makefile

@@ -71,6 +71,7 @@ tests += \
   tst-basic5 \
   tst-basic6 \
   tst-basic7 \
+  tst-bug24963 \
   tst-call-once \
   tst-cancel-self \
   tst-cancel-self-cancelstate \

sysdeps/pthread/tst-bug24963.c

@@ -0,0 +1,106 @@
+/* Test lock ordering of fflush (NULL) vs freopen, fclose (bug 24963).
+   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 <array_length.h>
+#include <stdint.h>
+#include <stdio_ext.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include <support/xthread.h>
+#include <unistd.h>
+
+static _Atomic bool running = true;
+
+static void *
+fflush_thread (void *ignored)
+{
+  while (running)
+    TEST_COMPARE (fflush (NULL), 0);
+
+  return NULL;
+}
+
+static void *
+flushlbf_thread (void *ignored)
+{
+  while (running)
+    _flushlbf ();
+
+  return NULL;
+}
+
+static void *
+fopen_thread (void *ignored)
+{
+  while (running)
+    {
+      FILE *fp = xfopen ("/etc/passwd", "r");
+      (void) fgetc (fp);
+      xfclose (fp);
+    }
+  return NULL;
+}
+
+static void *
+freopen_thread (void *fp)
+{
+  while (running)
+    {
+      uintptr_t old_address = (uintptr_t) fp;
+      FILE *fpnew = xfreopen ("/etc/passwd", "r", fp);
+      TEST_COMPARE (old_address, (uintptr_t) fpnew);
+    }
+  return NULL;
+}
+
+static int
+do_test (void)
+{
+  pthread_t fflush_thr = xpthread_create (NULL, fflush_thread, NULL);
+  pthread_t flushlbf_thr = xpthread_create (NULL, flushlbf_thread, NULL);
+
+  pthread_t fopens[2];
+  for (int i = 0; i < array_length(fopens); ++i)
+    fopens[i] = xpthread_create (NULL, fopen_thread, NULL);
+
+  FILE *fps[2];
+  pthread_t freopens[array_length (fps)];
+  for (int i = 0; i < array_length(fps); ++i)
+    {
+      fps[i] = xfopen ("/etc/passwd", "r");
+      freopens[i] = xpthread_create (NULL, freopen_thread, fps[i]);
+    }
+
+  usleep (2 * 1000 * 1000);
+  running = false;
+
+  for (int i = 0; i < array_length(fopens); ++i)
+    xpthread_join (fopens[i]);
+  for (int i = 0; i < array_length(fps); ++i)
+    {
+      xpthread_join (freopens[i]);
+      xfclose (fps[i]);
+    }
+
+  xpthread_join (flushlbf_thr);
+  xpthread_join (fflush_thr);
+
+  return 0;
+}
+
+#include <support/test-driver.c>