Page MenuHomeFreeBSD
Differential D57858 Diff 181241 sys/kern/kern_jail.c

Changeset View

Standalone View

View Options

sys/kern/kern_jail.c

Show First 20 Lines • Show All 152 Lines • ▼ Show 20 Lines
struct prisonlist allprison = TAILQ_HEAD_INITIALIZER(allprison); struct prisonlist allprison = TAILQ_HEAD_INITIALIZER(allprison);
LIST_HEAD(, prison_racct) allprison_racct; LIST_HEAD(, prison_racct) allprison_racct;
int lastprid = 0; int lastprid = 0;
int lastdeadid = 0; int lastdeadid = 0;
static int get_next_prid(struct prison **insprp); static int get_next_prid(struct prison **insprp);
static int get_next_deadid(struct prison **insprp); static int get_next_deadid(struct prison **insprp);
static int do_jail_attach(struct thread *td, struct prison *pr, int *drflagsp); static int do_jail_attach(struct thread *td, struct prison *pr, int *drflagsp);
static int prison_attach_thread_single(struct thread *td);
static void prison_attach_thread_single_end(struct thread *td);
static void prison_complete(void *context, int pending); static void prison_complete(void *context, int pending);
static void prison_deref(struct prison *pr, int flags); static void prison_deref(struct prison *pr, int flags);
static void prison_deref_kill(struct prison *pr, struct prisonlist *freeprison); static void prison_deref_kill(struct prison *pr, struct prisonlist *freeprison);
static int prison_lock_xlock(struct prison *pr, int flags); static int prison_lock_xlock(struct prison *pr, int flags);
static void prison_cleanup_locked(struct prison *pr); static void prison_cleanup_locked(struct prison *pr);
static void prison_cleanup_unlocked(struct prison *pr); static void prison_cleanup_unlocked(struct prison *pr);
static void prison_free_not_last(struct prison *pr); static void prison_free_not_last(struct prison *pr);
static void prison_proc_free_not_last(struct prison *pr); static void prison_proc_free_not_last(struct prison *pr);
▲ Show 20 Lines • Show All 887 Lines • ▼ Show 20 Lines#endif
if (((flags & (JAIL_CREATE | JAIL_AT_DESC)) == JAIL_CREATE) && if (((flags & (JAIL_CREATE | JAIL_AT_DESC)) == JAIL_CREATE) &&
mypr->pr_childmax == 0) mypr->pr_childmax == 0)
return (EPERM); return (EPERM);
if (flags & ~JAIL_SET_MASK) if (flags & ~JAIL_SET_MASK)
return (EINVAL); return (EINVAL);
if ((flags & (JAIL_USE_DESC | JAIL_AT_DESC)) == if ((flags & (JAIL_USE_DESC | JAIL_AT_DESC)) ==
(JAIL_USE_DESC | JAIL_AT_DESC)) (JAIL_USE_DESC | JAIL_AT_DESC))
return (EINVAL); return (EINVAL);
prison_hold(mypr);
/* Only let a single thread in the process try to attach at a time. */
if (flags & JAIL_ATTACH) {
error = prison_attach_thread_single(td);
if (error != 0)
return (error);
}
#ifdef INET #ifdef INET
ip4 = NULL; ip4 = NULL;
#endif #endif
#ifdef INET6 #ifdef INET6
ip6 = NULL; ip6 = NULL;
#endif #endif
g_path = NULL; g_path = NULL;
jfp_out = NULL; jfp_out = NULL;
jfd_out = -1; jfd_out = -1;
markjUnsubmitted
Done
Inline Actions

I prefer that we annotate unlocked accesses like this using atomic(9): if (atomic_load_int(&p->p_flag) & P_HADTHREADS) != 0. It's a signal to readers and concurrency sanitizers that the unlocked access is "safe", in this case because 1) P_HADTHREADS is never cleared once set, and 2) if we're the only thread in the process, then P_HADTHREADS isn't going to be toggled concurrently.

Same comment elsewhere.

markj: I prefer that we annotate unlocked accesses like this using atomic(9): `if (atomic_load_int(&p…
prison_hold(mypr);
/* /*
* Check all the parameters before committing to anything. Not all * Check all the parameters before committing to anything. Not all
* errors can be caught early, but we may as well try. Also, this * errors can be caught early, but we may as well try. Also, this
* takes care of some expensive stuff (path lookup) before getting * takes care of some expensive stuff (path lookup) before getting
* the allprison lock. * the allprison lock.
* *
* XXX Jails are not filesystems, and jail parameters are not mount * XXX Jails are not filesystems, and jail parameters are not mount
* options. But it makes more sense to re-use the vfsopt code * options. But it makes more sense to re-use the vfsopt code
▲ Show 20 Lines • Show All 1,328 Lines • ▼ Show 20 Lines if (jfp_out != NULL)
fdrop(jfp_out, td); fdrop(jfp_out, td);
if (error && jfd_out >= 0) if (error && jfd_out >= 0)
(void)kern_close(td, jfd_out); (void)kern_close(td, jfd_out);
if (g_path != NULL) if (g_path != NULL)
free(g_path, M_TEMP); free(g_path, M_TEMP);
if (opts != NULL) if (opts != NULL)
vfs_freeopts(opts); vfs_freeopts(opts);
prison_free(mypr); prison_free(mypr);
if (flags & JAIL_ATTACH)
prison_attach_thread_single_end(td);
return (error); return (error);
} }
/* /*
* Find the next available prison ID. Return the ID on success, or zero * Find the next available prison ID. Return the ID on success, or zero
* on failure. Also set a pointer to the allprison list entry the prison * on failure. Also set a pointer to the allprison list entry the prison
* should be inserted before. * should be inserted before.
*/ */
▲ Show 20 Lines • Show All 629 Lines • ▼ Show 20 Lines
sys_jail_attach(struct thread *td, struct jail_attach_args *uap) sys_jail_attach(struct thread *td, struct jail_attach_args *uap)
{ {
struct prison *pr; struct prison *pr;
int drflags, error; int drflags, error;
error = priv_check(td, PRIV_JAIL_ATTACH); error = priv_check(td, PRIV_JAIL_ATTACH);
if (error) if (error)
return (error); return (error);
/* Only let a single thread in the process try to attach at a time. */
markjUnsubmitted
Done
Inline Actions

You might point to the comment in kern_jail_set(). (Or use a helper function to wrap thread_single().)

markj: You might point to the comment in kern_jail_set(). (Or use a helper function to wrap…
jamieAuthorUnsubmitted
Done
Inline Actions

I went with the help function route, which also neatens it up nicely. Each call has the single-line comment, and the function has the larger one that kern_jail_set has (a little more than that, actually).

jamie: I went with the help function route, which also neatens it up nicely. Each call has the…
error = prison_attach_thread_single(td);
if (error != 0)
return (error);
sx_slock(&allprison_lock); sx_slock(&allprison_lock);
drflags = PD_LIST_SLOCKED; drflags = PD_LIST_SLOCKED;
pr = prison_find_child(td->td_ucred->cr_prison, uap->jid); pr = prison_find_child(td->td_ucred->cr_prison, uap->jid);
if (pr == NULL) { if (pr == NULL) {
error = EINVAL; error = EINVAL;
goto done; goto done;
} }
Show All 10 Lines if (!prison_isalive(pr)) {
error = EINVAL; error = EINVAL;
goto done; goto done;
} }
error = do_jail_attach(td, pr, &drflags); error = do_jail_attach(td, pr, &drflags);
done: done:
prison_deref(pr, drflags); prison_deref(pr, drflags);
prison_attach_thread_single_end(td);
return (error); return (error);
} }
/* /*
* struct jail_attach_jd_args { * struct jail_attach_jd_args {
* int fd; * int fd;
* }; * };
*/ */
int int
sys_jail_attach_jd(struct thread *td, struct jail_attach_jd_args *uap) sys_jail_attach_jd(struct thread *td, struct jail_attach_jd_args *uap)
{ {
struct prison *pr; struct prison *pr;
struct ucred *jdcred; struct ucred *jdcred;
int drflags, error; int drflags, error;
/* Only let a single thread in the process try to attach at a time. */
error = prison_attach_thread_single(td);
if (error != 0)
return (error);
sx_slock(&allprison_lock); sx_slock(&allprison_lock);
drflags = PD_LIST_SLOCKED; drflags = PD_LIST_SLOCKED;
pr = NULL; pr = NULL;
error = jaildesc_find(td, uap->fd, &pr, &jdcred); error = jaildesc_find(td, uap->fd, &pr, &jdcred);
if (error) if (error)
goto done; goto done;
drflags |= PD_DEREF; drflags |= PD_DEREF;
error = priv_check_cred(jdcred, PRIV_JAIL_ATTACH); error = priv_check_cred(jdcred, PRIV_JAIL_ATTACH);
Show All 10 Lines if (!prison_isalive(pr)) {
error = EINVAL; error = EINVAL;
goto done; goto done;
} }
error = do_jail_attach(td, pr, &drflags); error = do_jail_attach(td, pr, &drflags);
done: done:
prison_deref(pr, drflags); prison_deref(pr, drflags);
prison_attach_thread_single_end(td);
return (error); return (error);
} }
/* /*
* Attach the current process to a prison. On entry, the allprison * Attach the current process to a prison. On entry, the allprison
* lock should be at least shared. On exit, both it and the prison * lock should be at least shared. On exit, both it and the prison
* itself will be unlocked, which will be refelected in *drflagsp. * itself will be unlocked, which will be refelected in *drflagsp.
*/ */
static int static int
do_jail_attach(struct thread *td, struct prison *pr, int *drflagsp) do_jail_attach(struct thread *td, struct prison *pr, int *drflagsp)
{ {
struct proc *p; struct proc *p;
struct ucred *newcred, *oldcred; struct ucred *newcred, *oldcred;
int drflags, error; int drflags, error;
sx_assert(&allprison_lock, SX_LOCKED); sx_assert(&allprison_lock, SX_LOCKED);
KASSERT(prison_isvalid(pr), ("Attaching to invalid prison %p", pr)); KASSERT(prison_isvalid(pr), ("Attaching to invalid prison %p", pr));
/* /*
* XXX: Note that there is a slight race here if two threads
* in the same privileged process attempt to attach to two
* different jails at the same time. It is important for
* user processes not to do this, or they might end up with
* a process root from one prison, but attached to the jail
* of another.
*/
/*
* Note the caller's locking state, but gain and track our own * Note the caller's locking state, but gain and track our own
* references. The caller will see that locks have been * references. The caller will see that locks have been
* dropped (which isn't true now, but will be after OSD calls). * dropped (which isn't true now, but will be after OSD calls).
*/ */
prison_hold(pr); prison_hold(pr);
refcount_acquire(&pr->pr_uref); refcount_acquire(&pr->pr_uref);
drflags = PD_DEREF | PD_DEUREF | (*drflagsp & PD_LOCK_FLAGS); drflags = PD_DEREF | PD_DEUREF | (*drflagsp & PD_LOCK_FLAGS);
*drflagsp &= PD_OP_FLAGS; *drflagsp &= PD_OP_FLAGS;
/* Let modules do whatever they need to prepare for attaching. */ /* Let modules do whatever they need to prepare for attaching. */
if (drflags & PD_LOCKED) { if (drflags & PD_LOCKED) {
mtx_unlock(&pr->pr_mtx); mtx_unlock(&pr->pr_mtx);
drflags &= ~PD_LOCKED; drflags &= ~PD_LOCKED;
} }
error = osd_jail_call(pr, PR_METHOD_ATTACH, td); error = osd_jail_call(pr, PR_METHOD_ATTACH, td);
if (error) if (error)
goto e_revert_osd; goto e_revert_osd;
sx_unlock(&allprison_lock); sx_unlock(&allprison_lock);
drflags &= ~(PD_LIST_SLOCKED | PD_LIST_XLOCKED); drflags &= ~(PD_LIST_SLOCKED | PD_LIST_XLOCKED);
/* /*
* Reparent the newly attached process to this jail. * Reparent the newly attached process to this jail.
*/ */
p = td->td_proc; p = td->td_proc;
kibUnsubmitted
Done
Inline Actions

Do not do this when holding any locks, esp. sleepable.

Also be aware that other threads get spurious EINTRs if sleeping in the slow syscalls.

kib: Do not do this when holding any locks, esp. sleepable. Also be aware that other threads get…
jamieAuthorUnsubmitted
Done
Inline Actions

I could drop allprison_lock before this, or move this test to the front of the system calls (especially considering the ERESTART stuff mentioned below).

Unless "do this" means the entire single-threaded section between here and and thread_single_end shouldn't be sleepable. If that's the case, I'll have to find a completely different strategy.

jamie: I could drop allprison_lock before this, or move this test to the front of the system calls…
kibUnsubmitted
Done
Inline Actions

Single threading should be done at the beginning of the syscall, if possible. After single threading is established, there is no limitation on the synchronization operations that the executing thread can perform. Almost no limitations, the existing shenanigans are related to threading and debugging, which should be not relevant there.

kib: Single threading should be done at the beginning of the syscall, if possible. After single…
jamieAuthorUnsubmitted
Done
Inline Actions

I'm content for spurious EINTR to be the caller's problem. This situation is unlikely to happen in real code, and the main focus is to keep things out of an invalid state (including possible security concerns, though no specific issue has been noted).

jamie: I'm content for spurious EINTR to be the caller's problem. This situation is unlikely to…
error = cpuset_setproc_update_set(p, pr->pr_cpuset); error = cpuset_setproc_update_set(p, pr->pr_cpuset);
if (error) if (error)
kibUnsubmitted
Done
Inline Actions

This causes spurious EAGAIN returned to the caller sometimes. Why not return ERESTART?

kib: This causes spurious EAGAIN returned to the caller sometimes. Why not return ERESTART?
jamieAuthorUnsubmitted
Done
Inline Actions

Does ERESTART make it to the kernel/user boundary? I couldn't tell from a quick look through.

For jail_attach and jail_attach_jd that would work. jail_set(JAIL_CREATE | JAIL_ATTACH) is more complicated since restarting the system call would fail on attempting to re-create the jail. The solution may be to make this test in kern_jail_set before starting anything.

jamie: Does ERESTART make it to the kernel/user boundary? I couldn't tell from a quick look through.
kibUnsubmitted
Done
Inline Actions

It makes it back to userspace, and the syscall instruction is re-issued.

kib: It makes it back to userspace, and the syscall instruction is re-issued.
goto e_revert_osd; goto e_revert_osd;
vn_lock(pr->pr_root, LK_EXCLUSIVE | LK_RETRY); vn_lock(pr->pr_root, LK_EXCLUSIVE | LK_RETRY);
if ((error = change_dir(pr->pr_root, td)) != 0) if ((error = change_dir(pr->pr_root, td)) != 0)
goto e_unlock; goto e_unlock;
#ifdef MAC #ifdef MAC
if ((error = mac_vnode_check_chroot(td->td_ucred, pr->pr_root))) if ((error = mac_vnode_check_chroot(td->td_ucred, pr->pr_root)))
goto e_unlock; goto e_unlock;
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines if (!prison_isalive(pr)) {
PROC_UNLOCK(p); PROC_UNLOCK(p);
} }
return (0); return (0);
e_unlock: e_unlock:
VOP_UNLOCK(pr->pr_root); VOP_UNLOCK(pr->pr_root);
e_revert_osd: e_revert_osd:
/* Tell modules this thread is still in its old jail after all. */ /* Tell modules this process is still in its old jail after all. */
if (!(drflags & (PD_LIST_SLOCKED | PD_LIST_XLOCKED))) { if (!(drflags & (PD_LIST_SLOCKED | PD_LIST_XLOCKED))) {
sx_slock(&allprison_lock); sx_slock(&allprison_lock);
drflags |= PD_LIST_SLOCKED; drflags |= PD_LIST_SLOCKED;
} }
(void)osd_jail_call(td->td_ucred->cr_prison, PR_METHOD_ATTACH, td); (void)osd_jail_call(td->td_ucred->cr_prison, PR_METHOD_ATTACH, td);
prison_deref(pr, drflags); prison_deref(pr, drflags);
return (error); return (error);
}
/*
* Only one thread in a process should try to attach to a jail, or
* they might end up with a process root from one prison, but attached
* to the jail of another. Enforce this by making the process run in
* single-threaded mode for the duration of the system call, which
* also prevents other related calls such as chroot.
*/
static int
prison_attach_thread_single(struct thread *td)
{
struct proc *p;
kibUnsubmitted
Done
Inline Actions

int error;' should go after struct proc *p;` (style suggest ordering by alignment descending)

kib: `int error;' should go after `struct proc *p;` (style suggest ordering by alignment descending)
int error;
error = 0;
p = td->td_proc;
if ((atomic_load_int(&p->p_flag) & P_HADTHREADS) != 0) {
PROC_LOCK(p);
if (thread_single(p, SINGLE_BOUNDARY))
kibUnsubmitted
Done
Inline Actions

If you introduce a local error variable, then PROC_UNLOCK() can be done only once:

error = 0;
if ((atomic_load_int(&p->p_flag) & P_HADTHREADS) != 0) {
        PROC_LOCK(p);
        if (thread_single(p, SINGLE_BOUNDARY))
                error = ERESTART;
        PROC_UNLOCK(p);
}
return (error);
kib: If you introduce a local error variable, then PROC_UNLOCK() can be done only once: ```…
error = ERESTART;
PROC_UNLOCK(p);
}
return (error);
}
static void
prison_attach_thread_single_end(struct thread *td)
{
struct proc *p;
p = td->td_proc;
if ((atomic_load_int(&p->p_flag) & P_HADTHREADS) != 0) {
PROC_LOCK(p);
thread_single_end(p, SINGLE_BOUNDARY);
PROC_UNLOCK(p);
}
} }
/* /*
* Returns a locked prison instance, or NULL on failure. * Returns a locked prison instance, or NULL on failure.
*/ */
struct prison * struct prison *
prison_find(int prid) prison_find(int prid)
{ {
▲ Show 20 Lines • Show All 2,468 Lines • Show Last 20 Lines