Date: Mon, 7 Aug 2023 00:59:52 GMT From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 910aa553a9ba - stable/13 - killpg(): close a race with fork(), part 2 Message-ID: <202308070059.3770xqim077225@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=910aa553a9baaa66030b40dc56c254bcc74a9286 commit 910aa553a9baaa66030b40dc56c254bcc74a9286 Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2023-07-20 21:19:03 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2023-08-07 00:48:45 +0000 killpg(): close a race with fork(), part 2 (cherry picked from commit 232b922cb363e01ac0dd2a277d93cf74d8485e79) --- sys/kern/kern_fork.c | 47 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 6fb9d2a83c8f..3b612e7b8990 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -857,7 +857,7 @@ fork1(struct thread *td, struct fork_req *fr) static int curfail; static struct timeval lastfail; int flags, pages; - bool killsx_locked; + bool killsx_locked, singlethreaded; flags = fr->fr_flags; pages = fr->fr_pages; @@ -915,6 +915,7 @@ fork1(struct thread *td, struct fork_req *fr) newproc = NULL; vm2 = NULL; killsx_locked = false; + singlethreaded = false; /* * Increment the nprocs resource before allocations occur. @@ -945,14 +946,37 @@ fork1(struct thread *td, struct fork_req *fr) } /* - * Atomically check for signals and block threads from sending - * a signal to our process group until the child is visible. + * If we are possibly multi-threaded, and there is a process + * sending a signal to our group right now, ensure that our + * other threads cannot be chosen for the signal queueing. + * Otherwise, this might delay signal action, and make the new + * child escape the signaling. */ pg = p1->p_pgrp; - if (sx_slock_sig(&pg->pg_killsx) != 0) { + if (p1->p_numthreads > 1) { + if (sx_try_slock(&pg->pg_killsx) != 0) { + killsx_locked = true; + } else { + PROC_LOCK(p1); + if (thread_single(p1, SINGLE_BOUNDARY)) { + PROC_UNLOCK(p1); + error = ERESTART; + goto fail2; + } + PROC_UNLOCK(p1); + singlethreaded = true; + } + } + + /* + * Atomically check for signals and block processes from sending + * a signal to our process group until the child is visible. + */ + if (!killsx_locked && sx_slock_sig(&pg->pg_killsx) != 0) { error = ERESTART; goto fail2; - } else if (__predict_false(p1->p_pgrp != pg || sig_intr() != 0)) { + } + if (__predict_false(p1->p_pgrp != pg || sig_intr() != 0)) { /* * Either the process was moved to other process * group, or there is pending signal. sx_slock_sig() @@ -1056,8 +1080,8 @@ fork1(struct thread *td, struct fork_req *fr) } do_fork(td, fr, newproc, td2, vm2, fp_procdesc); - sx_sunlock(&pg->pg_killsx); - return (0); + error = 0; + goto cleanup; fail0: error = EAGAIN; #ifdef MAC @@ -1075,9 +1099,16 @@ fail2: fdrop(fp_procdesc, td); } atomic_add_int(&nprocs, -1); +cleanup: if (killsx_locked) sx_sunlock(&pg->pg_killsx); - pause("fork", hz / 2); + if (singlethreaded) { + PROC_LOCK(p1); + thread_single_end(p1, SINGLE_BOUNDARY); + PROC_UNLOCK(p1); + } + if (error != 0) + pause("fork", hz / 2); return (error); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202308070059.3770xqim077225>