From 73d9e52d2fe81f947749cebbe204af219b4910e5 Mon Sep 17 00:00:00 2001 From: Jamie Gritton Date: Wed, 27 Apr 2016 02:25:21 +0000 Subject: [PATCH] Delay revmoing the last jail reference in prison_proc_free, and instead put it off into the pr_task. This is similar to prison_free, and in fact uses the same task even though they do something slightly different. This resolves a LOR between the process lock and allprison_lock, which came about in r298565. PR: 48471 --- sys/kern/kern_jail.c | 44 ++++++++++++++++++++++++++++++++++---------- sys/sys/jail.h | 3 +-- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c index 04837f1bc60a..56944ac3add7 100644 --- a/sys/kern/kern_jail.c +++ b/sys/kern/kern_jail.c @@ -1328,6 +1328,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) LIST_INIT(&pr->pr_children); mtx_init(&pr->pr_mtx, "jail mutex", NULL, MTX_DEF | MTX_DUPOK); + TASK_INIT(&pr->pr_task, 0, prison_complete, pr); #ifdef VIMAGE /* Allocate a new vnet if specified. */ @@ -2575,16 +2576,13 @@ prison_allow(struct ucred *cred, unsigned flag) void prison_free_locked(struct prison *pr) { + int ref; mtx_assert(&pr->pr_mtx, MA_OWNED); - pr->pr_ref--; - if (pr->pr_ref == 0) { - mtx_unlock(&pr->pr_mtx); - TASK_INIT(&pr->pr_task, 0, prison_complete, pr); - taskqueue_enqueue(taskqueue_thread, &pr->pr_task); - return; - } + ref = --pr->pr_ref; mtx_unlock(&pr->pr_mtx); + if (ref == 0) + taskqueue_enqueue(taskqueue_thread, &pr->pr_task); } void @@ -2595,11 +2593,17 @@ prison_free(struct prison *pr) prison_free_locked(pr); } +/* + * Complete a call to either prison_free or prison_proc_free. + */ static void prison_complete(void *context, int pending) { + struct prison *pr = context; - prison_deref((struct prison *)context, 0); + mtx_lock(&pr->pr_mtx); + prison_deref(pr, pr->pr_uref + ? PD_DEREF | PD_DEUREF | PD_LOCKED : PD_LOCKED); } /* @@ -2618,6 +2622,9 @@ prison_deref(struct prison *pr, int flags) mtx_lock(&pr->pr_mtx); for (;;) { if (flags & PD_DEUREF) { + KASSERT(pr->pr_uref > 0, + ("prison_deref PD_DEUREF on a dead prison (jid=%d)", + pr->pr_id)); pr->pr_uref--; lasturef = pr->pr_uref == 0; if (lasturef) @@ -2625,8 +2632,12 @@ prison_deref(struct prison *pr, int flags) KASSERT(prison0.pr_uref != 0, ("prison0 pr_uref=0")); } else lasturef = 0; - if (flags & PD_DEREF) + if (flags & PD_DEREF) { + KASSERT(pr->pr_ref > 0, + ("prison_deref PD_DEREF on a dead prison (jid=%d)", + pr->pr_id)); pr->pr_ref--; + } ref = pr->pr_ref; mtx_unlock(&pr->pr_mtx); @@ -2740,7 +2751,20 @@ prison_proc_free(struct prison *pr) mtx_lock(&pr->pr_mtx); KASSERT(pr->pr_uref > 0, ("Trying to kill a process in a dead prison (jid=%d)", pr->pr_id)); - prison_deref(pr, PD_DEUREF | PD_LOCKED); + if (pr->pr_uref > 1) + pr->pr_uref--; + else { + /* + * Don't remove the last user reference in this context, which + * is expected to be a process that is not only locked, but + * also half dead. + */ + pr->pr_ref++; + mtx_unlock(&pr->pr_mtx); + taskqueue_enqueue(taskqueue_thread, &pr->pr_task); + return; + } + mtx_unlock(&pr->pr_mtx); } diff --git a/sys/sys/jail.h b/sys/sys/jail.h index 8069dda3b4fc..e824054ec0f0 100644 --- a/sys/sys/jail.h +++ b/sys/sys/jail.h @@ -149,7 +149,6 @@ struct prison_racct; * (p) locked by pr_mtx * (c) set only during creation before the structure is shared, no mutex * required to read - * (d) set only during destruction of jail, no mutex needed */ struct prison { TAILQ_ENTRY(prison) pr_list; /* (a) all prisons */ @@ -161,7 +160,7 @@ struct prison { LIST_ENTRY(prison) pr_sibling; /* (a) next in parent's list */ struct prison *pr_parent; /* (c) containing jail */ struct mtx pr_mtx; - struct task pr_task; /* (d) destroy task */ + struct task pr_task; /* (c) destroy task */ struct osd pr_osd; /* (p) additional data */ struct cpuset *pr_cpuset; /* (p) cpuset */ struct vnet *pr_vnet; /* (c) network stack */