diff --git a/lib/libc/sys/jail.2 b/lib/libc/sys/jail.2 --- a/lib/libc/sys/jail.2 +++ b/lib/libc/sys/jail.2 @@ -25,7 +25,7 @@ .\" .\" $FreeBSD$ .\" -.Dd February 19, 2021 +.Dd February 25, 2021 .Dt JAIL 2 .Os .Sh NAME @@ -187,7 +187,12 @@ .Fn jail_attach system call. .It Dv JAIL_DYING -Allow setting a jail that is in the process of being removed. +This is deprecated and has no effect. +It used to allow setting a jail that is in the process of being removed. +Now such jails are always replaced when a new jail is created with the same +.Va jid +or +.Va name . .El .Pp The diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c --- a/sys/kern/kern_jail.c +++ b/sys/kern/kern_jail.c @@ -136,8 +136,10 @@ struct prisonlist allprison = TAILQ_HEAD_INITIALIZER(allprison); LIST_HEAD(, prison_racct) allprison_racct; int lastprid = 0; +int lastdeadid = 0; static int get_next_prid(struct prison **insprp); +static int get_next_deadid(struct prison **insprp); static int do_jail_attach(struct thread *td, struct prison *pr, int drflags); static void prison_complete(void *context, int pending); static void prison_deref(struct prison *pr, int flags); @@ -519,7 +521,7 @@ #endif struct vfsopt *opt; struct vfsoptlist *opts; - struct prison *pr, *deadpr, *inspr, *mypr, *ppr, *tpr; + struct prison *pr, *deadpr, *dinspr, *inspr, *mypr, *ppr, *tpr; struct vnode *root; char *domain, *errmsg, *host, *name, *namelc, *p, *path, *uuid; char *g_path, *osrelstr; @@ -531,10 +533,10 @@ #endif unsigned long hid; size_t namelen, onamelen, pnamelen; - int born, created, cuflags, descend, drflags, enforce; + int created, cuflags, descend, drflags, enforce; int error, errmsg_len, errmsg_pos; int gotchildmax, gotenforce, gothid, gotrsnum, gotslevel; - int jid, jsys, len, level; + int deadid, jid, jsys, len, level; int childmax, osreldt, rsnum, slevel; #if defined(INET) || defined(INET6) int ii, ij; @@ -991,6 +993,7 @@ */ pr = NULL; inspr = NULL; + deadpr = NULL; if (cuflags == JAIL_CREATE && jid == 0 && name != NULL) { namelc = strrchr(name, '.'); jid = strtoul(namelc != NULL ? namelc + 1 : name, &p, 10); @@ -1020,69 +1023,38 @@ continue; if (inspr->pr_id > jid) break; - pr = inspr; - mtx_lock(&pr->pr_mtx); - drflags |= PD_LOCKED; + if (prison_isalive(inspr)) { + pr = inspr; + mtx_lock(&pr->pr_mtx); + drflags |= PD_LOCKED; + } else { + /* Note a dying jail to handle later. */ + deadpr = inspr; + } inspr = NULL; break; } - if (pr != NULL) { - /* Create: jid must not exist. */ - if (cuflags == JAIL_CREATE) { - /* - * Even creators that cannot see the jail will - * get EEXIST. - */ - error = EEXIST; - vfs_opterror(opts, "jail %d already exists", - jid); - goto done_deref; - } - if (!prison_ischild(mypr, pr)) { - /* - * Updaters get ENOENT if they cannot see the - * jail. This is true even for CREATE | UPDATE, - * which normally cannot give this error. - */ - error = ENOENT; - vfs_opterror(opts, "jail %d not found", jid); - goto done_deref; - } - ppr = pr->pr_parent; - if (!prison_isalive(ppr)) { - error = ENOENT; - vfs_opterror(opts, "jail %d is dying", - ppr->pr_id); - goto done_deref; - } - if (!prison_isalive(pr)) { - if (!(flags & JAIL_DYING)) { - error = ENOENT; - vfs_opterror(opts, "jail %d is dying", - jid); - goto done_deref; - } - if ((flags & JAIL_ATTACH) || - (pr_flags & PR_PERSIST)) { - /* - * A dying jail might be resurrected - * (via attach or persist), but first - * it must determine if another jail - * has claimed its name. Accomplish - * this by implicitly re-setting the - * name. - */ - if (name == NULL) - name = prison_name(mypr, pr); - } - } - } else { - /* Update: jid must exist. */ - if (cuflags == JAIL_UPDATE) { - error = ENOENT; - vfs_opterror(opts, "jail %d not found", jid); - goto done_deref; - } + if (cuflags == JAIL_CREATE && pr != NULL) { + /* + * Even creators that cannot see the jail will get + * EEXIST. + */ + error = EEXIST; + vfs_opterror(opts, "jail %d already exists", jid); + goto done_deref; + } + if ((pr == NULL) + ? cuflags == JAIL_UPDATE + : !prison_ischild(mypr, pr)) { + /* + * Updaters get ENOENT for nonexistent jails, or for + * jails they cannot see. The latter case is true + * even for CREATE | UPDATE, which normally cannot + * give this error. + */ + error = ENOENT; + vfs_opterror(opts, "jail %d not found", jid); + goto done_deref; } } /* @@ -1134,54 +1106,35 @@ if (namelc[0] != '\0') { pnamelen = (ppr == &prison0) ? 0 : strlen(ppr->pr_name) + 1; - deadpr = NULL; FOREACH_PRISON_CHILD(ppr, tpr) { - if (tpr != pr && - !strcmp(tpr->pr_name + pnamelen, namelc)) { - if (prison_isalive(tpr)) { - if (pr == NULL && - cuflags != JAIL_CREATE) { - /* - * Use this jail - * for updates. - */ - pr = tpr; - mtx_lock(&pr->pr_mtx); - drflags |= PD_LOCKED; - break; - } - /* - * Create, or update(jid): - * name must not exist in an - * active sibling jail. - */ - error = EEXIST; - vfs_opterror(opts, - "jail \"%s\" already exists", - name); - goto done_deref; - } - if (pr == NULL && - cuflags != JAIL_CREATE) { - deadpr = tpr; - } - } - } - /* If no active jail is found, use a dying one. */ - if (deadpr != NULL && pr == NULL) { - if (flags & JAIL_DYING) { - pr = deadpr; - mtx_lock(&pr->pr_mtx); - drflags |= PD_LOCKED; - } else if (cuflags == JAIL_UPDATE) { - error = ENOENT; + if (tpr == pr || !prison_isalive(tpr) || + strcmp(tpr->pr_name + pnamelen, namelc)) + continue; + if (cuflags == JAIL_CREATE || pr != NULL) { + /* + * Create, or update(jid): name must + * not exist in an active sibling jail. + */ + error = EEXIST; vfs_opterror(opts, - "jail \"%s\" is dying", name); + "jail \"%s\" already exists", name); goto done_deref; } + /* Use this jail for updates. */ + pr = tpr; + mtx_lock(&pr->pr_mtx); + drflags |= PD_LOCKED; + break; } - /* Update: name must exist if no jid. */ - else if (cuflags == JAIL_UPDATE && pr == NULL) { + /* + * Update: name must exist if no jid is specified. + * As with the jid case, the jail must be currently + * visible, or else even CREATE | UPDATE will get + * an error. + */ + if ((pr == NULL) + ? cuflags == JAIL_UPDATE + : !prison_isalive(pr)) { error = ENOENT; vfs_opterror(opts, "jail \"%s\" not found", name); @@ -1205,6 +1158,36 @@ vfs_opterror(opts, "prison limit exceeded"); goto done_deref; } + + if (deadpr != NULL) { + /* + * The prison being created has the same ID as a dying + * one. Handle this by giving the dying jail a new ID. + * This may cause some confusion to user space, but + * only to those listing dying jails. + */ + deadid = get_next_deadid(&dinspr); + if (deadid == 0) { + error = EAGAIN; + vfs_opterror(opts, "no available jail IDs"); + goto done_deref; + } + mtx_lock(&deadpr->pr_mtx); + deadpr->pr_id = deadid; + mtx_unlock(&deadpr->pr_mtx); + if (dinspr == deadpr) + inspr = deadpr; + else { + inspr = TAILQ_NEXT(deadpr, pr_list); + TAILQ_REMOVE(&allprison, deadpr, pr_list); + if (dinspr != NULL) + TAILQ_INSERT_AFTER(&allprison, dinspr, + deadpr, pr_list); + else + TAILQ_INSERT_HEAD(&allprison, deadpr, + pr_list); + } + } if (jid == 0 && (jid = get_next_prid(&inspr)) == 0) { error = EAGAIN; vfs_opterror(opts, "no available jail IDs"); @@ -1734,14 +1717,13 @@ * Persistent prisons get an extra reference, and prisons losing their * persist flag lose that reference. */ - born = !prison_isalive(pr); if (ch_flags & PR_PERSIST & (pr_flags ^ pr->pr_flags)) { if (pr_flags & PR_PERSIST) { prison_hold(pr); /* - * This may make a dead prison alive again, but wait - * to label it as such until after OSD calls have had - * a chance to run (and perhaps to fail). + * This may be a new prison's first user reference, + * but wait to call it alive until after OSD calls + * have had a chance to run (and perhaps to fail). */ refcount_acquire(&pr->pr_uref); } else { @@ -1756,7 +1738,7 @@ * Any errors past this point will need to de-persist newly created * prisons, as well as call remove methods. */ - if (born) + if (created) drflags |= PD_KILL; #ifdef RACCT @@ -1815,7 +1797,7 @@ #endif /* Let the modules do their work. */ - if (born) { + if (created) { error = osd_jail_call(pr, PR_METHOD_CREATE, opts); if (error) goto done_deref; @@ -1828,7 +1810,7 @@ * A new prison is now ready to be seen; either it has gained a user * reference via persistence, or is about to gain one via attachment. */ - if (born) { + if (created) { drflags = prison_lock_xlock(pr, drflags); pr->pr_state = PRISON_STATE_ALIVE; } @@ -1960,6 +1942,55 @@ return (jid); } +/* + * Find the next available ID for a renumbered dead prison. This is the same + * as get_next_prid, but counting backward from the end of the range. + */ +static int +get_next_deadid(struct prison **dinsprp) +{ + struct prison *dinspr; + int deadid, minid; + + deadid = lastdeadid ? lastdeadid - 1 : JAIL_MAX; + /* + * Take two reverse passes through the allprison list: first + * starting with the proposed deadid, then ending with it. + */ + for (minid = 1; minid != 0; ) { + TAILQ_FOREACH_REVERSE(dinspr, &allprison, prisonlist, pr_list) { + if (dinspr->pr_id > deadid) + continue; + if (dinspr->pr_id < deadid) { + /* Found an opening. */ + minid = 0; + break; + } + if (--deadid < minid) { + if (lastdeadid == minid || lastdeadid == 0) + { + /* + * The entire legal range + * has been traversed + */ + return 0; + } + /* Try again from the end. */ + deadid = JAIL_MAX; + minid = lastdeadid; + break; + } + } + if (dinspr == NULL) { + /* Found room at the beginning of the list. */ + break; + } + } + *dinsprp = dinspr; + lastdeadid = deadid; + return (deadid); +} + /* * struct jail_get_args { * struct iovec *iovp; diff --git a/sys/sys/jail.h b/sys/sys/jail.h --- a/sys/sys/jail.h +++ b/sys/sys/jail.h @@ -101,7 +101,7 @@ #define JAIL_UPDATE 0x02 /* Update parameters of existing jail */ #define JAIL_ATTACH 0x04 /* Attach to jail upon creation */ #define JAIL_DYING 0x08 /* Allow getting a dying jail */ -#define JAIL_SET_MASK 0x0f +#define JAIL_SET_MASK 0x0f /* JAIL_DYING is deprecated/ignored here */ #define JAIL_GET_MASK 0x08 #define JAIL_SYS_DISABLE 0 diff --git a/usr.sbin/jail/jail.8 b/usr.sbin/jail/jail.8 --- a/usr.sbin/jail/jail.8 +++ b/usr.sbin/jail/jail.8 @@ -25,7 +25,7 @@ .\" .\" $FreeBSD$ .\" -.Dd November 18, 2020 +.Dd February 25, 2021 .Dt JAIL 8 .Os .Sh NAME @@ -136,10 +136,6 @@ .Pp Other available options are: .Bl -tag -width indent -.It Fl d -Allow making changes to a dying jail, equivalent to the -.Va allow.dying -parameter. .It Fl f Ar conf_file Use configuration file .Ar conf_file @@ -207,6 +203,17 @@ .It Fl v Print a message on every operation, such as running commands and mounting filesystems. +.It Fl d +This is deprecated and is equivalent to the +.Va allow.dying +parameter, which is also deprecated. +It used to allow making changes to a +.Va dying +jail. +Now such jails are always replaced when a new jail is created with the same +.Va jid +or +.Va name . .El .Pp If no arguments are given after the options, the operation (except @@ -903,9 +910,14 @@ .Pa /proc directory. .It Va allow.dying -Allow making changes to a +This is deprecated and has no effect. +It used to allow making changes to a .Va dying jail. +Now such jails are always replaced when a new jail is created with the same +.Va jid +or +.Va name . .It Va depend Specify a jail (or jails) that this jail depends on. When this jail is to be created, any jail(s) it depends on must already exist. diff --git a/usr.sbin/jail/jail.c b/usr.sbin/jail/jail.c --- a/usr.sbin/jail/jail.c +++ b/usr.sbin/jail/jail.c @@ -65,7 +65,7 @@ static void clear_persist(struct cfjail *j); static int update_jail(struct cfjail *j); static int rdtun_params(struct cfjail *j, int dofail); -static void running_jid(struct cfjail *j, int dflag); +static void running_jid(struct cfjail *j); static void jail_quoted_warnx(const struct cfjail *j, const char *name_msg, const char *noname_msg); static int jailparam_set_note(const struct cfjail *j, struct jailparam *jp, @@ -140,7 +140,7 @@ char *JidFile; size_t sysvallen; unsigned op, pi; - int ch, docf, error, i, oldcl, sysval; + int ch, docf, error, i, oldcl, sysval, dying_warned; int dflag, Rflag; #if defined(INET) || defined(INET6) char *cs, *ncs; @@ -377,6 +377,7 @@ * operation on it. When that is done, the jail may be finished, * or it may go back for the next step. */ + dying_warned = 0; while ((j = next_jail())) { if (j->flags & JF_FAILED) { @@ -397,11 +398,13 @@ import_params(j) < 0) continue; } + if (j->intparams[IP_ALLOW_DYING] && !dying_warned) { + warnx("%s", "the 'allow.dying' parameter and '-d' flag" + "are deprecated and have no effect."); + dying_warned = 1; + } if (!j->jid) - running_jid(j, - (j->flags & (JF_SET | JF_DEPEND)) == JF_SET - ? dflag || bool_param(j->intparams[IP_ALLOW_DYING]) - : 0); + running_jid(j); if (finish_command(j)) continue; @@ -613,11 +616,10 @@ int create_jail(struct cfjail *j) { - struct iovec jiov[4]; struct stat st; - struct jailparam *jp, *setparams, *setparams2, *sjp; + struct jailparam *jp, *setparams, *sjp; const char *path; - int dopersist, ns, jid, dying, didfail; + int dopersist, ns; /* * Check the jail's path, with a better error message than jail_set @@ -657,57 +659,8 @@ *sjp++ = *jp; ns = sjp - setparams; - didfail = 0; j->jid = jailparam_set_note(j, setparams, ns, JAIL_CREATE); - if (j->jid < 0 && errno == EEXIST && - bool_param(j->intparams[IP_ALLOW_DYING]) && - int_param(j->intparams[KP_JID], &jid) && jid != 0) { - /* - * The jail already exists, but may be dying. - * Make sure it is, in which case an update is appropriate. - */ - jiov[0].iov_base = __DECONST(char *, "jid"); - jiov[0].iov_len = sizeof("jid"); - jiov[1].iov_base = &jid; - jiov[1].iov_len = sizeof(jid); - jiov[2].iov_base = __DECONST(char *, "dying"); - jiov[2].iov_len = sizeof("dying"); - jiov[3].iov_base = &dying; - jiov[3].iov_len = sizeof(dying); - if (jail_get(jiov, 4, JAIL_DYING) < 0) { - /* - * It could be that the jail just barely finished - * dying, or it could be that the jid never existed - * but the name does. In either case, another try - * at creating the jail should do the right thing. - */ - if (errno == ENOENT) - j->jid = jailparam_set_note(j, setparams, ns, - JAIL_CREATE); - } else if (dying) { - j->jid = jid; - if (rdtun_params(j, 1) < 0) { - j->jid = -1; - didfail = 1; - } else { - sjp = setparams2 = alloca((j->njp + dopersist) * - sizeof(struct jailparam)); - for (jp = setparams; jp < setparams + ns; jp++) - if (!JP_RDTUN(jp) || - !strcmp(jp->jp_name, "jid")) - *sjp++ = *jp; - j->jid = jailparam_set_note(j, setparams2, - sjp - setparams2, JAIL_UPDATE | JAIL_DYING); - /* - * Again, perhaps the jail just finished dying. - */ - if (j->jid < 0 && errno == ENOENT) - j->jid = jailparam_set_note(j, - setparams, ns, JAIL_CREATE); - } - } - } - if (j->jid < 0 && !didfail) { + if (j->jid < 0) { jail_warnx(j, "%s", jail_errmsg); failed(j); } @@ -772,9 +725,7 @@ if (!JP_RDTUN(jp)) *++sjp = *jp; - jid = jailparam_set_note(j, setparams, ns, - bool_param(j->intparams[IP_ALLOW_DYING]) - ? JAIL_UPDATE | JAIL_DYING : JAIL_UPDATE); + jid = jailparam_set_note(j, setparams, ns, JAIL_UPDATE); if (jid < 0) { jail_warnx(j, "%s", jail_errmsg); failed(j); @@ -813,8 +764,7 @@ rtjp->jp_value = NULL; } rval = 0; - if (jailparam_get(rtparams, nrt, - bool_param(j->intparams[IP_ALLOW_DYING]) ? JAIL_DYING : 0) > 0) { + if (jailparam_get(rtparams, nrt, 0) > 0) { rtjp = rtparams + 1; for (jp = j->jp; rtjp < rtparams + nrt; jp++) { if (JP_RDTUN(jp) && strcmp(jp->jp_name, "jid")) { @@ -851,7 +801,7 @@ * Get the jail's jid if it is running. */ static void -running_jid(struct cfjail *j, int dflag) +running_jid(struct cfjail *j) { struct iovec jiov[2]; const char *pval; @@ -877,7 +827,7 @@ j->jid = -1; return; } - j->jid = jail_get(jiov, 2, dflag ? JAIL_DYING : 0); + j->jid = jail_get(jiov, 2, 0); } static void @@ -906,10 +856,9 @@ jid = jailparam_set(jp, njp, flags); if (verbose > 0) { - jail_note(j, "jail_set(%s%s)", + jail_note(j, "jail_set(%s)", (flags & (JAIL_CREATE | JAIL_UPDATE)) == JAIL_CREATE - ? "JAIL_CREATE" : "JAIL_UPDATE", - (flags & JAIL_DYING) ? " | JAIL_DYING" : ""); + ? "JAIL_CREATE" : "JAIL_UPDATE"); for (i = 0; i < njp; i++) { printf(" %s", jp[i].jp_name); if (jp[i].jp_value == NULL)