fix deadlock in synccall after threaded fork

Submitted by Samuel Holland on July 1, 2019, 3:44 a.m.

Details

Message ID 20190701034428.62764-1-samuel@sholland.org
State New
Series "fix deadlock in synccall after threaded fork"
Headers show

Commit Message

Samuel Holland July 1, 2019, 3:44 a.m.
synccall may be called by AS-safe functions such as setuid/setgid after
fork. although fork() resets libc.threads_minus_one, causing synccall to
take the single-threaded path, synccall still takes the thread list
lock. This lock may be held by another thread if for example fork()
races with pthread_create(). After fork(), the value of the lock is
meaningless, so clear it.
---
 src/process/fork.c | 1 +
 1 file changed, 1 insertion(+)

Patch hide | download patch | download mbox

diff --git a/src/process/fork.c b/src/process/fork.c
index 11286ef4..fb42478a 100644
--- a/src/process/fork.c
+++ b/src/process/fork.c
@@ -28,6 +28,7 @@  pid_t fork(void)
 		self->robust_list.off = 0;
 		self->robust_list.pending = 0;
 		self->next = self->prev = self;
+		__thread_list_lock = 0;
 		libc.threads_minus_1 = 0;
 	}
 	__restore_sigs(&set);

Comments

Rich Felker July 1, 2019, 2:12 p.m.
On Sun, Jun 30, 2019 at 10:44:28PM -0500, Samuel Holland wrote:
> synccall may be called by AS-safe functions such as setuid/setgid after
> fork. although fork() resets libc.threads_minus_one, causing synccall to
> take the single-threaded path, synccall still takes the thread list
> lock. This lock may be held by another thread if for example fork()
> races with pthread_create(). After fork(), the value of the lock is
> meaningless, so clear it.
> ---
>  src/process/fork.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/process/fork.c b/src/process/fork.c
> index 11286ef4..fb42478a 100644
> --- a/src/process/fork.c
> +++ b/src/process/fork.c
> @@ -28,6 +28,7 @@ pid_t fork(void)
>  		self->robust_list.off = 0;
>  		self->robust_list.pending = 0;
>  		self->next = self->prev = self;
> +		__thread_list_lock = 0;
>  		libc.threads_minus_1 = 0;
>  	}
>  	__restore_sigs(&set);
> -- 
> 2.21.0

Thanks! Committing this with a maintainer's note referencing the
commits that caused the regression and a further remark on why it's
safe/correct to make this change.

Rich