Changeset View
Standalone View
sys/kern/kern_procctl.c
Show First 20 Lines • Show All 248 Lines • ▼ Show 20 Lines | reap_kill_proc_relock(struct proc *p, int xlocked) | ||||||||
PROC_UNLOCK(p); | PROC_UNLOCK(p); | ||||||||
if (xlocked) | if (xlocked) | ||||||||
sx_xlock(&proctree_lock); | sx_xlock(&proctree_lock); | ||||||||
else | else | ||||||||
sx_slock(&proctree_lock); | sx_slock(&proctree_lock); | ||||||||
PROC_LOCK(p); | PROC_LOCK(p); | ||||||||
} | } | ||||||||
static bool | static void | ||||||||
reap_kill_proc_locked(struct thread *td, struct proc *p2, | reap_kill_proc_locked(struct thread *td, struct proc *p2, | ||||||||
ksiginfo_t *ksi, struct procctl_reaper_kill *rk, int *error) | ksiginfo_t *ksi, struct procctl_reaper_kill *rk, int *error) | ||||||||
{ | { | ||||||||
int error1, r, xlocked; | int error1, r, xlocked; | ||||||||
bool need_stop; | bool need_stop; | ||||||||
PROC_LOCK_ASSERT(p2, MA_OWNED); | PROC_LOCK_ASSERT(p2, MA_OWNED); | ||||||||
PROC_ASSERT_HELD(p2); | PROC_ASSERT_HELD(p2); | ||||||||
error1 = p_cansignal(td, p2, rk->rk_sig); | error1 = p_cansignal(td, p2, rk->rk_sig); | ||||||||
if (error1 != 0) { | if (error1 != 0) { | ||||||||
if (*error == ESRCH) { | if (*error == ESRCH) { | ||||||||
rk->rk_fpid = p2->p_pid; | rk->rk_fpid = p2->p_pid; | ||||||||
*error = error1; | *error = error1; | ||||||||
} | } | ||||||||
return (true); | return; | ||||||||
} | } | ||||||||
/* | /* | ||||||||
* The need_stop indicates if the target process needs to be | * The need_stop indicates if the target process needs to be | ||||||||
* suspended before being signalled. This is needed when we | * suspended before being signalled. This is needed when we | ||||||||
* guarantee that all processes in subtree are signalled, | * guarantee that all processes in subtree are signalled, | ||||||||
* avoiding the race with some process not yet fully linked | * avoiding the race with some process not yet fully linked | ||||||||
* into all structures during fork, ignored by iterator, and | * into all structures during fork, ignored by iterator, and | ||||||||
* then escaping signalling. | * then escaping signalling. | ||||||||
* | * | ||||||||
* If need_stop is true, then reap_kill_proc() returns true if | * If need_stop is true, then reap_kill_proc() returns true if | ||||||||
* the process was successfully stopped and signalled, and | * the process was successfully stopped and signalled, and | ||||||||
* false if stopping failed and the signal was not sent. | * false if stopping failed and the signal was not sent. | ||||||||
* | * | ||||||||
* The thread cannot usefully stop itself anyway, and if other | * The thread cannot usefully stop itself anyway, and if other | ||||||||
* thread of the current process forks while the current | * thread of the current process forks while the current | ||||||||
* thread signals the whole subtree, it is an application | * thread signals the whole subtree, it is an application | ||||||||
* race. | * race. | ||||||||
*/ | */ | ||||||||
need_stop = p2 != td->td_proc && | need_stop = p2 != td->td_proc && | ||||||||
(p2->p_flag & (P_KPROC | P_SYSTEM)) == 0 && | (td->td_proc->p_flag2 & P2_WEXIT) == 0 && | ||||||||
(p2->p_flag & (P_KPROC | P_SYSTEM | P_STOPPED)) == 0 && | |||||||||
markj: P_STOPPED contains P_STOPPED_SINGLE. So if the target proc is already stopped by, e.g. | |||||||||
kibAuthorUnsubmitted Done Inline ActionsI do not see how would we unsuspend the target by delivering a signal. In fact, if we do, this is bug, as you noted, unrelated to this fragment. Imagine that the signal was delivered while forking thread is already in kernel but before thread_single(). Signal must not reach userspace then. That said, the main point there is if P_STOPPED_SINGLE is set, we *do not* need to single-thread because the process lock is not dropped and process is in the right state. kib: I do not see how would we unsuspend the target by delivering a signal. In fact, if we do, this… | |||||||||
markjUnsubmitted Done Inline Actionstdsendsignal() may unsuspend a target thread. I think there is at least one bug there: suppose thread T is suspended in thread_single(SINGLE_ALLPROC) when trying to kill another process with REAP_KILL. Suppose a different thread sends SIGKILL to T->td_proc. Then, tdsendsignal() calls thread_unsuspend(T, T->td_proc). thread_unsuspend() incorrectly decrements T->td_proc->p_suspcount to -1. Later, when T->td_proc exits, it will wait forever in thread_single(SINGLE_EXIT) since T->td_proc->p_suspcount never reaches 1. markj: tdsendsignal() may unsuspend a target thread. I think there is at least one bug there: suppose… | |||||||||
kibAuthorUnsubmitted Done Inline ActionsI agree. And I think that thread_unsuspend_one() can be skipped for T then. kib: I agree. And I think that thread_unsuspend_one() can be skipped for T then. | |||||||||
(rk->rk_flags & REAPER_KILL_CHILDREN) == 0; | (rk->rk_flags & REAPER_KILL_CHILDREN) == 0; | ||||||||
if (need_stop) { | if (need_stop) { | ||||||||
if (P_SHOULDSTOP(p2) == P_STOPPED_SINGLE) | |||||||||
return (false); /* retry later */ | |||||||||
xlocked = sx_xlocked(&proctree_lock); | xlocked = sx_xlocked(&proctree_lock); | ||||||||
sx_unlock(&proctree_lock); | sx_unlock(&proctree_lock); | ||||||||
r = thread_single(p2, SINGLE_ALLPROC); | r = thread_single(p2, SINGLE_ALLPROC); | ||||||||
if (r != 0) { | |||||||||
reap_kill_proc_relock(p2, xlocked); | reap_kill_proc_relock(p2, xlocked); | ||||||||
return (false); | if (r != 0) | ||||||||
need_stop = false; | |||||||||
} | } | ||||||||
} | |||||||||
pksignal(p2, rk->rk_sig, ksi); | pksignal(p2, rk->rk_sig, ksi); | ||||||||
rk->rk_killed++; | rk->rk_killed++; | ||||||||
*error = error1; | *error = error1; | ||||||||
if (need_stop) { | if (need_stop) | ||||||||
reap_kill_proc_relock(p2, xlocked); | |||||||||
thread_single_end(p2, SINGLE_ALLPROC); | thread_single_end(p2, SINGLE_ALLPROC); | ||||||||
} | } | ||||||||
return (true); | |||||||||
} | |||||||||
static bool | static void | ||||||||
reap_kill_proc(struct thread *td, struct proc *p2, ksiginfo_t *ksi, | reap_kill_proc(struct thread *td, struct proc *p2, ksiginfo_t *ksi, | ||||||||
struct procctl_reaper_kill *rk, int *error) | struct procctl_reaper_kill *rk, int *error) | ||||||||
{ | { | ||||||||
bool res; | |||||||||
res = true; | |||||||||
PROC_LOCK(p2); | PROC_LOCK(p2); | ||||||||
if ((p2->p_flag & P_WEXIT) == 0) { | if ((p2->p_flag2 & P2_WEXIT) == 0) { | ||||||||
_PHOLD_LITE(p2); | _PHOLD_LITE(p2); | ||||||||
res = reap_kill_proc_locked(td, p2, ksi, rk, error); | reap_kill_proc_locked(td, p2, ksi, rk, error); | ||||||||
_PRELE(p2); | _PRELE(p2); | ||||||||
} | } | ||||||||
PROC_UNLOCK(p2); | PROC_UNLOCK(p2); | ||||||||
return (res); | |||||||||
} | } | ||||||||
struct reap_kill_tracker { | struct reap_kill_tracker { | ||||||||
struct proc *parent; | struct proc *parent; | ||||||||
TAILQ_ENTRY(reap_kill_tracker) link; | TAILQ_ENTRY(reap_kill_tracker) link; | ||||||||
}; | }; | ||||||||
TAILQ_HEAD(reap_kill_tracker_head, reap_kill_tracker); | TAILQ_HEAD(reap_kill_tracker_head, reap_kill_tracker); | ||||||||
static void | static void | ||||||||
reap_kill_sched(struct reap_kill_tracker_head *tracker, struct proc *p2) | reap_kill_sched(struct reap_kill_tracker_head *tracker, struct proc *p2) | ||||||||
{ | { | ||||||||
struct reap_kill_tracker *t; | struct reap_kill_tracker *t; | ||||||||
PROC_LOCK(p2); | |||||||||
if ((p2->p_flag2 & P2_WEXIT) != 0) { | |||||||||
PROC_UNLOCK(p2); | |||||||||
return; | |||||||||
} | |||||||||
_PHOLD_LITE(p2); | |||||||||
PROC_UNLOCK(p2); | |||||||||
t = malloc(sizeof(struct reap_kill_tracker), M_TEMP, M_WAITOK); | t = malloc(sizeof(struct reap_kill_tracker), M_TEMP, M_WAITOK); | ||||||||
t->parent = p2; | t->parent = p2; | ||||||||
TAILQ_INSERT_TAIL(tracker, t, link); | TAILQ_INSERT_TAIL(tracker, t, link); | ||||||||
} | } | ||||||||
static void | static void | ||||||||
reap_kill_sched_free(struct reap_kill_tracker *t) | |||||||||
{ | |||||||||
PRELE(t->parent); | |||||||||
free(t, M_TEMP); | |||||||||
} | |||||||||
static void | |||||||||
reap_kill_children(struct thread *td, struct proc *reaper, | reap_kill_children(struct thread *td, struct proc *reaper, | ||||||||
struct procctl_reaper_kill *rk, ksiginfo_t *ksi, int *error) | struct procctl_reaper_kill *rk, ksiginfo_t *ksi, int *error) | ||||||||
{ | { | ||||||||
struct proc *p2; | struct proc *p2; | ||||||||
LIST_FOREACH(p2, &reaper->p_children, p_sibling) { | LIST_FOREACH(p2, &reaper->p_children, p_sibling) { | ||||||||
(void)reap_kill_proc(td, p2, ksi, rk, error); | (void)reap_kill_proc(td, p2, ksi, rk, error); | ||||||||
/* | /* | ||||||||
Show All 12 Lines | reap_kill_subtree_once(struct thread *td, struct proc *p, struct proc *reaper, | ||||||||
struct reap_kill_tracker *t; | struct reap_kill_tracker *t; | ||||||||
struct proc *p2; | struct proc *p2; | ||||||||
bool res; | bool res; | ||||||||
res = false; | res = false; | ||||||||
TAILQ_INIT(&tracker); | TAILQ_INIT(&tracker); | ||||||||
reap_kill_sched(&tracker, reaper); | reap_kill_sched(&tracker, reaper); | ||||||||
while ((t = TAILQ_FIRST(&tracker)) != NULL) { | while ((t = TAILQ_FIRST(&tracker)) != NULL) { | ||||||||
MPASS((t->parent->p_treeflag & P_TREE_REAPER) != 0); | |||||||||
TAILQ_REMOVE(&tracker, t, link); | TAILQ_REMOVE(&tracker, t, link); | ||||||||
/* | |||||||||
* Since reap_kill_proc() drops proctree_lock sx, it | |||||||||
* is possible that the tracker reapper is no longer. | |||||||||
Done Inline Actions
markj: | |||||||||
* In this case the subtree is reparented to the new | |||||||||
* reaper, which should handle it. | |||||||||
*/ | |||||||||
if ((t->parent->p_treeflag & P_TREE_REAPER) == 0) { | |||||||||
reap_kill_sched_free(t); | |||||||||
res = true; | |||||||||
continue; | |||||||||
} | |||||||||
LIST_FOREACH(p2, &t->parent->p_reaplist, p_reapsibling) { | LIST_FOREACH(p2, &t->parent->p_reaplist, p_reapsibling) { | ||||||||
if (t->parent == reaper && | if (t->parent == reaper && | ||||||||
(rk->rk_flags & REAPER_KILL_SUBTREE) != 0 && | (rk->rk_flags & REAPER_KILL_SUBTREE) != 0 && | ||||||||
p2->p_reapsubtree != rk->rk_subtree) | p2->p_reapsubtree != rk->rk_subtree) | ||||||||
continue; | continue; | ||||||||
if ((p2->p_treeflag & P_TREE_REAPER) != 0) | if ((p2->p_treeflag & P_TREE_REAPER) != 0) | ||||||||
reap_kill_sched(&tracker, p2); | reap_kill_sched(&tracker, p2); | ||||||||
if (alloc_unr_specific(pids, p2->p_pid) != p2->p_pid) | if (alloc_unr_specific(pids, p2->p_pid) != p2->p_pid) | ||||||||
continue; | continue; | ||||||||
if (!reap_kill_proc(td, p2, ksi, rk, error)) | reap_kill_proc(td, p2, ksi, rk, error); | ||||||||
free_unr(pids, p2->p_pid); | |||||||||
res = true; | res = true; | ||||||||
} | } | ||||||||
free(t, M_TEMP); | reap_kill_sched_free(t); | ||||||||
} | } | ||||||||
return (res); | return (res); | ||||||||
} | } | ||||||||
static void | static void | ||||||||
reap_kill_subtree(struct thread *td, struct proc *p, struct proc *reaper, | reap_kill_subtree(struct thread *td, struct proc *p, struct proc *reaper, | ||||||||
struct procctl_reaper_kill *rk, ksiginfo_t *ksi, int *error) | struct procctl_reaper_kill *rk, ksiginfo_t *ksi, int *error) | ||||||||
{ | { | ||||||||
struct unrhdr pids; | struct unrhdr pids; | ||||||||
/* | /* | ||||||||
* pids records processes which were already signalled, to | * pids records processes which were already signalled, to | ||||||||
* avoid doubling signals to them if iteration needs to be | * avoid doubling signals to them if iteration needs to be | ||||||||
* repeated. | * repeated. | ||||||||
*/ | */ | ||||||||
init_unrhdr(&pids, 1, PID_MAX, UNR_NO_MTX); | init_unrhdr(&pids, 1, PID_MAX, UNR_NO_MTX); | ||||||||
PROC_LOCK(td->td_proc); | |||||||||
if ((td->td_proc->p_flag2 & P2_WEXIT) != 0) { | |||||||||
PROC_UNLOCK(td->td_proc); | |||||||||
goto out; | |||||||||
} | |||||||||
td->td_proc->p_singlethr++; | |||||||||
PROC_UNLOCK(td->td_proc); | |||||||||
while (reap_kill_subtree_once(td, p, reaper, rk, ksi, error, &pids)) | while (reap_kill_subtree_once(td, p, reaper, rk, ksi, error, &pids)) | ||||||||
; | ; | ||||||||
PROC_LOCK(td->td_proc); | |||||||||
td->td_proc->p_singlethr--; | |||||||||
if (td->td_proc->p_singlethr == 0) | |||||||||
wakeup(&p->p_singlethr); | |||||||||
PROC_UNLOCK(td->td_proc); | |||||||||
out: | |||||||||
clean_unrhdr(&pids); | clean_unrhdr(&pids); | ||||||||
clear_unrhdr(&pids); | clear_unrhdr(&pids); | ||||||||
} | } | ||||||||
static bool | static bool | ||||||||
reap_kill_sapblk(struct thread *td __unused, void *data) | reap_kill_sapblk(struct thread *td __unused, void *data) | ||||||||
{ | { | ||||||||
struct procctl_reaper_kill *rk; | struct procctl_reaper_kill *rk; | ||||||||
▲ Show 20 Lines • Show All 620 Lines • ▼ Show 20 Lines | kern_procctl(struct thread *td, idtype_t idtype, id_t id, int com, void *data) | ||||||||
MPASS(com > 0 && com < nitems(procctl_cmds_info)); | MPASS(com > 0 && com < nitems(procctl_cmds_info)); | ||||||||
cmd_info = &procctl_cmds_info[com]; | cmd_info = &procctl_cmds_info[com]; | ||||||||
if (idtype != P_PID && cmd_info->one_proc) | if (idtype != P_PID && cmd_info->one_proc) | ||||||||
return (EINVAL); | return (EINVAL); | ||||||||
sapblk = false; | sapblk = false; | ||||||||
if (cmd_info->sapblk != NULL) { | if (cmd_info->sapblk != NULL) { | ||||||||
sapblk = cmd_info->sapblk(td, data); | sapblk = cmd_info->sapblk(td, data); | ||||||||
if (sapblk) | if (sapblk && !stop_all_proc_block()) | ||||||||
stop_all_proc_block(); | return (ERESTART); | ||||||||
} | } | ||||||||
switch (cmd_info->lock_tree) { | switch (cmd_info->lock_tree) { | ||||||||
case PCTL_XLOCKED: | case PCTL_XLOCKED: | ||||||||
sx_xlock(&proctree_lock); | sx_xlock(&proctree_lock); | ||||||||
break; | break; | ||||||||
case PCTL_SLOCKED: | case PCTL_SLOCKED: | ||||||||
sx_slock(&proctree_lock); | sx_slock(&proctree_lock); | ||||||||
▲ Show 20 Lines • Show All 86 Lines • Show Last 20 Lines |
P_STOPPED contains P_STOPPED_SINGLE. So if the target proc is already stopped by, e.g., thread_single(SINGLE_BOUNDARY) in fork(), then here we will not stop the proc, and will deliver a signal. This may unsuspend the target and violate assumptions of the stopping thread.
Hmm, but this problem is not specific to reap_kill. Any kill(2) call could have the same effect...
At least, I suspect that need_stop should be true if P_STOPPED_SINGLE is set in the target.