libio: Fix a deadlock after fork in popen
libio: Correctly link tst-popen-fork against libpthread (RHEL-86018) Resolves: RHEL-86018
This commit is contained in:
Patsy Griffin
parent
01b9bca186
commit
68ae5c20b5
194
glibc-RHEL-86018-1.patch
Normal file
194
glibc-RHEL-86018-1.patch
Normal file
@ -0,0 +1,194 @@
|
|||||||
|
commit 9f0d2c0ee6c728643fcf9a4879e9f20f5e45ce5f
|
||||||
|
Author: Arjun Shankar <arjun@redhat.com>
|
||||||
|
Date: Fri Oct 18 16:03:25 2024 +0200
|
||||||
|
|
||||||
|
libio: Fix a deadlock after fork in popen
|
||||||
|
|
||||||
|
popen modifies its file handler book-keeping under a lock that wasn't
|
||||||
|
being taken during fork. This meant that a concurrent popen and fork
|
||||||
|
could end up copying the lock in a "locked" state into the fork child,
|
||||||
|
where subsequently calling popen would lead to a deadlock due to the
|
||||||
|
already (spuriously) held lock.
|
||||||
|
|
||||||
|
This commit fixes the deadlock by appropriately taking the lock before
|
||||||
|
fork, and releasing/resetting it in the parent/child after the fork.
|
||||||
|
|
||||||
|
A new test for concurrent popen and fork is also added. It consistently
|
||||||
|
hangs (and therefore fails via timeout) without the fix applied.
|
||||||
|
Reviewed-by: Florian Weimer <fweimer@redhat.com>
|
||||||
|
|
||||||
|
Conflicts:
|
||||||
|
libio/Makefile
|
||||||
|
(Usual conflict when adding tests due to missing backports.)
|
||||||
|
posix/fork.c
|
||||||
|
(reworked due to file rename upstream and libpthread integration)
|
||||||
|
|
||||||
|
|
||||||
|
diff -Nrup a/libio/Makefile b/libio/Makefile
|
||||||
|
--- a/libio/Makefile 2025-04-23 15:51:08.439579602 -0400
|
||||||
|
+++ b/libio/Makefile 2025-04-23 15:11:32.124866600 -0400
|
||||||
|
@@ -62,7 +62,7 @@ tests = tst_swprintf tst_wprintf tst_sws
|
||||||
|
tst-memstream1 tst-memstream2 tst-memstream3 \
|
||||||
|
tst-wmemstream1 tst-wmemstream2 tst-wmemstream3 \
|
||||||
|
bug-memstream1 bug-wmemstream1 \
|
||||||
|
- tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \
|
||||||
|
+ tst-setvbuf1 tst-popen-fork tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \
|
||||||
|
tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \
|
||||||
|
tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof \
|
||||||
|
tst-wfile-sync
|
||||||
|
diff -Nrup a/libio/iopopen.c b/libio/iopopen.c
|
||||||
|
--- a/libio/iopopen.c 2025-04-23 15:51:09.145583660 -0400
|
||||||
|
+++ b/libio/iopopen.c 2025-04-23 15:07:45.167549822 -0400
|
||||||
|
@@ -60,6 +60,26 @@ unlock (void *not_used)
|
||||||
|
}
|
||||||
|
#endif
|
||||||
|
|
||||||
|
+/* These lock/unlock/resetlock functions are used during fork. */
|
||||||
|
+
|
||||||
|
+void
|
||||||
|
+_IO_proc_file_chain_lock (void)
|
||||||
|
+{
|
||||||
|
+ _IO_lock_lock (proc_file_chain_lock);
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+void
|
||||||
|
+_IO_proc_file_chain_unlock (void)
|
||||||
|
+{
|
||||||
|
+ _IO_lock_unlock (proc_file_chain_lock);
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+void
|
||||||
|
+_IO_proc_file_chain_resetlock (void)
|
||||||
|
+{
|
||||||
|
+ _IO_lock_init (proc_file_chain_lock);
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
/* POSIX states popen shall ensure that any streams from previous popen()
|
||||||
|
calls that remain open in the parent process should be closed in the new
|
||||||
|
child process.
|
||||||
|
diff -Nrup a/libio/libioP.h b/libio/libioP.h
|
||||||
|
--- a/libio/libioP.h 2025-04-23 15:51:08.012577147 -0400
|
||||||
|
+++ b/libio/libioP.h 2025-04-23 15:07:45.167549822 -0400
|
||||||
|
@@ -423,6 +423,12 @@ libc_hidden_proto (_IO_list_resetlock)
|
||||||
|
extern void _IO_enable_locks (void) __THROW;
|
||||||
|
libc_hidden_proto (_IO_enable_locks)
|
||||||
|
|
||||||
|
+/* Functions for operating popen's proc_file_chain_lock during fork. */
|
||||||
|
+
|
||||||
|
+extern void _IO_proc_file_chain_lock (void) __THROW attribute_hidden;
|
||||||
|
+extern void _IO_proc_file_chain_unlock (void) __THROW attribute_hidden;
|
||||||
|
+extern void _IO_proc_file_chain_resetlock (void) __THROW attribute_hidden;
|
||||||
|
+
|
||||||
|
/* Default jumptable functions. */
|
||||||
|
|
||||||
|
extern int _IO_default_underflow (FILE *) __THROW;
|
||||||
|
diff -Nrup a/libio/tst-popen-fork.c b/libio/tst-popen-fork.c
|
||||||
|
--- a/libio/tst-popen-fork.c 1969-12-31 19:00:00.000000000 -0500
|
||||||
|
+++ b/libio/tst-popen-fork.c 2025-04-23 15:07:45.168549828 -0400
|
||||||
|
@@ -0,0 +1,80 @@
|
||||||
|
+/* Test concurrent popen and fork.
|
||||||
|
+ Copyright (C) 2024 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 <unistd.h>
|
||||||
|
+#include <sys/wait.h>
|
||||||
|
+
|
||||||
|
+#include <support/check.h>
|
||||||
|
+#include <support/xthread.h>
|
||||||
|
+#include <support/xunistd.h>
|
||||||
|
+
|
||||||
|
+static void
|
||||||
|
+popen_and_pclose (void)
|
||||||
|
+{
|
||||||
|
+ FILE *f = popen ("true", "r");
|
||||||
|
+ TEST_VERIFY_EXIT (f != NULL);
|
||||||
|
+ pclose (f);
|
||||||
|
+ return;
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+static atomic_bool done = ATOMIC_VAR_INIT (0);
|
||||||
|
+
|
||||||
|
+static void *
|
||||||
|
+popen_and_pclose_forever (__attribute__ ((unused))
|
||||||
|
+ void *arg)
|
||||||
|
+{
|
||||||
|
+ while (!atomic_load_explicit (&done, memory_order_acquire))
|
||||||
|
+ popen_and_pclose ();
|
||||||
|
+ return NULL;
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+static int
|
||||||
|
+do_test (void)
|
||||||
|
+{
|
||||||
|
+
|
||||||
|
+ /* Repeatedly call popen in a loop during the entire test. */
|
||||||
|
+ pthread_t t = xpthread_create (NULL, popen_and_pclose_forever, NULL);
|
||||||
|
+
|
||||||
|
+ /* Repeatedly fork off and reap child processes one-by-one.
|
||||||
|
+ Each child calls popen once, then exits, leading to the possibility
|
||||||
|
+ that a child forks *during* our own popen call, thus inheriting any
|
||||||
|
+ intermediate popen state, possibly including lock state(s). */
|
||||||
|
+ for (int i = 0; i < 100; i++)
|
||||||
|
+ {
|
||||||
|
+ int cpid = xfork ();
|
||||||
|
+
|
||||||
|
+ if (cpid == 0)
|
||||||
|
+ {
|
||||||
|
+ popen_and_pclose ();
|
||||||
|
+ _exit (0);
|
||||||
|
+ }
|
||||||
|
+ else
|
||||||
|
+ xwaitpid (cpid, NULL, 0);
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ /* Stop calling popen. */
|
||||||
|
+ atomic_store_explicit (&done, 1, memory_order_release);
|
||||||
|
+ xpthread_join (t);
|
||||||
|
+
|
||||||
|
+ return 0;
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+#include <support/test-driver.c>
|
||||||
|
--- a/sysdeps/nptl/fork.c 2025-04-23 16:33:49.283403134 -0400
|
||||||
|
+++ b/sysdeps/nptl/fork.c 2025-04-23 16:33:12.240190356 -0400
|
||||||
|
@@ -65,6 +65,7 @@ __libc_fork (void)
|
||||||
|
not matter if fork was called from a signal handler. */
|
||||||
|
if (multiple_threads)
|
||||||
|
{
|
||||||
|
+ _IO_proc_file_chain_lock ();
|
||||||
|
_IO_list_lock ();
|
||||||
|
|
||||||
|
/* Acquire malloc locks. This needs to come last because fork
|
||||||
|
@@ -121,6 +122,7 @@ __libc_fork (void)
|
||||||
|
|
||||||
|
/* Reset locks in the I/O code. */
|
||||||
|
_IO_list_resetlock ();
|
||||||
|
+ _IO_proc_file_chain_resetlock ();
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Reset the lock the dynamic loader uses to protect its data. */
|
||||||
|
@@ -142,6 +144,7 @@ __libc_fork (void)
|
||||||
|
|
||||||
|
/* We execute this even if the 'fork' call failed. */
|
||||||
|
_IO_list_unlock ();
|
||||||
|
+ _IO_proc_file_chain_unlock ();
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Run the handlers registered for the parent. */
|
||||||
29
glibc-RHEL-86018-2.patch
Normal file
29
glibc-RHEL-86018-2.patch
Normal file
@ -0,0 +1,29 @@
|
|||||||
|
commit 6a290b2895b77be839fcb7c44a6a9879560097ad
|
||||||
|
Author: Arjun Shankar <arjun@redhat.com>
|
||||||
|
Date: Fri Oct 25 09:33:45 2024 +0200
|
||||||
|
|
||||||
|
libio: Correctly link tst-popen-fork against libpthread
|
||||||
|
|
||||||
|
tst-popen-fork failed to build for Hurd due to not being linked with
|
||||||
|
libpthread. This commit fixes that.
|
||||||
|
|
||||||
|
Tested with build-many-glibcs.py for i686-gnu.
|
||||||
|
|
||||||
|
Reviewed-by: Florian Weimer <fweimer@redhat.com>
|
||||||
|
|
||||||
|
Conflicts:
|
||||||
|
libio/Makefile
|
||||||
|
(Usual conflict when adding tests due to missing backports.)
|
||||||
|
|
||||||
|
diff -Nrup a/libio/Makefile b/libio/Makefile
|
||||||
|
--- a/libio/Makefile 2025-04-23 16:41:58.028210552 -0400
|
||||||
|
+++ b/libio/Makefile 2025-04-23 16:41:30.195050679 -0400
|
||||||
|
@@ -67,6 +67,8 @@ tests = tst_swprintf tst_wprintf tst_sws
|
||||||
|
tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof \
|
||||||
|
tst-wfile-sync
|
||||||
|
|
||||||
|
+$(objpfx)tst-popen-fork: $(shared-thread-library)
|
||||||
|
+
|
||||||
|
tests-internal = tst-vtables tst-vtables-interposed
|
||||||
|
|
||||||
|
ifeq (yes,$(build-shared))
|
||||||
@ -115,7 +115,7 @@ end \
|
|||||||
Summary: The GNU libc libraries
|
Summary: The GNU libc libraries
|
||||||
Name: glibc
|
Name: glibc
|
||||||
Version: %{glibcversion}
|
Version: %{glibcversion}
|
||||||
Release: %{glibcrelease}.18
|
Release: %{glibcrelease}.19
|
||||||
|
|
||||||
# In general, GPLv2+ is used by programs, LGPLv2+ is used for
|
# In general, GPLv2+ is used by programs, LGPLv2+ is used for
|
||||||
# libraries.
|
# libraries.
|
||||||
@ -1264,6 +1264,8 @@ Patch1029: glibc-RHEL-83306-2.patch
|
|||||||
Patch1030: glibc-RHEL-35280.patch
|
Patch1030: glibc-RHEL-35280.patch
|
||||||
Patch1031: glibc-RHEL-76211.patch
|
Patch1031: glibc-RHEL-76211.patch
|
||||||
Patch1032: glibc-RHEL-76387.patch
|
Patch1032: glibc-RHEL-76387.patch
|
||||||
|
Patch1033: glibc-RHEL-86018-1.patch
|
||||||
|
Patch1034: glibc-RHEL-86018-2.patch
|
||||||
|
|
||||||
##############################################################################
|
##############################################################################
|
||||||
# Continued list of core "glibc" package information:
|
# Continued list of core "glibc" package information:
|
||||||
@ -2925,6 +2927,10 @@ fi
|
|||||||
%{_libdir}/libpthread_nonshared.a
|
%{_libdir}/libpthread_nonshared.a
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Fri Apr 25 2025 Patsy Griffin <patsy@redhat.com> - 2.28-251.19
|
||||||
|
- libio: Fix a deadlock after fork in popen
|
||||||
|
- libio: Correctly link tst-popen-fork against libpthread (RHEL-86018)
|
||||||
|
|
||||||
* Fri Apr 18 2025 Patsy Griffin <patsy@redhat.com> - 2.28-251.18
|
* Fri Apr 18 2025 Patsy Griffin <patsy@redhat.com> - 2.28-251.18
|
||||||
- x86: Avoid integer truncation with large cache sizes (RHEL-76387)
|
- x86: Avoid integer truncation with large cache sizes (RHEL-76387)
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user