[PR PATCH] musl: backport changes for forking from multithreaded processes

[PR PATCH] musl: backport changes for forking from multithreaded processes

[Gorggg]

[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]

There is a new pull request by Gorggg against master on the void-packages repository

https://github.com/Gorggg/void-packages musl-mt-fork-backport
https://github.com/void-linux/void-packages/pull/37833

musl: backport changes for forking from multithreaded processes
https://git.musl-libc.org/cgit/musl/commit/?id=167390f05564e0a4d3fcb4329377fd7743267560

This change allows child processes that have been forked from multithreaded parents to use non-AS-safe functions, as described in the original commit. Otherwise, there is a risk of deadlocks and/or state corruption. This makes fork() itself non-AS-safe, also as described. Minimal modifications were made to the original changes in order to accommodate different types and locations of files at different points in musl's history.

I encountered such a deadlock in a child process in the SANE pixma driver, which creates a reader process for communicating the device, which itself allocates memory using malloc().

These changes introduced no new test failures in [libc-test](https://wiki.musl-libc.org/libc-test.html).

Thank you for looking at this. If there is anything more I could do to test these changes, please let me know.

<!-- Uncomment relevant sections and delete options which are not applicable -->

#### Testing the changes
- I tested the changes in this PR: **YES**

#### Local build testing
- I built this PR locally for my native architecture, (x86_64-musl)


A patch file from https://github.com/void-linux/void-packages/pull/37833.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-musl-mt-fork-backport-37833.patch --]
[-- Type: text/x-diff, Size: 13050 bytes --]

From 8d66b772d932762c1f41c5cf1a72a96a61a4e70b Mon Sep 17 00:00:00 2001
From: George Matsumura <gmmatsumura01@bvsd.org>
Date: Mon, 4 Jul 2022 04:57:43 -0600
Subject: [PATCH] musl: backport changes for forking from multithreaded
 processes

https://git.musl-libc.org/cgit/musl/commit/?id=167390f05564e0a4d3fcb4329377fd7743267560

This change allows child processes that have been forked from
multithreaded parents to use non-AS-safe functions, as described
in the original commit. Otherwise, there is a risk of deadlocks
and/or state corruption. This makes fork() itself non-AS-safe,
also as described.
---
 srcpkgs/musl/patches/mt_fork.patch | 410 +++++++++++++++++++++++++++++
 srcpkgs/musl/template              |   2 +-
 2 files changed, 411 insertions(+), 1 deletion(-)
 create mode 100644 srcpkgs/musl/patches/mt_fork.patch

diff --git a/srcpkgs/musl/patches/mt_fork.patch b/srcpkgs/musl/patches/mt_fork.patch
new file mode 100644
index 000000000000..f36135a6765d
--- /dev/null
+++ b/srcpkgs/musl/patches/mt_fork.patch
@@ -0,0 +1,410 @@
+diff -ruN a/ldso/dynlink.c b/ldso/dynlink.c
+--- a/ldso/dynlink.c	2019-10-13 15:58:27.000000000 -0600
++++ b/ldso/dynlink.c	2022-07-04 04:30:37.592492602 -0600
+@@ -21,6 +21,7 @@
+ #include <semaphore.h>
+ #include <sys/membarrier.h>
+ #include "pthread_impl.h"
++#include "fork_impl.h"
+ #include "libc.h"
+ #include "dynlink.h"
+ #include "malloc_impl.h"
+@@ -1399,6 +1400,17 @@
+ 	}
+ }
+ 
++void __ldso_atfork(int who)
++{
++	if (who<0) {
++		pthread_rwlock_wrlock(&lock);
++		pthread_mutex_lock(&init_fini_lock);
++	} else {
++		pthread_mutex_unlock(&init_fini_lock);
++		pthread_rwlock_unlock(&lock);
++	}
++}
++
+ static struct dso **queue_ctors(struct dso *dso)
+ {
+ 	size_t cnt, qpos, spos, i;
+@@ -1457,6 +1469,13 @@
+ 	}
+ 	queue[qpos] = 0;
+ 	for (i=0; i<qpos; i++) queue[i]->mark = 0;
++	for (i=0; i<qpos; i++)
++		if (queue[i]->ctor_visitor < 0) {
++			error("State of %s is inconsistent due to multithreaded fork\n",
++				queue[i]->name);
++			free(queue);
++			if (runtime) longjmp(*rtld_fail, 1);
++		}
+ 
+ 	return queue;
+ }
+diff -ruN a/src/exit/at_quick_exit.c b/src/exit/at_quick_exit.c
+--- a/src/exit/at_quick_exit.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/exit/at_quick_exit.c	2022-07-04 03:50:18.609291784 -0600
+@@ -1,12 +1,14 @@
+ #include <stdlib.h>
+ #include "libc.h"
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ #define COUNT 32
+ 
+ static void (*funcs[COUNT])(void);
+ static int count;
+ static volatile int lock[1];
++volatile int *const __at_quick_exit_lockptr = lock;
+ 
+ void __funcs_on_quick_exit()
+ {
+diff -ruN a/src/exit/atexit.c b/src/exit/atexit.c
+--- a/src/exit/atexit.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/exit/atexit.c	2022-07-04 03:51:02.474292436 -0600
+@@ -2,6 +2,7 @@
+ #include <stdint.h>
+ #include "libc.h"
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ /* Ensure that at least 32 atexit handlers can be registered without malloc */
+ #define COUNT 32
+@@ -15,6 +16,7 @@
+ 
+ static int slot;
+ static volatile int lock[1];
++volatile int *const __atexit_lockptr = lock;
+ 
+ void __funcs_on_exit()
+ {
+diff -ruN a/src/internal/fork_impl.h b/src/internal/fork_impl.h
+--- a/src/internal/fork_impl.h	1969-12-31 17:00:00.000000000 -0700
++++ b/src/internal/fork_impl.h	2022-07-04 03:53:11.639719874 -0600
+@@ -0,0 +1,19 @@
++#include <features.h>
++
++extern hidden volatile int *const __at_quick_exit_lockptr;
++extern hidden volatile int *const __atexit_lockptr;
++extern hidden volatile int *const __dlerror_lockptr;
++extern hidden volatile int *const __gettext_lockptr;
++extern hidden volatile int *const __locale_lockptr;
++extern hidden volatile int *const __random_lockptr;
++extern hidden volatile int *const __sem_open_lockptr;
++extern hidden volatile int *const __stdio_ofl_lockptr;
++extern hidden volatile int *const __syslog_lockptr;
++extern hidden volatile int *const __timezone_lockptr;
++
++extern hidden volatile int *const __bump_lockptr;
++
++extern hidden volatile int *const __vmlock_lockptr;
++
++hidden void __malloc_atfork(int);
++hidden void __ldso_atfork(int);
+diff -ruN a/src/ldso/dlerror.c b/src/ldso/dlerror.c
+--- a/src/ldso/dlerror.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/ldso/dlerror.c	2022-07-04 03:54:03.838720649 -0600
+@@ -4,6 +4,7 @@
+ #include "pthread_impl.h"
+ #include "dynlink.h"
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ char *dlerror()
+ {
+@@ -19,6 +20,7 @@
+ 
+ static volatile int freebuf_queue_lock[1];
+ static void **freebuf_queue;
++volatile int *const __dlerror_lockptr = freebuf_queue_lock;
+ 
+ void __dl_thread_cleanup(void)
+ {
+diff -ruN a/src/locale/dcngettext.c b/src/locale/dcngettext.c
+--- a/src/locale/dcngettext.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/locale/dcngettext.c	2022-07-04 03:55:06.441903485 -0600
+@@ -10,6 +10,7 @@
+ #include "atomic.h"
+ #include "pleval.h"
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ struct binding {
+ 	struct binding *next;
+@@ -34,9 +35,11 @@
+ 	return 0;
+ }
+ 
++static volatile int lock[1];
++volatile int *const __gettext_lockptr = lock;
++
+ char *bindtextdomain(const char *domainname, const char *dirname)
+ {
+-	static volatile int lock[1];
+ 	struct binding *p, *q;
+ 
+ 	if (!domainname) return 0;
+diff -ruN a/src/locale/locale_map.c b/src/locale/locale_map.c
+--- a/src/locale/locale_map.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/locale/locale_map.c	2022-07-04 03:56:00.688904291 -0600
+@@ -4,6 +4,7 @@
+ #include "locale_impl.h"
+ #include "libc.h"
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ const char *__lctrans_impl(const char *msg, const struct __locale_map *lm)
+ {
+@@ -21,9 +22,11 @@
+ 	"LC_MESSAGES",
+ };
+ 
++static volatile int lock[1];
++volatile int *const __locale_lockptr = lock;
++
+ const struct __locale_map *__get_locale(int cat, const char *val)
+ {
+-	static volatile int lock[1];
+ 	static void *volatile loc_head;
+ 	const struct __locale_map *p;
+ 	struct __locale_map *new = 0;
+diff -ruN a/src/malloc/lite_malloc.c b/src/malloc/lite_malloc.c
+--- a/src/malloc/lite_malloc.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/malloc/lite_malloc.c	2022-07-04 03:44:54.995267064 -0600
+@@ -7,10 +7,12 @@
+ 
+ #define ALIGN 16
+ 
++static volatile int lock[1];
++volatile int *const __bump_lockptr = lock;
++
+ static void *__simple_malloc(size_t n)
+ {
+ 	static char *cur, *end;
+-	static volatile int lock[1];
+ 	size_t align=1, pad;
+ 	void *p;
+ 
+diff -ruN a/src/malloc/malloc.c b/src/malloc/malloc.c
+--- a/src/malloc/malloc.c	2022-07-04 04:00:34.790456619 -0600
++++ b/src/malloc/malloc.c	2022-07-04 03:44:54.995267064 -0600
+@@ -549,3 +549,21 @@
+ 	c->csize = n->psize = C_INUSE | (end-start);
+ 	__bin_chunk(c);
+ }
++
++void __malloc_atfork(int who)
++{
++	if (who<0) {
++		lock(mal.free_lock);
++		for (int i=0; i<64; i++)
++			lock(mal.bins[i].lock);
++	} else if (!who) {
++		for (int i=0; i<64; i++)
++			unlock(mal.bins[i].lock);
++		unlock(mal.free_lock);
++	} else {
++		for (int i=0; i<64; i++)
++			mal.bins[i].lock[0] = mal.bins[i].lock[1] = 0;
++		mal.free_lock[1] = 0;
++		mal.free_lock[0] = 0;
++	}
++}
+diff -ruN a/src/misc/syslog.c b/src/misc/syslog.c
+--- a/src/misc/syslog.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/misc/syslog.c	2022-07-04 03:56:54.985905097 -0600
+@@ -10,6 +10,7 @@
+ #include <errno.h>
+ #include <fcntl.h>
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ static volatile int lock[1];
+ static char log_ident[32];
+@@ -17,6 +18,7 @@
+ static int log_facility = LOG_USER;
+ static int log_mask = 0xff;
+ static int log_fd = -1;
++volatile int *const __syslog_lockptr = lock;
+ 
+ int setlogmask(int maskpri)
+ {
+diff -ruN a/src/prng/random.c b/src/prng/random.c
+--- a/src/prng/random.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/prng/random.c	2022-07-04 04:01:20.971457305 -0600
+@@ -1,6 +1,7 @@
+ #include <stdlib.h>
+ #include <stdint.h>
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ /*
+ this code uses the same lagged fibonacci generator as the
+@@ -23,6 +24,7 @@
+ static int j = 0;
+ static uint32_t *x = init+1;
+ static volatile int lock[1];
++volatile int *const __random_lockptr = lock;
+ 
+ static uint32_t lcg31(uint32_t x) {
+ 	return (1103515245*x + 12345) & 0x7fffffff;
+diff -ruN a/src/process/fork.c b/src/process/fork.c
+--- a/src/process/fork.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/process/fork.c	2022-07-04 04:11:00.700964128 -0600
+@@ -3,13 +3,48 @@
+ #include <signal.h>
+ #include "syscall.h"
+ #include "libc.h"
++#include "lock.h"
+ #include "pthread_impl.h"
++#include "fork_impl.h"
+ 
+-static void dummy(int x)
+-{
+-}
++static volatile int *const dummy_lockptr = 0;
++
++weak_alias(dummy_lockptr, __at_quick_exit_lockptr);
++weak_alias(dummy_lockptr, __atexit_lockptr);
++weak_alias(dummy_lockptr, __dlerror_lockptr);
++weak_alias(dummy_lockptr, __gettext_lockptr);
++weak_alias(dummy_lockptr, __locale_lockptr);
++weak_alias(dummy_lockptr, __random_lockptr);
++weak_alias(dummy_lockptr, __sem_open_lockptr);
++weak_alias(dummy_lockptr, __stdio_ofl_lockptr);
++weak_alias(dummy_lockptr, __syslog_lockptr);
++weak_alias(dummy_lockptr, __timezone_lockptr);
++weak_alias(dummy_lockptr, __bump_lockptr);
++
++weak_alias(dummy_lockptr, __vmlock_lockptr);
++
++static volatile int *const *const atfork_locks[] = {
++	&__at_quick_exit_lockptr,
++	&__atexit_lockptr,
++	&__dlerror_lockptr,
++	&__gettext_lockptr,
++	&__locale_lockptr,
++	&__random_lockptr,
++	&__sem_open_lockptr,
++	&__stdio_ofl_lockptr,
++	&__syslog_lockptr,
++	&__timezone_lockptr,
++	&__bump_lockptr,
++};
+ 
++static void dummy(int x) { }
+ weak_alias(dummy, __fork_handler);
++weak_alias(dummy, __malloc_atfork);
++weak_alias(dummy, __ldso_atfork);
++
++static void dummy_0(void) { }
++weak_alias(dummy_0, __tl_lock);
++weak_alias(dummy_0, __tl_unlock);
+ 
+ pid_t fork(void)
+ {
+@@ -17,6 +52,16 @@
+ 	sigset_t set;
+ 	__fork_handler(-1);
+ 	__block_all_sigs(&set);
++        int need_locks = libc.need_locks > 0;
++	if (need_locks) {
++		__ldso_atfork(-1);
++		__inhibit_ptc();
++		for (int i=0; i<sizeof atfork_locks/sizeof *atfork_locks; i++)
++			if (*atfork_locks[i]) LOCK(*atfork_locks[i]);
++		__malloc_atfork(-1);
++		__tl_lock();
++	}
++	pthread_t self=__pthread_self(), next=self->next;
+ #ifdef SYS_fork
+ 	ret = __syscall(SYS_fork);
+ #else
+@@ -31,6 +76,24 @@
+ 		__thread_list_lock = 0;
+ 		libc.threads_minus_1 = 0;
+ 	}
++	if (need_locks) {
++		if (!ret) {
++			for (pthread_t td=next; td!=self; td=td->next)
++				td->tid = -1;
++			if (__vmlock_lockptr) {
++				__vmlock_lockptr[0] = 0;
++				__vmlock_lockptr[1] = 0;
++			}
++		}
++		__tl_unlock();
++		__malloc_atfork(!ret);
++		for (int i=0; i<sizeof atfork_locks/sizeof *atfork_locks; i++)
++			if (*atfork_locks[i])
++				if (ret) UNLOCK(*atfork_locks[i]);
++				else **atfork_locks[i] = 0;
++		__release_ptc();
++		__ldso_atfork(!ret);
++	}
+ 	__restore_sigs(&set);
+ 	__fork_handler(!ret);
+ 	return __syscall_ret(ret);
+diff -ruN a/src/stdio/ofl.c b/src/stdio/ofl.c
+--- a/src/stdio/ofl.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/stdio/ofl.c	2022-07-04 04:01:47.286457695 -0600
+@@ -1,8 +1,10 @@
+ #include "stdio_impl.h"
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ static FILE *ofl_head;
+ static volatile int ofl_lock[1];
++volatile int *const __stdio_ofl_lockptr = ofl_lock;
+ 
+ FILE **__ofl_lock()
+ {
+diff -ruN a/src/thread/sem_open.c b/src/thread/sem_open.c
+--- a/src/thread/sem_open.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/thread/sem_open.c	2022-07-04 04:02:09.618458027 -0600
+@@ -12,6 +12,7 @@
+ #include <stdlib.h>
+ #include <pthread.h>
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ static struct {
+ 	ino_t ino;
+@@ -19,6 +20,7 @@
+ 	int refcnt;
+ } *semtab;
+ static volatile int lock[1];
++volatile int *const __sem_open_lockptr = lock;
+ 
+ #define FLAGS (O_RDWR|O_NOFOLLOW|O_CLOEXEC|O_NONBLOCK)
+ 
+diff -ruN a/src/thread/vmlock.c b/src/thread/vmlock.c
+--- a/src/thread/vmlock.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/thread/vmlock.c	2022-07-04 03:44:55.233267067 -0600
+@@ -1,6 +1,7 @@
+ #include "pthread_impl.h"
+ 
+ static volatile int vmlock[2];
++volatile int *const __vmlock_lockptr = vmlock;
+ 
+ void __vm_wait()
+ {
+diff -ruN a/src/time/__tz.c b/src/time/__tz.c
+--- a/src/time/__tz.c	2019-10-13 15:58:27.000000000 -0600
++++ b/src/time/__tz.c	2022-07-04 04:03:10.097331296 -0600
+@@ -6,6 +6,7 @@
+ #include <sys/mman.h>
+ #include "libc.h"
+ #include "lock.h"
++#include "fork_impl.h"
+ 
+ long  __timezone = 0;
+ int   __daylight = 0;
+@@ -30,6 +31,7 @@
+ static size_t old_tz_size = sizeof old_tz_buf;
+ 
+ static volatile int lock[1];
++volatile int *const __timezone_lockptr = lock;
+ 
+ static int getint(const char **p)
+ {
diff --git a/srcpkgs/musl/template b/srcpkgs/musl/template
index d30c99a563a0..5bdc5c48e623 100644
--- a/srcpkgs/musl/template
+++ b/srcpkgs/musl/template
@@ -2,7 +2,7 @@
 pkgname=musl
 reverts="1.2.0_1"
 version=1.1.24
-revision=9
+revision=10
 archs="*-musl"
 bootstrap=yes
 build_style=gnu-configure

Re: musl: backport changes for forking from multithreaded processes

[Gorggg]

[-- Attachment #1: Type: text/plain, Size: 382 bytes --]

New comment by Gorggg on void-packages repository

https://github.com/void-linux/void-packages/pull/37833#issuecomment-1173735728

Comment:
The x86_64-musl build seems to have exceeded its size limits within the "Verify repository state" task. I apologize for not forseeing this and setting [ci skip]. However, this is the platform I use, and I can directly verify its functioning.

Re: musl: backport changes for forking from multithreaded processes

[github-actions]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]

New comment by github-actions[bot] on void-packages repository

https://github.com/void-linux/void-packages/pull/37833#issuecomment-1264840723

Comment:
Pull Requests become stale 90 days after last activity and are closed 14 days after that.  If this pull request is still relevant bump it or assign it.

Re: musl: backport changes for forking from multithreaded processes

[Gorggg]

[-- Attachment #1: Type: text/plain, Size: 147 bytes --]

New comment by Gorggg on void-packages repository

https://github.com/void-linux/void-packages/pull/37833#issuecomment-1264841894

Comment:
<bump>

Re: musl: backport changes for forking from multithreaded processes

[github-actions]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]

New comment by github-actions[bot] on void-packages repository

https://github.com/void-linux/void-packages/pull/37833#issuecomment-1369319585

Comment:
Pull Requests become stale 90 days after last activity and are closed 14 days after that.  If this pull request is still relevant bump it or assign it.

Re: [PR PATCH] [Closed]: musl: backport changes for forking from multithreaded processes

[github-actions]

[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]

There's a closed pull request on the void-packages repository

musl: backport changes for forking from multithreaded processes
https://github.com/void-linux/void-packages/pull/37833

Description:
https://git.musl-libc.org/cgit/musl/commit/?id=167390f05564e0a4d3fcb4329377fd7743267560

This change allows child processes that have been forked from multithreaded parents to use non-AS-safe functions, as described in the original commit. Otherwise, there is a risk of deadlocks and/or state corruption. This makes fork() itself non-AS-safe, also as described. Minimal modifications were made to the original changes in order to accommodate different types and locations of files at different points in musl's history.

I encountered such a deadlock in a child process in the SANE pixma driver, which creates a reader process for communicating the device, which itself allocates memory using malloc().

These changes introduced no new test failures in [libc-test](https://wiki.musl-libc.org/libc-test.html).

Thank you for looking at this. If there is anything more I could do to test these changes, please let me know.

<!-- Uncomment relevant sections and delete options which are not applicable -->

#### Testing the changes
- I tested the changes in this PR: **YES**

#### Local build testing
- I built this PR locally for my native architecture, (x86_64-musl)

Re: musl: backport changes for forking from multithreaded processes

[JamiKettunen]

[-- Attachment #1: Type: text/plain, Size: 321 bytes --]

New comment by JamiKettunen on void-packages repository

https://github.com/void-linux/void-packages/pull/37833#issuecomment-1384757224

Comment:
@Gorggg Can you still confirm deadlocks with `musl-1.1.24_12` from repo (i.e. without this patch)? Otherwise would you mind rebasing this to fix the merge conflict, thanks :)