Page MenuHomeFreeBSD

jail: Add pr_state to struct prison
ClosedPublic

Authored by jamie on Jan 1 2021, 1:09 AM.

Details

Summary

This is the first of two related changes, the second change making dying jails unresurrectable.

Currently, the state of a prison is deduced from its pr_ref and pr_uref counters, with three or four possible states:
pr_ref == 0: new (internal to kern_jail_set), or dead but not yet unlinked.
pr_ref > 0 && pr_uref > 0: alive, visible to kernel and user space
pr_ref > 0 && pr_uref == 0: dying, vidible to kernel but hidden from user space

The difference between the very beginning and end of a jail life cycle is subtle: both are linked into allprisons and pr_children, and are skipped when searching those lists. New prisons are referenced within kern_jail_set and given references to make them visible when the system call succeeds. Dead prisons are only linked into the lists when the last reference is dropped in a context that isn't known to support sleep locks.

This patch makes the jail state explicit, though it mostly follows the above model. The state may be one of the three already-defined constants in jail.h: PRISON_STATE_INVALID (new), PRISON_STATE_ALIVE, PRISON_STATE_DYING. The difference between ALIVE and DYING states is exactly as before: a prison is ALIVE when its pr_uref > 0. Fully dead jails are still deduced from pr_ref == 0, but new jails are not. With an explicit INVALID state, new jails can get a reference added at creation time, avoiding accidental removal coming from a ref being added and removed before the prison is fully created. I don't think there's a code path that does this, though it could possible happen in OSD modules.

Really though, this isn't intended as a standalone change, but part of a set. The next patch, removing the ability to "resurrect" a dying jail, relies on breaking the linkage between pr_uref and aliveness.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

jamie requested review of this revision.Jan 1 2021, 1:09 AM
jamie created this revision.

In theory I like this change. Can you please upload it with full context either using arc or if doing diff file upload using -U10000?

Add full context diff for kern_jail.c

It doesn't really make sense for this to be a separate revision, considering it's part of D28150 which overwrites much of the change here. So I'm rolling it all into a single revision instead.

A new version, built on top of D28419, and updated with other recent jail changes. With the addition of prison_isvalid() and prison_isalive(), the changes are significantly reduced.

One notable change is that prison_free no longer removed the final reference, leaving that for prison_complete/prison_deref.

In general looks ok to me; didn't entirely review it. "Glanced-at-by: bz"

kern/kern_jail.c
1870

You check for shared lock as well but then acquire an exclusive lock?
Do you need the exclusive lock or is that just so that you don't have to upgrade later? .. or would an slock suffice here?

kern/kern_jail.c
1870

Right, an slock is all that's needed for osd_jail_call(), and often what's set at this point - in fact allprison_lock is never xlocked in that path, but it seemed right to check that bit just on the safe side.

Later on, prison_deref() is expected to destroy the prison, calling prison_lock_xlock(). I would have called it here, except that the prison mutex itself needs to be unlocked for osd_jail_call().

This revision was not accepted when it landed; it landed in state Needs Review.Feb 21 2021, 9:26 PM
This revision was automatically updated to reflect the committed changes.