From 68ae5c20b5542ce6b1132457b4a8180d6218f195 Mon Sep 17 00:00:00 2001 From: Patsy Griffin Date: Fri, 25 Apr 2025 09:37:32 -0400 Subject: [PATCH] libio: Fix a deadlock after fork in popen libio: Correctly link tst-popen-fork against libpthread (RHEL-86018) Resolves: RHEL-86018 --- glibc-RHEL-86018-1.patch | 194 +++++++++++++++++++++++++++++++++++++++ glibc-RHEL-86018-2.patch | 29 ++++++ glibc.spec | 8 +- 3 files changed, 230 insertions(+), 1 deletion(-) create mode 100644 glibc-RHEL-86018-1.patch create mode 100644 glibc-RHEL-86018-2.patch diff --git a/glibc-RHEL-86018-1.patch b/glibc-RHEL-86018-1.patch new file mode 100644 index 0000000..1186217 --- /dev/null +++ b/glibc-RHEL-86018-1.patch @@ -0,0 +1,194 @@ +commit 9f0d2c0ee6c728643fcf9a4879e9f20f5e45ce5f +Author: Arjun Shankar +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 + +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 ++ . */ ++ ++#include ++#include ++#include ++#include ++#include ++ ++#include ++#include ++#include ++ ++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 +--- 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. */ diff --git a/glibc-RHEL-86018-2.patch b/glibc-RHEL-86018-2.patch new file mode 100644 index 0000000..5b3b7ca --- /dev/null +++ b/glibc-RHEL-86018-2.patch @@ -0,0 +1,29 @@ +commit 6a290b2895b77be839fcb7c44a6a9879560097ad +Author: Arjun Shankar +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 + +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)) diff --git a/glibc.spec b/glibc.spec index 8ea9aad..2df88c2 100644 --- a/glibc.spec +++ b/glibc.spec @@ -115,7 +115,7 @@ end \ Summary: The GNU libc libraries Name: glibc Version: %{glibcversion} -Release: %{glibcrelease}.18 +Release: %{glibcrelease}.19 # In general, GPLv2+ is used by programs, LGPLv2+ is used for # libraries. @@ -1264,6 +1264,8 @@ Patch1029: glibc-RHEL-83306-2.patch Patch1030: glibc-RHEL-35280.patch Patch1031: glibc-RHEL-76211.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: @@ -2925,6 +2927,10 @@ fi %{_libdir}/libpthread_nonshared.a %changelog +* Fri Apr 25 2025 Patsy Griffin - 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 - 2.28-251.18 - x86: Avoid integer truncation with large cache sizes (RHEL-76387)