git: effad35ed1e5 - main - jail: Clean up some function placement and improve comments.
Jamie Gritton
jamie at FreeBSD.org
Tue Jan 19 01:27:44 UTC 2021
The branch main has been updated by jamie:
URL: https://cgit.FreeBSD.org/src/commit/?id=effad35ed1e52196469f8f2709b0f1f74cd2ce8c
commit effad35ed1e52196469f8f2709b0f1f74cd2ce8c
Author: Jamie Gritton <jamie at FreeBSD.org>
AuthorDate: 2021-01-19 01:23:51 +0000
Commit: Jamie Gritton <jamie at FreeBSD.org>
CommitDate: 2021-01-19 01:23:51 +0000
jail: Clean up some function placement and improve comments.
Move prison_hold, prison_hold_locked ,prison_proc_hold, and
prison_proc_free to a more intuitive part of the file (together with
with prison_free and prison_free_locked), and add or improve comments
to these and others, to better describe what's going in the prison
reference cycle.
No functional changes.
---
sys/kern/kern_jail.c | 157 +++++++++++++++++++++++++++++++--------------------
1 file changed, 96 insertions(+), 61 deletions(-)
diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index 4893e4df2781..757b8bd06b89 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -2604,8 +2604,35 @@ prison_allow(struct ucred *cred, unsigned flag)
}
/*
- * Remove a prison reference. If that was the last reference, remove the
- * prison itself - but not in this context in case there are locks held.
+ * Hold a prison reference, by incrementing pr_ref. It is generally
+ * an error to hold a prison that does not already have a reference.
+ * A prison record will remain valid as long as it has at least one
+ * reference, and will not be removed as long as either the prison
+ * mutex or the allprison lock is held (allprison_lock may be shared).
+ */
+void
+prison_hold_locked(struct prison *pr)
+{
+
+ mtx_assert(&pr->pr_mtx, MA_OWNED);
+ KASSERT(pr->pr_ref > 0,
+ ("Trying to hold dead prison %p (jid=%d).", pr, pr->pr_id));
+ pr->pr_ref++;
+}
+
+void
+prison_hold(struct prison *pr)
+{
+
+ mtx_lock(&pr->pr_mtx);
+ prison_hold_locked(pr);
+ mtx_unlock(&pr->pr_mtx);
+}
+
+/*
+ * Remove a prison reference. If that was the last reference, the
+ * prison will be removed (at a later time). Return with the prison
+ * unlocked.
*/
void
prison_free_locked(struct prison *pr)
@@ -2615,8 +2642,13 @@ prison_free_locked(struct prison *pr)
mtx_assert(&pr->pr_mtx, MA_OWNED);
ref = --pr->pr_ref;
mtx_unlock(&pr->pr_mtx);
- if (ref == 0)
+ if (ref == 0) {
+ /*
+ * Don't remove the last reference in this context,
+ * in case there are locks held.
+ */
taskqueue_enqueue(taskqueue_thread, &pr->pr_task);
+ }
}
void
@@ -2627,6 +2659,54 @@ prison_free(struct prison *pr)
prison_free_locked(pr);
}
+/*
+ * Hold a a prison for user visibility, by incrementing pr_uref.
+ * It is generally an error to hold a prison that isn't already
+ * user-visible, except through the the jail system calls. It is also
+ * an error to hold an invalid prison. A prison record will remain
+ * alive as long as it has at least one user reference, and will not
+ * be set to the dying state was long as the prison mutex is held.
+ */
+void
+prison_proc_hold(struct prison *pr)
+{
+
+ mtx_lock(&pr->pr_mtx);
+ KASSERT(pr->pr_uref > 0,
+ ("Cannot add a process to a non-alive prison (jid=%d)", pr->pr_id));
+ pr->pr_uref++;
+ mtx_unlock(&pr->pr_mtx);
+}
+
+/*
+ * Remove a prison user reference. If it was the last reference, the
+ * prison will be considered "dying", and may be removed once all of
+ * its references are dropped.
+ */
+void
+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));
+ 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. Add a reference so any calls to
+ * prison_free() won't re-submit the task.
+ */
+ pr->pr_ref++;
+ mtx_unlock(&pr->pr_mtx);
+ taskqueue_enqueue(taskqueue_thread, &pr->pr_task);
+ return;
+ }
+ mtx_unlock(&pr->pr_mtx);
+}
+
/*
* Complete a call to either prison_free or prison_proc_free.
*/
@@ -2637,16 +2717,24 @@ prison_complete(void *context, int pending)
sx_xlock(&allprison_lock);
mtx_lock(&pr->pr_mtx);
- prison_deref(pr, pr->pr_uref
+ /*
+ * If this is completing a call to prison_proc_free, there will still
+ * be a user reference held; clear that as well as the reference that
+ * was added. No references are expected if this is completing a call
+ * to prison_free, but prison_deref is still called for the cleanup.
+ */
+ prison_deref(pr, pr->pr_uref > 0
? PD_DEREF | PD_DEUREF | PD_LOCKED | PD_LIST_XLOCKED
: PD_LOCKED | PD_LIST_XLOCKED);
}
/*
- * Remove a prison reference (usually). This internal version assumes no
- * mutexes are held, except perhaps the prison itself. If there are no more
- * references, release and delist the prison. On completion, the prison lock
- * and the allprison lock are both unlocked.
+ * Remove a prison reference and/or user reference (usually).
+ * This assumes context that allows sleeping (for allprison_lock),
+ * with no non-sleeping locks held, except perhaps the prison itself.
+ * If there are no more references, release and delist the prison.
+ * On completion, the prison lock and the allprison lock are both
+ * unlocked.
*/
static void
prison_deref(struct prison *pr, int flags)
@@ -2750,59 +2838,6 @@ prison_deref(struct prison *pr, int flags)
}
}
-void
-prison_hold_locked(struct prison *pr)
-{
-
- mtx_assert(&pr->pr_mtx, MA_OWNED);
- KASSERT(pr->pr_ref > 0,
- ("Trying to hold dead prison %p (jid=%d).", pr, pr->pr_id));
- pr->pr_ref++;
-}
-
-void
-prison_hold(struct prison *pr)
-{
-
- mtx_lock(&pr->pr_mtx);
- prison_hold_locked(pr);
- mtx_unlock(&pr->pr_mtx);
-}
-
-void
-prison_proc_hold(struct prison *pr)
-{
-
- mtx_lock(&pr->pr_mtx);
- KASSERT(pr->pr_uref > 0,
- ("Cannot add a process to a non-alive prison (jid=%d)", pr->pr_id));
- pr->pr_uref++;
- mtx_unlock(&pr->pr_mtx);
-}
-
-void
-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));
- 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);
-}
-
/*
* Set or clear a permission bit in the pr_allow field, passing restrictions
* (cleared permission) down to child jails.
More information about the dev-commits-src-all
mailing list