mirror/freebsd
1
0
Fork 0
mirror of https://git.FreeBSD.org/src.git synced 2024-12-11 09:50:12 +00:00
Code Issues Releases Activity

ktrace: Fix a race with fork()

Browse Source 
ktrace(2) may toggle trace points in any of
1. a single process
2. all members of a process group
3. all descendents of the processes in 1 or 2

In the first two cases, we do not permit the operation if the process is
being forked or not visible. However, in case 3 we did not enforce this
restriction for descendents. As a result, the assertions about the child
in ktrprocfork() may be violated.

Move these checks into ktrops() so that they are applied consistently.

Allow KTROP_CLEAR for nascent processes. Otherwise, there is a window
where we cannot clear trace points for a nascent child if they are
inherited from the parent.

Reported by:	syzbot+d96676592978f137e05c@syzkaller.appspotmail.com
Reported by:	syzbot+7c98fcf84a4439f2817f@syzkaller.appspotmail.com
Reviewed by:	kib
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D30481
This commit is contained in:
Mark Johnston 2021-05-27 15:49:59 -04:00
parent e00bae5c18
commit f3851b235b
1 changed files with 22 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
sys/kern/kern_ktrace.c
View File

@ -1006,7 +1006,7 @@ sys_ktrace(struct thread *td, struct ktrace_args *uap)
int facs = uap->facs & ~KTRFAC_ROOT;
int ops = KTROP(uap->ops);
int descend = uap->ops & KTRFLAG_DESCEND;
int nfound, ret = 0;
int ret = 0;
int flags, error = 0;
struct nameidata nd;
struct ktr_io_params *kiop, *old_kiop;
@ -1080,42 +1080,31 @@ sys_ktrace(struct thread *td, struct ktrace_args *uap)
error = ESRCH;
goto done;
}
/*
* ktrops() may call vrele(). Lock pg_members
* by the proctree_lock rather than pg_mtx.
*/
PGRP_UNLOCK(pg);
nfound = 0;
if (LIST_EMPTY(&pg->pg_members)) {
sx_sunlock(&proctree_lock);
error = ESRCH;
goto done;
}
LIST_FOREACH(p, &pg->pg_members, p_pglist) {
PROC_LOCK(p);
if (p->p_state == PRS_NEW ||
p_cansee(td, p) != 0) {
PROC_UNLOCK(p);
continue;
}
nfound++;
if (descend)
ret |= ktrsetchildren(td, p, ops, facs, kiop);
else
ret |= ktrops(td, p, ops, facs, kiop);
}
if (nfound == 0) {
sx_sunlock(&proctree_lock);
error = ESRCH;
goto done;
}
} else {
/*
* by pid
*/
p = pfind(uap->pid);
if (p == NULL)
if (p == NULL) {
error = ESRCH;
else
error = p_cansee(td, p);
if (error) {
if (p != NULL)
PROC_UNLOCK(p);
sx_sunlock(&proctree_lock);
goto done;
}
@ -1187,8 +1176,20 @@ ktrops(struct thread *td, struct proc *p, int ops, int facs,
PROC_UNLOCK(p);
return (0);
}
if (p->p_flag & P_WEXIT) {
/* If the process is exiting, just ignore it. */
if ((ops == KTROP_SET && p->p_state == PRS_NEW) || !p_cansee(td, p)) {
/*
* Disallow setting trace points if the process is being born.
* This avoids races with trace point inheritance in
* ktrprocfork().
*/
PROC_UNLOCK(p);
return (0);
}
if ((p->p_flag & P_WEXIT) != 0) {
/*
* There's nothing to do if the process is exiting, but avoid
* signaling an error.
*/
PROC_UNLOCK(p);
return (1);
}