Date: Fri, 22 Apr 2022 15:25:02 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 8394a99386fe - stable/13 - setitimer: Fix exit race Message-ID: <202204221525.23MFP23N081966@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=8394a99386fe5ff98b50fedde480ec35fb397717 commit 8394a99386fe5ff98b50fedde480ec35fb397717 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2022-03-23 16:36:12 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2022-04-22 14:35:42 +0000 setitimer: Fix exit race We use the p_itcallout callout, interlocked by the proc lock, to schedule timeouts for the setitimer(2) system call. When a process exits, the callout must be stopped before the process struct is recycled. Currently we attempt to stop the callout in exit1() with the call _callout_stop_safe(&p->p_itcallout, CS_EXECUTING). If this call returns 0, then we sleep in order to drain the callout. However, this happens only if the callout is not scheduled at all. If the callout thread is blocked on the proc lock, then exit1() will not block and the callout may execute after the process has fully exited, typically resulting in a panic. I cannot see a reason to use the CS_EXECUTING flag here. Instead, use the regular callout_stop()/callout_drain() dance to halt the callout. Reported by: ler Tested by: ler, pho Sponsored by: The FreeBSD Foundation (cherry picked from commit b319171861464f6c445905e7649cb43bf9bc78be) --- sys/kern/kern_exit.c | 11 +++++------ sys/kern/kern_time.c | 2 -- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index 226cd5991261..e89139edc825 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -369,15 +369,14 @@ exit1(struct thread *td, int rval, int signo) * executing, prevent it from rearming itself and let it finish. */ if (timevalisset(&p->p_realtimer.it_value) && - _callout_stop_safe(&p->p_itcallout, CS_EXECUTING, NULL) == 0) { + callout_stop(&p->p_itcallout) == 0) { timevalclear(&p->p_realtimer.it_interval); - msleep(&p->p_itcallout, &p->p_mtx, PWAIT, "ritwait", 0); - KASSERT(!timevalisset(&p->p_realtimer.it_value), - ("realtime timer is still armed")); + PROC_UNLOCK(p); + callout_drain(&p->p_itcallout); + } else { + PROC_UNLOCK(p); } - PROC_UNLOCK(p); - if (p->p_sysent->sv_onexit != NULL) p->p_sysent->sv_onexit(p); seltdfini(td); diff --git a/sys/kern/kern_time.c b/sys/kern/kern_time.c index 0bab05c65ffc..b02b6a2021e0 100644 --- a/sys/kern/kern_time.c +++ b/sys/kern/kern_time.c @@ -952,8 +952,6 @@ realitexpire(void *arg) kern_psignal(p, SIGALRM); if (!timevalisset(&p->p_realtimer.it_interval)) { timevalclear(&p->p_realtimer.it_value); - if (p->p_flag & P_WEXIT) - wakeup(&p->p_itcallout); return; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202204221525.23MFP23N081966>