Open Build Service

_service:tar_scm_kernel_repo:libcap.spec Changed

_service:tar_scm_kernel_repo:backport-Avoid-a-deadlock-in-forked-psx-thread-exit.patch Added

@@ -0,0 +1,60 @@
+From 7617af6b0754da00c1094215ee7828d6592f8ade Mon Sep 17 00:00:00 2001
+From: "Andrew G. Morgan" <morgan@kernel.org>
+Date: Sun, 10 Apr 2022 15:39:14 -0700
+Subject: [PATCH] Avoid a deadlock in forked psx thread exit.
+
+go/captree was seeing lots of libcap_psx_test processes hanging around.
+It turns out that the newly added _psx_cleanup() function was deadlocking
+because inside a forked processes the psx_tracker.state was _PSX_INFORK
+and never _PSX_IDLE.
+
+This completes the fix for:
+
+  https://bugzilla.kernel.org/show_bug.cgi?id=215551
+
+Signed-off-by: Andrew G. Morgan <morgan@kernel.org>
+---
+ psx/psx.c | 13 ++++++++++---
+ 1 file changed, 10 insertions(+), 3 deletions(-)
+
+diff --git a/psx/psx.c b/psx/psx.c
+index 1876978..d9c0485 100644
+--- a/psx/psx.c
++++ b/psx/psx.c
+@@ -287,7 +287,9 @@ static void psx_unlock(void)
+ }
+ 
+ /*
+- * under lock perform a state transition.
++ * under lock perform a state transition. Changing state is generally
++ * done via this function. However, there is a single exception in
++ * _psx_cleanup().
+  */
+ static void psx_new_state(psx_tracker_state_t was, psx_tracker_state_t is)
+ {
+@@ -351,7 +353,7 @@ static void _psx_forked_child(void) {
+      *
+      * We do this because the glibc man page for fork() suggests that
+      * only a subset of things will work post fork(). Specifically,
+-     * only a "async-signal-safe functions (see signal- safety(7))
++     * only a "async-signal-safe functions (see signal-safety(7))
+      * until such time as it calls execve(2)" can be relied upon. That
+      * man page suggests that you can't expect mutexes to work: "not
+      * async-signal-safe because it uses pthread_mutex_lock(3)
+@@ -733,7 +735,12 @@ static void _psx_cleanup(void) {
+      * never leave this state since this cleanup is only done at
+      * program exit.
+      */
+-    psx_new_state(_PSX_IDLE, _PSX_EXITING);
++    psx_lock();
++    while (psx_tracker.state != _PSX_IDLE && psx_tracker.state != _PSX_INFORK) {
++	pthread_cond_wait(&psx_tracker.cond, &psx_tracker.state_mu);
++    }
++    psx_tracker.state = _PSX_EXITING;
++    psx_unlock();
+ 
+     for (ref = psx_tracker.root; ref; ref = next) {
+ 	next = ref->next;
+-- 
+2.27.0
+

_service:tar_scm_kernel_repo:backport-psx-free-allocated-memory-at-exit.patch Added

@@ -0,0 +1,102 @@
+From 66a8a1421e4520e9dda0a46704e25bafb989b1ae Mon Sep 17 00:00:00 2001
+From: "Andrew G. Morgan" <morgan@kernel.org>
+Date: Sat, 5 Feb 2022 17:26:05 -0800
+Subject: [PATCH] psx: free allocated memory at exit.
+
+Kalen Hall reported that Valgrind detected a memory leak associated
+with a multi-threaded program linked against libcap and libpsx.
+
+  https://bugzilla.kernel.org/show_bug.cgi?id=215551
+
+I've been unable to validate this myself with valgrind (likely holding
+it wrong), but did explore psx for allocated memory and via fprintf's
+convinced myself that this change should pair all calloc()s with a
+corresponding free().
+
+Signed-off-by: Andrew G. Morgan <morgan@kernel.org>
+---
+ psx/psx.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
+ 1 file changed, 43 insertions(+), 1 deletion(-)
+
+diff --git a/psx/psx.c b/psx/psx.c
+index 6b669ae..1876978 100644
+--- a/psx/psx.c
++++ b/psx/psx.c
+@@ -29,6 +29,26 @@
+ 
+ #include "psx_syscall.h"
+ 
++#ifdef _PSX_DEBUG_MEMORY
++
++static void *_psx_calloc(const char *file, const int line,
++			 size_t nmemb, size_t size) {
++    void *ptr = calloc(nmemb, size);
++    fprintf(stderr, "psx:%d:%s:%d: calloc(%ld, %ld) -> %p\n", gettid(),
++	    file, line, (long int)nmemb, (long int)size, ptr);
++    return ptr;
++}
++
++static void _psx_free(const char *file, const int line, void *ptr) {
++    fprintf(stderr, "psx:%d:%s:%d: free(%p)\n", gettid(), file, line, ptr);
++    return free(ptr);
++}
++
++#define calloc(a, b)  _psx_calloc(__FILE__, __LINE__, a, b)
++#define free(a)       _psx_free(__FILE__, __LINE__, a)
++
++#endif /* def _PSX_DEBUG_MEMORY */
++
+ /*
+  * psx_load_syscalls() can be weakly defined in dependent libraries to
+  * provide a mechanism for a library to optionally leverage this psx
+@@ -177,6 +197,7 @@ static void psx_posix_syscall_actor(int signum, siginfo_t *info, void *ignore) {
+  * Some forward declarations for the initialization
+  * psx_syscall_start() routine.
+  */
++static void _psx_cleanup(void);
+ static void _psx_prepare_fork(void);
+ static void _psx_fork_completed(void);
+ static void _psx_forked_child(void);
+@@ -240,6 +261,7 @@ static void psx_syscall_start(void) {
+ 
+     psx_confirm_sigaction();
+     psx_do_registration(); /* register the main thread. */
++    atexit(_psx_cleanup);
+ 
+     psx_tracker.initialized = 1;
+ }
+@@ -420,7 +442,7 @@ static void _psx_exiting(void *node) {
+     pthread_sigmask(SIG_SETMASK, &orig_sigbits, NULL);
+ 
+     /*
+-     * Allow the rest of the psx system carry on as per normal.
++     * Allow the rest of the psx system to carry on as per normal.
+      */
+     psx_new_state(_PSX_EXITING, _PSX_IDLE);
+ }
+@@ -699,2 +721,22 @@ defer:
+     return ret;
+ }
++/*
++ * _psx_cleanup its called when the program exits. It is used to free
++ * any memory used by the thread tracker.
++ */
++static void _psx_cleanup(void) {
++    registered_thread_t *ref, *next;
++
++    /*
++     * We enter the exiting state. Unlike exiting a single thread we
++     * never leave this state since this cleanup is only done at
++     * program exit.
++     */
++    psx_new_state(_PSX_IDLE, _PSX_EXITING);
++
++    for (ref = psx_tracker.root; ref; ref = next) {
++	next = ref->next;
++	psx_do_unregister(ref);
++    }
++}
++
+-- 
+2.27.0
+

Changes of Revision 8