Script 'mail_helper' called by obssrc Hello community, here is the log from the commit of package glibc for openSUSE:Factory checked in at 2021-10-11 15:30:31 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/glibc (Old) and /work/SRC/openSUSE:Factory/.glibc.new.2443 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "glibc" Mon Oct 11 15:30:31 2021 rev:254 rq:923223 version:2.34 Changes: -------- --- /work/SRC/openSUSE:Factory/glibc/glibc.changes 2021-09-17 23:25:37.841219012 +0200 +++ /work/SRC/openSUSE:Factory/.glibc.new.2443/glibc.changes 2021-10-11 15:30:47.682765845 +0200 @@ -1,0 +2,27 @@ +Tue Oct 5 10:58:00 UTC 2021 - Andreas Schwab <sch...@suse.de> + +- ld-show-auxv-colon.patch: elf: Fix missing colon in LD_SHOW_AUXV output + (BZ #282539 +- x86-string-control-test.patch: x86-64: Use testl to check + __x86_string_control +- pthread-kill-fail-after-exit.patch: nptl: pthread_kill, pthread_cancel + should not fail after exit (BZ #19193) +- pthread-kill-race-thread-exit.patch: nptl: Fix race between pthread_kill + and thread exit (BZ #12889) +- getcwd-attribute-access.patch: posix: Fix attribute access mode on + getcwd (BZ #27476) +- pthread-kill-return-esrch.patch: nptl: pthread_kill needs to return + ESRCH for old programs (BZ #19193) +- pthread-mutexattr-getrobust-np-type.patch: nptl: Fix type of + pthread_mutexattr_getrobust_np, pthread_mutexattr_setrobust_np (BZ + #28036) +- setxid-deadlock-blocked-signals.patch: nptl: Avoid setxid deadlock with + blocked signals in thread exit (BZ #28361) +- pthread-kill-send-specific-thread.patch: nptl: pthread_kill must send + signals to a specific thread (BZ #28407) +- sysconf-nprocessors-affinity.patch: linux: Revert the use of + sched_getaffinity on get_nproc (BZ #28310) +- iconv-charmap-close-output.patch: renamed from + icon-charmap-close-output.patch + +------------------------------------------------------------------- Old: ---- icon-charmap-close-output.patch New: ---- getcwd-attribute-access.patch iconv-charmap-close-output.patch ld-show-auxv-colon.patch pthread-kill-fail-after-exit.patch pthread-kill-race-thread-exit.patch pthread-kill-return-esrch.patch pthread-kill-send-specific-thread.patch pthread-mutexattr-getrobust-np-type.patch setxid-deadlock-blocked-signals.patch sysconf-nprocessors-affinity.patch x86-string-control-test.patch ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ glibc.spec ++++++ --- /var/tmp/diff_new_pack.4G1phN/_old 2021-10-11 15:30:48.906767807 +0200 +++ /var/tmp/diff_new_pack.4G1phN/_new 2021-10-11 15:30:48.910767814 +0200 @@ -290,11 +290,31 @@ # PATCH-FIX-UPSTREAM copy_and_spawn_sgid: Avoid double calls to close() Patch1003: copy-and-spawn-sgid-double-close.patch # PATCH-FIX-UPSTREAM iconv_charmap: Close output file when done -Patch1004: icon-charmap-close-output.patch +Patch1004: iconv-charmap-close-output.patch # PATCH-FIX-UPSTREAM Linux: Fix fcntl, ioctl, prctl redirects for _TIME_BITS=64 (BZ #28182) Patch1005: fcntl-time-bits-64-redirect.patch # PATCH-FIX-UPSTREAM librt: fix NULL pointer dereference (BZ #28213) Patch1006: librt-null-pointer.patch +# PATCH-FIX-UPSTREAM elf: Fix missing colon in LD_SHOW_AUXV output (BZ #282539 +Patch1007: ld-show-auxv-colon.patch +# PATCH-FIX-UPSTREAM x86-64: Use testl to check __x86_string_control +Patch1008: x86-string-control-test.patch +# PATCH-FIX-UPSTREAM nptl: pthread_kill, pthread_cancel should not fail after exit (BZ #19193) +Patch1009: pthread-kill-fail-after-exit.patch +# PATCH-FIX-UPSTREAM nptl: Fix race between pthread_kill and thread exit (BZ #12889) +Patch1010: pthread-kill-race-thread-exit.patch +# PATCH-FIX-UPSTREAM posix: Fix attribute access mode on getcwd (BZ #27476) +Patch1011: getcwd-attribute-access.patch +# PATCH-FIX-UPSTREAM nptl: pthread_kill needs to return ESRCH for old programs (BZ #19193) +Patch1012: pthread-kill-return-esrch.patch +# PATCH-FIX-UPSTREAM nptl: Fix type of pthread_mutexattr_getrobust_np, pthread_mutexattr_setrobust_np (BZ #28036) +Patch1013: pthread-mutexattr-getrobust-np-type.patch +# PATCH-FIX-UPSTREAM nptl: Avoid setxid deadlock with blocked signals in thread exit (BZ #28361) +Patch1014: setxid-deadlock-blocked-signals.patch +# PATCH-FIX-UPSTREAM nptl: pthread_kill must send signals to a specific thread (BZ #28407) +Patch1015: pthread-kill-send-specific-thread.patch +# PATCH-FIX-UPSTREAM linux: Revert the use of sched_getaffinity on get_nproc (BZ #28310) +Patch1016: sysconf-nprocessors-affinity.patch ### # Patches awaiting upstream approval @@ -525,6 +545,16 @@ %patch1004 -p1 %patch1005 -p1 %patch1006 -p1 +%patch1007 -p1 +%patch1008 -p1 +%patch1009 -p1 +%patch1010 -p1 +%patch1011 -p1 +%patch1012 -p1 +%patch1013 -p1 +%patch1014 -p1 +%patch1015 -p1 +%patch1016 -p1 %patch3000 ++++++ getcwd-attribute-access.patch ++++++ >From 433ec4f14a5753c7689c83c20c9972915c53c204 Mon Sep 17 00:00:00 2001 From: Aurelien Jarno <aurel...@aurel32.net> Date: Fri, 10 Sep 2021 19:39:35 +0200 Subject: [PATCH] posix: Fix attribute access mode on getcwd [BZ #27476] There is a GNU extension that allows to call getcwd(NULL, >0). It is described in the documentation, but also directly in the unistd.h header, just above the declaration. Therefore the attribute access mode added in commit 06febd8c6705 is not correct. Drop it. --- posix/bits/unistd.h | 5 ++--- posix/unistd.h | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/posix/bits/unistd.h b/posix/bits/unistd.h index f0831386c7..622adeb2b2 100644 --- a/posix/bits/unistd.h +++ b/posix/bits/unistd.h @@ -199,10 +199,9 @@ __NTH (readlinkat (int __fd, const char *__restrict __path, #endif extern char *__getcwd_chk (char *__buf, size_t __size, size_t __buflen) - __THROW __wur __attr_access ((__write_only__, 1, 2)); + __THROW __wur; extern char *__REDIRECT_NTH (__getcwd_alias, - (char *__buf, size_t __size), getcwd) - __wur __attr_access ((__write_only__, 1, 2)); + (char *__buf, size_t __size), getcwd) __wur; extern char *__REDIRECT_NTH (__getcwd_chk_warn, (char *__buf, size_t __size, size_t __buflen), __getcwd_chk) diff --git a/posix/unistd.h b/posix/unistd.h index 3dca65732f..8224c5fbc9 100644 --- a/posix/unistd.h +++ b/posix/unistd.h @@ -528,8 +528,7 @@ extern int fchdir (int __fd) __THROW __wur; an array is allocated with `malloc'; the array is SIZE bytes long, unless SIZE == 0, in which case it is as big as necessary. */ -extern char *getcwd (char *__buf, size_t __size) __THROW __wur - __attr_access ((__write_only__, 1, 2)); +extern char *getcwd (char *__buf, size_t __size) __THROW __wur; #ifdef __USE_GNU /* Return a malloc'd string containing the current directory name. -- 2.33.0 ++++++ iconv-charmap-close-output.patch ++++++ >From 1e0e6d656db9dfa12ef7eb67976385d3deb0d4ff Mon Sep 17 00:00:00 2001 From: Siddhesh Poyarekar <siddh...@sourceware.org> Date: Tue, 3 Aug 2021 21:10:29 +0530 Subject: [PATCH] iconv_charmap: Close output file when done Reviewed-by: Arjun Shankar <ar...@redhat.com> --- iconv/iconv_charmap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/iconv/iconv_charmap.c b/iconv/iconv_charmap.c index e2d53fee3c..a8b6b56124 100644 --- a/iconv/iconv_charmap.c +++ b/iconv/iconv_charmap.c @@ -234,6 +234,8 @@ charmap_conversion (const char *from_code, struct charmap_t *from_charmap, while (++remaining < argc); /* All done. */ + if (output != stdout) + fclose (output); free_table (cvtbl); return status; } -- 2.32.0 ++++++ ld-show-auxv-colon.patch ++++++ >From 9acab0bba6a5a57323b1f94bf95b21618a9e5aa4 Mon Sep 17 00:00:00 2001 From: Arjun Shankar <ar...@redhat.com> Date: Fri, 20 Aug 2021 16:24:05 +0200 Subject: [PATCH] elf: Fix missing colon in LD_SHOW_AUXV output [BZ #28253] This commit adds a missing colon in the AT_MINSIGSTKSZ entry in the _dl_show_auxv function. (cherry picked from commit 82fbcd7118d760492e2ecc9fa291e358b9ba0361) --- elf/dl-sysdep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c index d47bef1340..2c684c2db2 100644 --- a/elf/dl-sysdep.c +++ b/elf/dl-sysdep.c @@ -317,7 +317,7 @@ _dl_show_auxv (void) [AT_SYSINFO_EHDR - 2] = { "SYSINFO_EHDR: 0x", hex }, [AT_RANDOM - 2] = { "RANDOM: 0x", hex }, [AT_HWCAP2 - 2] = { "HWCAP2: 0x", hex }, - [AT_MINSIGSTKSZ - 2] = { "MINSIGSTKSZ ", dec }, + [AT_MINSIGSTKSZ - 2] = { "MINSIGSTKSZ: ", dec }, [AT_L1I_CACHESIZE - 2] = { "L1I_CACHESIZE: ", dec }, [AT_L1I_CACHEGEOMETRY - 2] = { "L1I_CACHEGEOMETRY: 0x", hex }, [AT_L1D_CACHESIZE - 2] = { "L1D_CACHESIZE: ", dec }, -- 2.33.0 ++++++ nscd.service ++++++ --- /var/tmp/diff_new_pack.4G1phN/_old 2021-10-11 15:30:49.118768147 +0200 +++ /var/tmp/diff_new_pack.4G1phN/_new 2021-10-11 15:30:49.118768147 +0200 @@ -1,3 +1,5 @@ +# systemd service file for nscd + [Unit] Description=Name Service Cache Daemon After=sysinit.target ++++++ pthread-kill-fail-after-exit.patch ++++++ >From 3abf3bd4edc86fb28c099cc85203cb46a811e0b8 Mon Sep 17 00:00:00 2001 From: Florian Weimer <fwei...@redhat.com> Date: Mon, 13 Sep 2021 11:06:08 +0200 Subject: [PATCH] nptl: pthread_kill, pthread_cancel should not fail after exit (bug 19193) This closes one remaining race condition related to bug 12889: if the thread already exited on the kernel side, returning ESRCH is not correct because that error is reserved for the thread IDs (pthread_t values) whose lifetime has ended. In case of a kernel-side exit and a valid thread ID, no signal needs to be sent and cancellation does not have an effect, so just return 0. sysdeps/pthread/tst-kill4.c triggers undefined behavior and is removed with this commit. Reviewed-by: Adhemerval Zanella <adhemerval.zane...@linaro.org> (cherry picked from commit 8af8456004edbab71f8903a60a3cae442cf6fe69) --- NEWS | 1 + nptl/pthread_cancel.c | 9 ++- nptl/pthread_kill.c | 7 +- sysdeps/pthread/Makefile | 5 +- sysdeps/pthread/tst-kill4.c | 90 --------------------- sysdeps/pthread/tst-pthread_cancel-exited.c | 45 +++++++++++ sysdeps/pthread/tst-pthread_kill-exited.c | 46 +++++++++++ 7 files changed, 107 insertions(+), 96 deletions(-) delete mode 100644 sysdeps/pthread/tst-kill4.c create mode 100644 sysdeps/pthread/tst-pthread_cancel-exited.c create mode 100644 sysdeps/pthread/tst-pthread_kill-exited.c diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index cc25ff21f3..9bac6e3b76 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -62,10 +62,11 @@ __pthread_cancel (pthread_t th) { volatile struct pthread *pd = (volatile struct pthread *) th; - /* Make sure the descriptor is valid. */ - if (INVALID_TD_P (pd)) - /* Not a valid thread handle. */ - return ESRCH; + if (pd->tid == 0) + /* The thread has already exited on the kernel side. Its outcome + (regular exit, other cancelation) has already been + determined. */ + return 0; static int init_sigcancel = 0; if (atomic_load_relaxed (&init_sigcancel) == 0) diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c index f79a2b26fc..5d4c86f920 100644 --- a/nptl/pthread_kill.c +++ b/nptl/pthread_kill.c @@ -46,7 +46,12 @@ __pthread_kill_internal (pthread_t threadid, int signo) ? INTERNAL_SYSCALL_ERRNO (val) : 0); } else - val = ESRCH; + /* The kernel reports that the thread has exited. POSIX specifies + the ESRCH error only for the case when the lifetime of a thread + ID has ended, but calling pthread_kill on such a thread ID is + undefined in glibc. Therefore, do not treat kernel thread exit + as an error. */ + val = 0; return val; } diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile index 42f9fc5072..dedfa0d290 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -89,7 +89,7 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ tst-join8 tst-join9 tst-join10 tst-join11 tst-join12 tst-join13 \ tst-join14 tst-join15 \ tst-key1 tst-key2 tst-key3 tst-key4 \ - tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6 \ + tst-kill1 tst-kill2 tst-kill3 tst-kill5 tst-kill6 \ tst-locale1 tst-locale2 \ tst-memstream \ tst-mutex-errorcheck tst-mutex1 tst-mutex2 tst-mutex3 tst-mutex4 \ diff --git a/sysdeps/pthread/tst-kill4.c b/sysdeps/pthread/tst-kill4.c deleted file mode 100644 index 9563939792..0000000000 --- a/sysdeps/pthread/tst-kill4.c +++ /dev/null @@ -1,90 +0,0 @@ -/* Copyright (C) 2003-2021 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Ulrich Drepper <drep...@redhat.com>, 2003. - - 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 <errno.h> -#include <pthread.h> -#include <signal.h> -#include <stdio.h> -#include <stdlib.h> -#include <unistd.h> - - -static void * -tf (void *a) -{ - return NULL; -} - - -int -do_test (void) -{ - pthread_attr_t at; - if (pthread_attr_init (&at) != 0) - { - puts ("attr_create failed"); - exit (1); - } - - /* Limit thread stack size, because if it is too large, pthread_join - will free it immediately rather than put it into stack cache. */ - if (pthread_attr_setstacksize (&at, 2 * 1024 * 1024) != 0) - { - puts ("setstacksize failed"); - exit (1); - } - - pthread_t th; - if (pthread_create (&th, &at, tf, NULL) != 0) - { - puts ("create failed"); - exit (1); - } - - pthread_attr_destroy (&at); - - if (pthread_join (th, NULL) != 0) - { - puts ("join failed"); - exit (1); - } - - /* The following only works because we assume here something about - the implementation. Namely, that the memory allocated for the - thread descriptor is not going away, that the TID field is - cleared and therefore the signal is sent to process 0, and that - we can savely assume there is no other process with this ID at - that time. */ - int e = pthread_kill (th, 0); - if (e == 0) - { - puts ("pthread_kill succeeded"); - exit (1); - } - if (e != ESRCH) - { - puts ("pthread_kill didn't return ESRCH"); - exit (1); - } - - return 0; -} - - -#define TEST_FUNCTION do_test () -#include "../test-skeleton.c" ++++++ pthread-kill-race-thread-exit.patch ++++++ >From a8ac8c4725ddb1119764126a8674a04c9dd5aea8 Mon Sep 17 00:00:00 2001 From: Florian Weimer <fwei...@redhat.com> Date: Mon, 13 Sep 2021 11:06:08 +0200 Subject: [PATCH] nptl: Fix race between pthread_kill and thread exit (bug 12889) A new thread exit lock and flag are introduced. They are used to detect that the thread is about to exit or has exited in __pthread_kill_internal, and the signal is not sent in this case. The test sysdeps/pthread/tst-pthread_cancel-select-loop.c is derived from a downstream test originally written by Marek Polacek. Reviewed-by: Adhemerval Zanella <adhemerval.zane...@linaro.org> (cherry picked from commit 526c3cf11ee9367344b6b15d669e4c3cb461a2be) --- NEWS | 1 + nptl/allocatestack.c | 3 + nptl/descr.h | 6 + nptl/pthread_create.c | 14 ++ nptl/pthread_kill.c | 65 +++++---- sysdeps/pthread/Makefile | 2 + .../pthread/tst-pthread_cancel-select-loop.c | 87 +++++++++++++ sysdeps/pthread/tst-pthread_kill-exiting.c | 123 ++++++++++++++++++ 8 files changed, 276 insertions(+), 25 deletions(-) create mode 100644 sysdeps/pthread/tst-pthread_cancel-select-loop.c create mode 100644 sysdeps/pthread/tst-pthread_kill-exiting.c diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index cfe37a3443..50065bc9bd 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -32,6 +32,7 @@ #include <futex-internal.h> #include <kernel-features.h> #include <nptl-stack.h> +#include <libc-lock.h> /* Default alignment of stack. */ #ifndef STACK_ALIGN @@ -127,6 +128,8 @@ get_cached_stack (size_t *sizep, void **memp) /* No pending event. */ result->nextevent = NULL; + result->exiting = false; + __libc_lock_init (result->exit_lock); result->tls_state = (struct tls_internal_t) { 0 }; /* Clear the DTV. */ diff --git a/nptl/descr.h b/nptl/descr.h index c85778d449..4de84138fb 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -396,6 +396,12 @@ struct pthread PTHREAD_CANCEL_ASYNCHRONOUS). */ unsigned char canceltype; + /* Used in __pthread_kill_internal to detected a thread that has + exited or is about to exit. exit_lock must only be acquired + after blocking signals. */ + bool exiting; + int exit_lock; /* A low-level lock (for use with __libc_lock_init etc). */ + /* Used on strsignal. */ struct tls_internal_t tls_state; diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index d8ec299cb1..33b426fc68 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -37,6 +37,7 @@ #include <sys/single_threaded.h> #include <version.h> #include <clone_internal.h> +#include <futex-internal.h> #include <shlib-compat.h> @@ -485,6 +486,19 @@ start_thread (void *arg) /* This was the last thread. */ exit (0); + /* This prevents sending a signal from this thread to itself during + its final stages. This must come after the exit call above + because atexit handlers must not run with signals blocked. */ + __libc_signal_block_all (NULL); + + /* Tell __pthread_kill_internal that this thread is about to exit. + If there is a __pthread_kill_internal in progress, this delays + the thread exit until the signal has been queued by the kernel + (so that the TID used to send it remains valid). */ + __libc_lock_lock (pd->exit_lock); + pd->exiting = true; + __libc_lock_unlock (pd->exit_lock); + #ifndef __ASSUME_SET_ROBUST_LIST /* If this thread has any robust mutexes locked, handle them now. */ # if __PTHREAD_MUTEX_HAVE_PREV diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c index 5d4c86f920..fb7862eff7 100644 --- a/nptl/pthread_kill.c +++ b/nptl/pthread_kill.c @@ -16,6 +16,7 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#include <libc-lock.h> #include <unistd.h> #include <pthreadP.h> #include <shlib-compat.h> @@ -23,37 +24,51 @@ int __pthread_kill_internal (pthread_t threadid, int signo) { - pid_t tid; struct pthread *pd = (struct pthread *) threadid; - if (pd == THREAD_SELF) - /* It is a special case to handle raise() implementation after a vfork - call (which does not update the PD tid field). */ - tid = INLINE_SYSCALL_CALL (gettid); - else - /* Force load of pd->tid into local variable or register. Otherwise - if a thread exits between ESRCH test and tgkill, we might return - EINVAL, because pd->tid would be cleared by the kernel. */ - tid = atomic_forced_read (pd->tid); - - int val; - if (__glibc_likely (tid > 0)) { - pid_t pid = __getpid (); - - val = INTERNAL_SYSCALL_CALL (tgkill, pid, tid, signo); - val = (INTERNAL_SYSCALL_ERROR_P (val) - ? INTERNAL_SYSCALL_ERRNO (val) : 0); + /* Use the actual TID from the kernel, so that it refers to the + current thread even if called after vfork. There is no + signal blocking in this case, so that the signal is delivered + immediately, before __pthread_kill_internal returns: a signal + sent to the thread itself needs to be delivered + synchronously. (It is unclear if Linux guarantees the + delivery of all pending signals after unblocking in the code + below. POSIX only guarantees delivery of a single signal, + which may not be the right one.) */ + pid_t tid = INTERNAL_SYSCALL_CALL (gettid); + int ret = INTERNAL_SYSCALL_CALL (kill, tid, signo); + return INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0; } + + /* Block all signals, as required by pd->exit_lock. */ + sigset_t old_mask; + __libc_signal_block_all (&old_mask); + __libc_lock_lock (pd->exit_lock); + + int ret; + if (pd->exiting) + /* The thread is about to exit (or has exited). Sending the + signal is either not observable (the target thread has already + blocked signals at this point), or it will fail, or it might be + delivered to a new, unrelated thread that has reused the TID. + So do not actually send the signal. Do not report an error + because the threadid argument is still valid (the thread ID + lifetime has not ended), and ESRCH (for example) would be + misleading. */ + ret = 0; else - /* The kernel reports that the thread has exited. POSIX specifies - the ESRCH error only for the case when the lifetime of a thread - ID has ended, but calling pthread_kill on such a thread ID is - undefined in glibc. Therefore, do not treat kernel thread exit - as an error. */ - val = 0; + { + /* Using tgkill is a safety measure. pd->exit_lock ensures that + the target thread cannot exit. */ + ret = INTERNAL_SYSCALL_CALL (tgkill, __getpid (), pd->tid, signo); + ret = INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0; + } + + __libc_lock_unlock (pd->exit_lock); + __libc_signal_restore_set (&old_mask); - return val; + return ret; } int diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile index dedfa0d290..48dba717a1 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -119,5 +119,7 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ tst-unwind-thread \ tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \ + tst-pthread_cancel-select-loop \ + tst-pthread_kill-exiting \ tests-time64 := \ tst-abstime-time64 \ diff --git a/sysdeps/pthread/tst-pthread_cancel-select-loop.c b/sysdeps/pthread/tst-pthread_cancel-select-loop.c new file mode 100644 index 0000000000..a62087589c --- /dev/null +++ b/sysdeps/pthread/tst-pthread_cancel-select-loop.c @@ -0,0 +1,87 @@ +/* Test that pthread_cancel succeeds during thread exit. + Copyright (C) 2021 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/>. */ + +/* This test tries to trigger an internal race condition in + pthread_cancel, where the cancellation signal is sent after the + thread has begun the cancellation process. This can result in a + spurious ESRCH error. For the original bug 12889, the window is + quite small, so the bug was not reproduced in every run. */ + +#include <stdbool.h> +#include <stddef.h> +#include <support/check.h> +#include <support/xthread.h> +#include <support/xunistd.h> +#include <sys/select.h> +#include <unistd.h> + +/* Set to true by timeout_thread_function when the test should + terminate. */ +static bool timeout; + +static void * +timeout_thread_function (void *unused) +{ + usleep (5 * 1000 * 1000); + __atomic_store_n (&timeout, true, __ATOMIC_RELAXED); + return NULL; +} + +/* Used for blocking the select function below. */ +static int pipe_fds[2]; + +static void * +canceled_thread_function (void *unused) +{ + while (true) + { + fd_set rfs; + fd_set wfs; + fd_set efs; + FD_ZERO (&rfs); + FD_ZERO (&wfs); + FD_ZERO (&efs); + FD_SET (pipe_fds[0], &rfs); + + /* If the cancellation request is recognized early, the thread + begins exiting while the cancellation signal arrives. */ + select (FD_SETSIZE, &rfs, &wfs, &efs, NULL); + } + return NULL; +} + +static int +do_test (void) +{ + xpipe (pipe_fds); + pthread_t thr_timeout = xpthread_create (NULL, timeout_thread_function, NULL); + + while (!__atomic_load_n (&timeout, __ATOMIC_RELAXED)) + { + pthread_t thr = xpthread_create (NULL, canceled_thread_function, NULL); + xpthread_cancel (thr); + TEST_VERIFY (xpthread_join (thr) == PTHREAD_CANCELED); + } + + xpthread_join (thr_timeout); + xclose (pipe_fds[0]); + xclose (pipe_fds[1]); + return 0; +} + +#include <support/test-driver.c> diff --git a/sysdeps/pthread/tst-pthread_kill-exiting.c b/sysdeps/pthread/tst-pthread_kill-exiting.c new file mode 100644 index 0000000000..f803e94f11 --- /dev/null +++ b/sysdeps/pthread/tst-pthread_kill-exiting.c @@ -0,0 +1,123 @@ +/* Test that pthread_kill succeeds during thread exit. + Copyright (C) 2021 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/>. */ + +/* This test verifies that pthread_kill for a thread that is exiting + succeeds (with or without actually delivering the signal). */ + +#include <array_length.h> +#include <stdbool.h> +#include <stddef.h> +#include <support/xsignal.h> +#include <support/xthread.h> +#include <unistd.h> + +/* Set to true by timeout_thread_function when the test should + terminate. */ +static bool timeout; + +static void * +timeout_thread_function (void *unused) +{ + usleep (1000 * 1000); + __atomic_store_n (&timeout, true, __ATOMIC_RELAXED); + return NULL; +} + +/* Used to synchronize the sending threads with the target thread and + main thread. */ +static pthread_barrier_t barrier_1; +static pthread_barrier_t barrier_2; + +/* The target thread to which signals are to be sent. */ +static pthread_t target_thread; + +/* Set by the main thread to true after timeout has been set to + true. */ +static bool exiting; + +static void * +sender_thread_function (void *unused) +{ + while (true) + { + /* Wait until target_thread has been initialized. The target + thread and main thread participate in this barrier. */ + xpthread_barrier_wait (&barrier_1); + + if (exiting) + break; + + xpthread_kill (target_thread, SIGUSR1); + + /* Communicate that the signal has been sent. The main thread + participates in this barrier. */ + xpthread_barrier_wait (&barrier_2); + } + return NULL; +} + +static void * +target_thread_function (void *unused) +{ + target_thread = pthread_self (); + xpthread_barrier_wait (&barrier_1); + return NULL; +} + +static int +do_test (void) +{ + xsignal (SIGUSR1, SIG_IGN); + + pthread_t thr_timeout = xpthread_create (NULL, timeout_thread_function, NULL); + + pthread_t threads[4]; + xpthread_barrier_init (&barrier_1, NULL, array_length (threads) + 2); + xpthread_barrier_init (&barrier_2, NULL, array_length (threads) + 1); + + for (int i = 0; i < array_length (threads); ++i) + threads[i] = xpthread_create (NULL, sender_thread_function, NULL); + + while (!__atomic_load_n (&timeout, __ATOMIC_RELAXED)) + { + xpthread_create (NULL, target_thread_function, NULL); + + /* Wait for the target thread to be set up and signal sending to + start. */ + xpthread_barrier_wait (&barrier_1); + + /* Wait for signal sending to complete. */ + xpthread_barrier_wait (&barrier_2); + + xpthread_join (target_thread); + } + + exiting = true; + + /* Signal the sending threads to exit. */ + xpthread_create (NULL, target_thread_function, NULL); + xpthread_barrier_wait (&barrier_1); + + for (int i = 0; i < array_length (threads); ++i) + xpthread_join (threads[i]); + xpthread_join (thr_timeout); + + return 0; +} + +#include <support/test-driver.c> -- 2.33.0 ++++++ pthread-kill-return-esrch.patch ++++++ >From 73c7f5a87971de2797f261e1a447f68dce09284b Mon Sep 17 00:00:00 2001 From: Florian Weimer <fwei...@redhat.com> Date: Mon, 20 Sep 2021 14:56:08 +0200 Subject: [PATCH] nptl: pthread_kill needs to return ESRCH for old programs (bug 19193) The fix for bug 19193 breaks some old applications which appear to use pthread_kill to probe if a thread is still running, something that is not supported by POSIX. (cherry picked from commit 95dba35bf05e4a5d69dfae5e9c9d4df3646a7f93) --- nptl/pthread_kill.c | 37 ++++++++++++++++++----- sysdeps/pthread/tst-pthread_kill-exited.c | 21 +++++++++++-- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c index fb7862eff7..a44dc8f2d9 100644 --- a/nptl/pthread_kill.c +++ b/nptl/pthread_kill.c @@ -21,8 +21,11 @@ #include <pthreadP.h> #include <shlib-compat.h> -int -__pthread_kill_internal (pthread_t threadid, int signo) +/* Sends SIGNO to THREADID. If the thread is about to exit or has + already exited on the kernel side, return NO_TID. Otherwise return + 0 or an error code. */ +static int +__pthread_kill_implementation (pthread_t threadid, int signo, int no_tid) { struct pthread *pd = (struct pthread *) threadid; if (pd == THREAD_SELF) @@ -52,11 +55,8 @@ __pthread_kill_internal (pthread_t threadid, int signo) signal is either not observable (the target thread has already blocked signals at this point), or it will fail, or it might be delivered to a new, unrelated thread that has reused the TID. - So do not actually send the signal. Do not report an error - because the threadid argument is still valid (the thread ID - lifetime has not ended), and ESRCH (for example) would be - misleading. */ - ret = 0; + So do not actually send the signal. */ + ret = no_tid; else { /* Using tgkill is a safety measure. pd->exit_lock ensures that @@ -71,6 +71,15 @@ __pthread_kill_internal (pthread_t threadid, int signo) return ret; } +int +__pthread_kill_internal (pthread_t threadid, int signo) +{ + /* Do not report an error in the no-tid case because the threadid + argument is still valid (the thread ID lifetime has not ended), + and ESRCH (for example) would be misleading. */ + return __pthread_kill_implementation (threadid, signo, 0); +} + int __pthread_kill (pthread_t threadid, int signo) { @@ -81,6 +90,7 @@ __pthread_kill (pthread_t threadid, int signo) return __pthread_kill_internal (threadid, signo); } + /* Some architectures (for instance arm) might pull raise through libgcc, so avoid the symbol version if it ends up being used on ld.so. */ #if !IS_IN(rtld) @@ -88,6 +98,17 @@ libc_hidden_def (__pthread_kill) versioned_symbol (libc, __pthread_kill, pthread_kill, GLIBC_2_34); # if OTHER_SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_34) -compat_symbol (libc, __pthread_kill, pthread_kill, GLIBC_2_0); +/* Variant which returns ESRCH in the no-TID case, for backwards + compatibility. */ +int +attribute_compat_text_section +__pthread_kill_esrch (pthread_t threadid, int signo) +{ + if (__is_internal_signal (signo)) + return EINVAL; + + return __pthread_kill_implementation (threadid, signo, ESRCH); +} +compat_symbol (libc, __pthread_kill_esrch, pthread_kill, GLIBC_2_0); # endif #endif ++++++ pthread-kill-send-specific-thread.patch ++++++ >From 40bade26d5bcbda3d21fb598c5063d9df62de966 Mon Sep 17 00:00:00 2001 From: Florian Weimer <fwei...@redhat.com> Date: Fri, 1 Oct 2021 18:16:41 +0200 Subject: [PATCH] nptl: pthread_kill must send signals to a specific thread [BZ #28407] The choice between the kill vs tgkill system calls is not just about the TID reuse race, but also about whether the signal is sent to the whole process (and any thread in it) or to a specific thread. This was caught by the openposix test suite: LTP: openposix test suite - FAIL: SIGUSR1 is member of new thread pendingset. <https://gitlab.com/cki-project/kernel-tests/-/issues/764> Fixes commit 526c3cf11ee9367344b6b15d669e4c3cb461a2be ("nptl: Fix race between pthread_kill and thread exit (bug 12889)"). Reviewed-by: Carlos O'Donell <car...@redhat.com> Tested-by: Carlos O'Donell <car...@redhat.com> (cherry picked from commit eae81d70574e923ce3c59078b8df857ae192efa6) --- NEWS | 1 + nptl/pthread_kill.c | 4 +- sysdeps/pthread/Makefile | 1 + .../pthread/tst-pthread-raise-blocked-self.c | 92 +++++++++++++++++++ 4 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 sysdeps/pthread/tst-pthread-raise-blocked-self.c Index: glibc-2.34/nptl/pthread_kill.c =================================================================== --- glibc-2.34.orig/nptl/pthread_kill.c +++ glibc-2.34/nptl/pthread_kill.c @@ -40,7 +40,7 @@ __pthread_kill_implementation (pthread_t below. POSIX only guarantees delivery of a single signal, which may not be the right one.) */ pid_t tid = INTERNAL_SYSCALL_CALL (gettid); - int ret = INTERNAL_SYSCALL_CALL (kill, tid, signo); + int ret = INTERNAL_SYSCALL_CALL (tgkill, __getpid (), tid, signo); return INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0; } @@ -59,8 +59,6 @@ __pthread_kill_implementation (pthread_t ret = no_tid; else { - /* Using tgkill is a safety measure. pd->exit_lock ensures that - the target thread cannot exit. */ ret = INTERNAL_SYSCALL_CALL (tgkill, __getpid (), pd->tid, signo); ret = INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0; } Index: glibc-2.34/sysdeps/pthread/Makefile =================================================================== --- glibc-2.34.orig/sysdeps/pthread/Makefile +++ glibc-2.34/sysdeps/pthread/Makefile @@ -120,6 +120,7 @@ tests += tst-cnd-basic tst-mtx-trylock t tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \ tst-pthread-setuid-loop \ tst-pthread_cancel-select-loop \ + tst-pthread-raise-blocked-self \ tst-pthread_kill-exiting \ tests-time64 := \ Index: glibc-2.34/sysdeps/pthread/tst-pthread-raise-blocked-self.c =================================================================== --- /dev/null +++ glibc-2.34/sysdeps/pthread/tst-pthread-raise-blocked-self.c @@ -0,0 +1,92 @@ +/* Test that raise sends signal to current thread even if blocked. + Copyright (C) 2021 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 <signal.h> +#include <support/check.h> +#include <support/xsignal.h> +#include <support/xthread.h> +#include <pthread.h> +#include <unistd.h> + +/* Used to create a dummy thread ID distinct from all other thread + IDs. */ +static void * +noop (void *ignored) +{ + return NULL; +} + +static volatile pthread_t signal_thread; + +static void +signal_handler (int signo) +{ + signal_thread = pthread_self (); +} + +/* Used to ensure that waiting_thread has launched and can accept + signals. */ +static pthread_barrier_t barrier; + +static void * +waiting_thread (void *ignored) +{ + xpthread_barrier_wait (&barrier); + pause (); + return NULL; +} + +static int +do_test (void) +{ + xsignal (SIGUSR1, signal_handler); + xpthread_barrier_init (&barrier, NULL, 2); + + /* Distinct thread ID value to */ + pthread_t dummy = xpthread_create (NULL, noop, NULL); + signal_thread = dummy; + + pthread_t helper = xpthread_create (NULL, waiting_thread, NULL); + + /* Make sure that the thread is running. */ + xpthread_barrier_wait (&barrier); + + /* Block signals on this thread. */ + sigset_t set; + sigfillset (&set); + xpthread_sigmask (SIG_BLOCK, &set, NULL); + + /* Send the signal to this thread. It must not be delivered. */ + raise (SIGUSR1); + TEST_VERIFY (signal_thread == dummy); + + /* Wait a bit to give a chance for signal delivery (increases + chances of failure with bug 28407). */ + usleep (50 * 1000); + + /* Unblocking should cause synchronous delivery of the signal. */ + xpthread_sigmask (SIG_UNBLOCK, &set, NULL); + TEST_VERIFY (signal_thread == pthread_self ()); + + xpthread_cancel (helper); + xpthread_join (helper); + xpthread_join (dummy); + return 0; +} + +#include <support/test-driver.c> ++++++ pthread-mutexattr-getrobust-np-type.patch ++++++ >From 8b8a1d0b7375c547ae905917a03743ed6759c5bc Mon Sep 17 00:00:00 2001 From: Florian Weimer <fwei...@redhat.com> Date: Tue, 21 Sep 2021 07:12:56 +0200 Subject: [PATCH] nptl: Fix type of pthread_mutexattr_getrobust_np, pthread_mutexattr_setrobust_np (bug 28036) Reviewed-by: Carlos O'Donell <car...@redhat.com> Tested-by: Carlos O'Donell <car...@redhat.com> (cherry picked from commit f3e664563361dc17530113b3205998d1f19dc4d9) --- NEWS | 1 + sysdeps/nptl/pthread.h | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h index f1b7f2bdc6..43146e91c9 100644 --- a/sysdeps/nptl/pthread.h +++ b/sysdeps/nptl/pthread.h @@ -933,7 +933,7 @@ extern int pthread_mutexattr_getrobust (const pthread_mutexattr_t *__attr, # ifdef __USE_GNU # ifdef __REDIRECT_NTH extern int __REDIRECT_NTH (pthread_mutexattr_getrobust_np, - (pthread_mutex_t *, int *), + (pthread_mutexattr_t *, int *), pthread_mutexattr_getrobust) __nonnull ((1)) __attribute_deprecated_msg__ ("\ pthread_mutexattr_getrobust_np is deprecated, use pthread_mutexattr_getrobust"); @@ -949,7 +949,7 @@ extern int pthread_mutexattr_setrobust (pthread_mutexattr_t *__attr, # ifdef __USE_GNU # ifdef __REDIRECT_NTH extern int __REDIRECT_NTH (pthread_mutexattr_setrobust_np, - (pthread_mutex_t *, int), + (pthread_mutexattr_t *, int), pthread_mutexattr_setrobust) __nonnull ((1)) __attribute_deprecated_msg__ ("\ pthread_mutexattr_setrobust_np is deprecated, use pthread_mutexattr_setrobust"); -- 2.33.0 ++++++ setxid-deadlock-blocked-signals.patch ++++++ >From 33adeaa3e2b9143c38884bc5aa65ded222ed274e Mon Sep 17 00:00:00 2001 From: Florian Weimer <fwei...@redhat.com> Date: Thu, 23 Sep 2021 09:55:54 +0200 Subject: [PATCH] nptl: Avoid setxid deadlock with blocked signals in thread exit [BZ #28361] As part of the fix for bug 12889, signals are blocked during thread exit, so that application code cannot run on the thread that is about to exit. This would cause problems if the application expected signals to be delivered after the signal handler revealed the thread to still exist, despite pthread_kill can no longer be used to send signals to it. However, glibc internally uses the SIGSETXID signal in a way that is incompatible with signal blocking, due to the way the setxid handshake delays thread exit until the setxid operation has completed. With a blocked SIGSETXID, the handshake can never complete, causing a deadlock. As a band-aid, restore the previous handshake protocol by not blocking SIGSETXID during thread exit. The new test sysdeps/pthread/tst-pthread-setuid-loop.c is based on a downstream test by Martin Osvald. Reviewed-by: Carlos O'Donell <car...@redhat.com> Tested-by: Carlos O'Donell <car...@redhat.com> (cherry picked from commit 2849e2f53311b66853cb5159b64cba2bddbfb854) --- NEWS | 1 + nptl/pthread_create.c | 12 ++++- sysdeps/pthread/Makefile | 1 + sysdeps/pthread/tst-pthread-setuid-loop.c | 61 +++++++++++++++++++++++ 4 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 sysdeps/pthread/tst-pthread-setuid-loop.c Index: glibc-2.34/nptl/pthread_create.c =================================================================== --- glibc-2.34.orig/nptl/pthread_create.c +++ glibc-2.34/nptl/pthread_create.c @@ -488,8 +488,16 @@ start_thread (void *arg) /* This prevents sending a signal from this thread to itself during its final stages. This must come after the exit call above - because atexit handlers must not run with signals blocked. */ - __libc_signal_block_all (NULL); + because atexit handlers must not run with signals blocked. + + Do not block SIGSETXID. The setxid handshake below expects the + signal to be delivered. (SIGSETXID cannot run application code, + nor does it use pthread_kill.) Reuse the pd->sigmask space for + computing the signal mask, to save stack space. */ + __sigfillset (&pd->sigmask); + __sigdelset (&pd->sigmask, SIGSETXID); + INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &pd->sigmask, NULL, + __NSIG_BYTES); /* Tell __pthread_kill_internal that this thread is about to exit. If there is a __pthread_kill_internal in progress, this delays Index: glibc-2.34/sysdeps/pthread/Makefile =================================================================== --- glibc-2.34.orig/sysdeps/pthread/Makefile +++ glibc-2.34/sysdeps/pthread/Makefile @@ -118,6 +118,7 @@ tests += tst-cnd-basic tst-mtx-trylock t tst-unload \ tst-unwind-thread \ tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \ + tst-pthread-setuid-loop \ tst-pthread_cancel-select-loop \ tst-pthread_kill-exiting \ Index: glibc-2.34/sysdeps/pthread/tst-pthread-setuid-loop.c =================================================================== --- /dev/null +++ glibc-2.34/sysdeps/pthread/tst-pthread-setuid-loop.c @@ -0,0 +1,61 @@ +/* Test that setuid, pthread_create, thread exit do not deadlock (bug 28361). + Copyright (C) 2021 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 <support/check.h> +#include <support/xthread.h> +#include <unistd.h> + +/* How many threads to launch during each iteration. */ +enum { threads = 4 }; + +/* How many iterations to perform. This value seems to reproduce + bug 28361 in a bout one in three runs. */ +enum { iterations = 5000 }; + +/* Cache of the real user ID used by setuid_thread. */ +static uid_t uid; + +/* Start routine for the threads. */ +static void * +setuid_thread (void *closure) +{ + TEST_COMPARE (setuid (uid), 0); + return NULL; +} + +static int +do_test (void) +{ + /* The setxid machinery is still invoked even if the UID is + unchanged. (The kernel might reset other credentials as part of + the system call.) */ + uid = getuid (); + + for (int i = 0; i < iterations; ++i) + { + pthread_t thread_ids[threads]; + for (int j = 0; j < threads; ++j) + thread_ids[j] = xpthread_create (NULL, setuid_thread, NULL); + for (int j = 0; j < threads; ++j) + xpthread_join (thread_ids[j]); + } + + return 0; +} + +#include <support/test-driver.c> ++++++ sysconf-nprocessors-affinity.patch ++++++ >From 822662cf2a4b170ade4c5342f035d68815a03276 Mon Sep 17 00:00:00 2001 From: Adhemerval Zanella <adhemerval.zane...@linaro.org> Date: Mon, 6 Sep 2021 14:19:51 -0300 Subject: [PATCH] linux: Revert the use of sched_getaffinity on get_nproc (BZ #28310) The use of sched_getaffinity on get_nproc and sysconf (_SC_NPROCESSORS_ONLN) done in 903bc7dcc2acafc40 (BZ #27645) breaks the top command in common hypervisor configurations and also other monitoring tools. The main issue using sched_getaffinity changed the symbols semantic from system-wide scope of online CPUs to per-process one (which can be changed with kernel cpusets or book parameters in VM). This patch reverts mostly of the 903bc7dcc2acafc40, with the exceptions: * No more cached values and atomic updates, since they are inherent racy. * No /proc/cpuinfo fallback, since /proc/stat is already used and it would require to revert more arch-specific code. * The alloca is replace with a static buffer of 1024 bytes. So the implementation first consult the sysfs, and fallbacks to procfs. Checked on x86_64-linux-gnu. Reviewed-by: Florian Weimer <fwei...@redhat.com> (cherry picked from commit 342298278eabc75baabcaced110a11a02c3d3580) Index: glibc-2.34/include/sys/sysinfo.h =================================================================== --- glibc-2.34.orig/include/sys/sysinfo.h +++ glibc-2.34/include/sys/sysinfo.h @@ -9,10 +9,15 @@ extern int __get_nprocs_conf (void); libc_hidden_proto (__get_nprocs_conf) -/* Return number of available processors. */ +/* Return number of available processors (not all of them will be + available to the caller process). */ extern int __get_nprocs (void); libc_hidden_proto (__get_nprocs) +/* Return the number of available processors which the process can + be scheduled. */ +extern int __get_nprocs_sched (void) attribute_hidden; + /* Return number of physical pages of memory in the system. */ extern long int __get_phys_pages (void); libc_hidden_proto (__get_phys_pages) Index: glibc-2.34/malloc/arena.c =================================================================== --- glibc-2.34.orig/malloc/arena.c +++ glibc-2.34/malloc/arena.c @@ -879,7 +879,7 @@ arena_get2 (size_t size, mstate avoid_ar narenas_limit = mp_.arena_max; else if (narenas > mp_.arena_test) { - int n = __get_nprocs (); + int n = __get_nprocs_sched (); if (n >= 1) narenas_limit = NARENAS_FROM_NCORES (n); Index: glibc-2.34/misc/getsysstats.c =================================================================== --- glibc-2.34.orig/misc/getsysstats.c +++ glibc-2.34/misc/getsysstats.c @@ -45,6 +45,12 @@ weak_alias (__get_nprocs, get_nprocs) link_warning (get_nprocs, "warning: get_nprocs will always return 1") +int +__get_nprocs_sched (void) +{ + return 1; +} + long int __get_phys_pages (void) { Index: glibc-2.34/posix/Makefile =================================================================== --- glibc-2.34.orig/posix/Makefile +++ glibc-2.34/posix/Makefile @@ -107,7 +107,8 @@ tests := test-errno tstgetopt testfnm r tst-sysconf-empty-chroot tst-glob_symlinks tst-fexecve \ tst-glob-tilde test-ssize-max tst-spawn4 bug-regex37 \ bug-regex38 tst-regcomp-truncated tst-spawn-chdir \ - tst-wordexp-nocmd tst-execveat tst-spawn5 + tst-wordexp-nocmd tst-execveat tst-spawn5 \ + tst-sched_getaffinity # Test for the glob symbol version that was replaced in glibc 2.27. ifeq ($(have-GLIBC_2.26)$(build-shared),yesyes) Index: glibc-2.34/posix/tst-sched_getaffinity.c =================================================================== --- /dev/null +++ glibc-2.34/posix/tst-sched_getaffinity.c @@ -0,0 +1,48 @@ +/* Tests for sched_getaffinity with large buffers. + Copyright (C) 2021 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 <sched.h> +#include <support/check.h> + +/* NB: this test may fail on system with more than 32k cpus. */ + +static int +do_test (void) +{ + /* The values are larger than the default cpu_set_t. */ + const int bufsize[] = { 1<<11, 1<<12, 1<<13, 1<<14, 1<<15, 1<<16, 1<<17 }; + int cpucount[array_length (bufsize)]; + + for (int i = 0; i < array_length (bufsize); i++) + { + cpu_set_t *cpuset = CPU_ALLOC (bufsize[i]); + TEST_VERIFY (cpuset != NULL); + size_t size = CPU_ALLOC_SIZE (bufsize[i]); + TEST_COMPARE (sched_getaffinity (0, size, cpuset), 0); + cpucount[i] = CPU_COUNT_S (size, cpuset); + CPU_FREE (cpuset); + } + + for (int i = 0; i < array_length (cpucount) - 1; i++) + TEST_COMPARE (cpucount[i], cpucount[i + 1]); + + return 0; +} + +#include <support/test-driver.c> Index: glibc-2.34/sysdeps/mach/getsysstats.c =================================================================== --- glibc-2.34.orig/sysdeps/mach/getsysstats.c +++ glibc-2.34/sysdeps/mach/getsysstats.c @@ -62,6 +62,12 @@ __get_nprocs (void) libc_hidden_def (__get_nprocs) weak_alias (__get_nprocs, get_nprocs) +int +__get_nprocs_sched (void) +{ + return __get_nprocs (); +} + /* Return the number of physical pages on the system. */ long int __get_phys_pages (void) Index: glibc-2.34/sysdeps/unix/sysv/linux/getsysstats.c =================================================================== --- glibc-2.34.orig/sysdeps/unix/sysv/linux/getsysstats.c +++ glibc-2.34/sysdeps/unix/sysv/linux/getsysstats.c @@ -18,6 +18,8 @@ <https://www.gnu.org/licenses/>. */ #include <array_length.h> +#include <assert.h> +#include <ctype.h> #include <dirent.h> #include <errno.h> #include <ldsodefs.h> @@ -29,61 +31,162 @@ #include <sys/sysinfo.h> #include <sysdep.h> -/* Compute the population count of the entire array. */ -static int -__get_nprocs_count (const unsigned long int *array, size_t length) +int +__get_nprocs_sched (void) { - int count = 0; - for (size_t i = 0; i < length; ++i) - if (__builtin_add_overflow (count, __builtin_popcountl (array[i]), - &count)) - return INT_MAX; - return count; -} + enum + { + max_num_cpus = 32768, + cpu_bits_size = CPU_ALLOC_SIZE (32768) + }; -/* __get_nprocs with a large buffer. */ -static int -__get_nprocs_large (void) -{ - /* This code cannot use scratch_buffer because it is used during - malloc initialization. */ - size_t pagesize = GLRO (dl_pagesize); - unsigned long int *page = __mmap (0, pagesize, PROT_READ | PROT_WRITE, - MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); - if (page == MAP_FAILED) - return 2; - int r = INTERNAL_SYSCALL_CALL (sched_getaffinity, 0, pagesize, page); - int count; + /* This cannot use malloc because it is used on malloc initialization. */ + __cpu_mask cpu_bits[cpu_bits_size / sizeof (__cpu_mask)]; + int r = INTERNAL_SYSCALL_CALL (sched_getaffinity, 0, cpu_bits_size, + cpu_bits); if (r > 0) - count = __get_nprocs_count (page, pagesize / sizeof (unsigned long int)); + return CPU_COUNT_S (cpu_bits_size, (cpu_set_t*) cpu_bits); else if (r == -EINVAL) - /* One page is still not enough to store the bits. A more-or-less - arbitrary value. This assumes t hat such large systems never - happen in practice. */ - count = GLRO (dl_pagesize) * CHAR_BIT; - else - count = 2; - __munmap (page, GLRO (dl_pagesize)); - return count; + /* The input buffer is still not enough to store the number of cpus. This + is an arbitrary values assuming such systems should be rare and there + is no offline cpus. */ + return max_num_cpus; + /* Some other error. 2 is conservative (not a uniprocessor system, so + atomics are needed). */ + return 2; } +static char * +next_line (int fd, char *const buffer, char **cp, char **re, + char *const buffer_end) +{ + char *res = *cp; + char *nl = memchr (*cp, '\n', *re - *cp); + if (nl == NULL) + { + if (*cp != buffer) + { + if (*re == buffer_end) + { + memmove (buffer, *cp, *re - *cp); + *re = buffer + (*re - *cp); + *cp = buffer; + + ssize_t n = __read_nocancel (fd, *re, buffer_end - *re); + if (n < 0) + return NULL; + + *re += n; + + nl = memchr (*cp, '\n', *re - *cp); + while (nl == NULL && *re == buffer_end) + { + /* Truncate too long lines. */ + *re = buffer + 3 * (buffer_end - buffer) / 4; + n = __read_nocancel (fd, *re, buffer_end - *re); + if (n < 0) + return NULL; + + nl = memchr (*re, '\n', n); + **re = '\n'; + *re += n; + } + } + else + nl = memchr (*cp, '\n', *re - *cp); + + res = *cp; + } + + if (nl == NULL) + nl = *re - 1; + } + + *cp = nl + 1; + assert (*cp <= *re); + + return res == *re ? NULL : res; +} + + int __get_nprocs (void) { - /* Fast path for most systems. The kernel expects a buffer size - that is a multiple of 8. */ - unsigned long int small_buffer[1024 / CHAR_BIT / sizeof (unsigned long int)]; - int r = INTERNAL_SYSCALL_CALL (sched_getaffinity, 0, - sizeof (small_buffer), small_buffer); - if (r > 0) - return __get_nprocs_count (small_buffer, r / sizeof (unsigned long int)); - else if (r == -EINVAL) - /* The kernel requests a larger buffer to store the data. */ - return __get_nprocs_large (); - else - /* Some other error. 2 is conservative (not a uniprocessor - system, so atomics are needed). */ - return 2; + enum { buffer_size = 1024 }; + char buffer[buffer_size]; + char *buffer_end = buffer + buffer_size; + char *cp = buffer_end; + char *re = buffer_end; + + const int flags = O_RDONLY | O_CLOEXEC; + /* This file contains comma-separated ranges. */ + int fd = __open_nocancel ("/sys/devices/system/cpu/online", flags); + char *l; + int result = 0; + if (fd != -1) + { + l = next_line (fd, buffer, &cp, &re, buffer_end); + if (l != NULL) + do + { + char *endp; + unsigned long int n = strtoul (l, &endp, 10); + if (l == endp) + { + result = 0; + break; + } + + unsigned long int m = n; + if (*endp == '-') + { + l = endp + 1; + m = strtoul (l, &endp, 10); + if (l == endp) + { + result = 0; + break; + } + } + + result += m - n + 1; + + l = endp; + if (l < re && *l == ',') + ++l; + } + while (l < re && *l != '\n'); + + __close_nocancel_nostatus (fd); + + if (result > 0) + return result; + } + + cp = buffer_end; + re = buffer_end; + + /* Default to an SMP system in case we cannot obtain an accurate + number. */ + result = 2; + + fd = __open_nocancel ("/proc/stat", flags); + if (fd != -1) + { + result = 0; + + while ((l = next_line (fd, buffer, &cp, &re, buffer_end)) != NULL) + /* The current format of /proc/stat has all the cpu* entries + at the front. We assume here that stays this way. */ + if (strncmp (l, "cpu", 3) != 0) + break; + else if (isdigit (l[3])) + ++result; + + __close_nocancel_nostatus (fd); + } + + return result; } libc_hidden_def (__get_nprocs) weak_alias (__get_nprocs, get_nprocs) ++++++ x86-string-control-test.patch ++++++ >From f2413f2710d5d5cc884b413b83fcf8198e3717fa Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.to...@gmail.com> Date: Sat, 28 Aug 2021 06:10:38 -0700 Subject: [PATCH] x86-64: Use testl to check __x86_string_control Use testl, instead of andl, to check __x86_string_control to avoid updating __x86_string_control. Reviewed-by: Carlos O'Donell <car...@redhat.com> (cherry picked from commit 3c8b9879cab6d41787bc5b14c1748f62fd6d0e5f) --- sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S b/sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S index 9f02624375..abde8438d4 100644 --- a/sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S +++ b/sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S @@ -325,7 +325,7 @@ L(movsb): /* Avoid slow backward REP MOVSB. */ jb L(more_8x_vec_backward) # if AVOID_SHORT_DISTANCE_REP_MOVSB - andl $X86_STRING_CONTROL_AVOID_SHORT_DISTANCE_REP_MOVSB, __x86_string_control(%rip) + testl $X86_STRING_CONTROL_AVOID_SHORT_DISTANCE_REP_MOVSB, __x86_string_control(%rip) jz 3f movq %rdi, %rcx subq %rsi, %rcx @@ -333,7 +333,7 @@ L(movsb): # endif 1: # if AVOID_SHORT_DISTANCE_REP_MOVSB - andl $X86_STRING_CONTROL_AVOID_SHORT_DISTANCE_REP_MOVSB, __x86_string_control(%rip) + testl $X86_STRING_CONTROL_AVOID_SHORT_DISTANCE_REP_MOVSB, __x86_string_control(%rip) jz 3f movq %rsi, %rcx subq %rdi, %rcx -- 2.33.0