Date: Mon, 25 Apr 2016 04:27:59 +0000 (UTC) From: Jamie Gritton <jamie@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r298566 - head/sys/kern Message-ID: <201604250427.u3P4RxCf093894@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jamie Date: Mon Apr 25 04:27:58 2016 New Revision: 298566 URL: https://svnweb.freebsd.org/changeset/base/298566 Log: Pass the current/new jail to PR_METHOD_CHECK, which pushes the call until after the jail is found or created. This requires unlocking the jail for the call and re-locking it afterward, but that works because nothing in the jail has been changed yet, and other processes won't change the important fields as long as allprison_lock remains held. Keep better track of name vs namelc in kern_jail_set. Name should always be the hierarchical name (relative to the caller), and namelc the last component. PR: 48471 MFC after: 5 days Modified: head/sys/kern/kern_jail.c Modified: head/sys/kern/kern_jail.c ============================================================================== --- head/sys/kern/kern_jail.c Mon Apr 25 04:24:00 2016 (r298565) +++ head/sys/kern/kern_jail.c Mon Apr 25 04:27:58 2016 (r298566) @@ -555,7 +555,7 @@ kern_jail_set(struct thread *td, struct void *op; #endif unsigned long hid; - size_t namelen, onamelen; + size_t namelen, onamelen, pnamelen; int born, created, cuflags, descend, enforce; int error, errmsg_len, errmsg_pos; int gotchildmax, gotenforce, gothid, gotrsnum, gotslevel; @@ -580,7 +580,7 @@ kern_jail_set(struct thread *td, struct error = priv_check(td, PRIV_JAIL_ATTACH); if (error) return (error); - mypr = ppr = td->td_ucred->cr_prison; + mypr = td->td_ucred->cr_prison; if ((flags & JAIL_CREATE) && mypr->pr_childmax == 0) return (EPERM); if (flags & ~JAIL_SET_MASK) @@ -607,6 +607,13 @@ kern_jail_set(struct thread *td, struct #endif g_path = NULL; + cuflags = flags & (JAIL_CREATE | JAIL_UPDATE); + if (!cuflags) { + error = EINVAL; + vfs_opterror(opts, "no valid operation (create or update)"); + goto done_errmsg; + } + error = vfs_copyopt(opts, "jid", &jid, sizeof(jid)); if (error == ENOENT) jid = 0; @@ -1009,42 +1016,18 @@ kern_jail_set(struct thread *td, struct } /* - * Grab the allprison lock before letting modules check their - * parameters. Once we have it, do not let go so we'll have a - * consistent view of the OSD list. - */ - sx_xlock(&allprison_lock); - error = osd_jail_call(NULL, PR_METHOD_CHECK, opts); - if (error) - goto done_unlock_list; - - /* By now, all parameters should have been noted. */ - TAILQ_FOREACH(opt, opts, link) { - if (!opt->seen && strcmp(opt->name, "errmsg")) { - error = EINVAL; - vfs_opterror(opts, "unknown parameter: %s", opt->name); - goto done_unlock_list; - } - } - - /* - * See if we are creating a new record or updating an existing one. + * Find the specified jail, or at least its parent. * This abuses the file error codes ENOENT and EEXIST. */ - cuflags = flags & (JAIL_CREATE | JAIL_UPDATE); - if (!cuflags) { - error = EINVAL; - vfs_opterror(opts, "no valid operation (create or update)"); - goto done_unlock_list; - } pr = NULL; - namelc = NULL; + ppr = mypr; if (cuflags == JAIL_CREATE && jid == 0 && name != NULL) { namelc = strrchr(name, '.'); jid = strtoul(namelc != NULL ? namelc + 1 : name, &p, 10); if (*p != '\0') jid = 0; } + sx_xlock(&allprison_lock); if (jid != 0) { /* * See if a requested jid already exists. There is an @@ -1110,6 +1093,7 @@ kern_jail_set(struct thread *td, struct * and updates keyed by the name itself (where the name must exist * because that is the jail being updated). */ + namelc = NULL; if (name != NULL) { namelc = strrchr(name, '.'); if (namelc == NULL) @@ -1120,7 +1104,6 @@ kern_jail_set(struct thread *td, struct * parent and child names, and make sure the parent * exists or matches an already found jail. */ - *namelc = '\0'; if (pr != NULL) { if (strncmp(name, ppr->pr_name, namelc - name) || ppr->pr_name[namelc - name] != '\0') { @@ -1131,6 +1114,7 @@ kern_jail_set(struct thread *td, struct goto done_unlock_list; } } else { + *namelc = '\0'; ppr = prison_find_name(mypr, name); if (ppr == NULL) { error = ENOENT; @@ -1139,17 +1123,18 @@ kern_jail_set(struct thread *td, struct goto done_unlock_list; } mtx_unlock(&ppr->pr_mtx); + *namelc = '.'; } - name = ++namelc; + namelc++; } - if (name[0] != '\0') { - namelen = + if (namelc[0] != '\0') { + pnamelen = (ppr == &prison0) ? 0 : strlen(ppr->pr_name) + 1; name_again: deadpr = NULL; FOREACH_PRISON_CHILD(ppr, tpr) { if (tpr != pr && tpr->pr_ref > 0 && - !strcmp(tpr->pr_name + namelen, name)) { + !strcmp(tpr->pr_name + pnamelen, namelc)) { if (pr == NULL && cuflags != JAIL_CREATE) { mtx_lock(&tpr->pr_mtx); @@ -1226,7 +1211,8 @@ kern_jail_set(struct thread *td, struct if (ppr->pr_ref == 0) { mtx_unlock(&ppr->pr_mtx); error = ENOENT; - vfs_opterror(opts, "parent jail went away!"); + vfs_opterror(opts, "jail \"%s\" not found", + prison_name(mypr, ppr)); goto done_unlock_list; } ppr->pr_ref++; @@ -1280,8 +1266,8 @@ kern_jail_set(struct thread *td, struct pr->pr_id = jid; /* Set some default values, and inherit some from the parent. */ - if (name == NULL) - name = ""; + if (namelc == NULL) + namelc = ""; if (path == NULL) { path = "/"; root = mypr->pr_root; @@ -1361,7 +1347,7 @@ kern_jail_set(struct thread *td, struct mtx_lock(&pr->pr_mtx); /* * New prisons do not yet have a reference, because we do not - * want other to see the incomplete prison once the + * want others to see the incomplete prison once the * allprison_lock is downgraded. */ } else { @@ -1575,13 +1561,13 @@ kern_jail_set(struct thread *td, struct } #endif onamelen = namelen = 0; - if (name != NULL) { + if (namelc != NULL) { /* Give a default name of the jid. Also allow the name to be * explicitly the jid - but not any other number, and only in * normal form (no leading zero/etc). */ - if (name[0] == '\0') - snprintf(name = numbuf, sizeof(numbuf), "%d", jid); + if (namelc[0] == '\0') + snprintf(namelc = numbuf, sizeof(numbuf), "%d", jid); else if ((strtoul(namelc, &p, 10) != jid || namelc[0] < '1' || namelc[0] > '9') && *p == '\0') { error = EINVAL; @@ -1593,9 +1579,10 @@ kern_jail_set(struct thread *td, struct * Make sure the name isn't too long for the prison or its * children. */ - onamelen = strlen(pr->pr_name); - namelen = strlen(name); - if (strlen(ppr->pr_name) + namelen + 2 > sizeof(pr->pr_name)) { + pnamelen = (ppr == &prison0) ? 0 : strlen(ppr->pr_name) + 1; + onamelen = strlen(pr->pr_name + pnamelen); + namelen = strlen(namelc); + if (pnamelen + namelen + 1 > sizeof(pr->pr_name)) { error = ENAMETOOLONG; goto done_deref_locked; } @@ -1612,6 +1599,30 @@ kern_jail_set(struct thread *td, struct goto done_deref_locked; } + /* + * Let modules check their parameters. This requires unlocking and + * then re-locking the prison, but this is still a valid state as long + * as allprison_lock remains xlocked. + */ + mtx_unlock(&pr->pr_mtx); + error = osd_jail_call(pr, PR_METHOD_CHECK, opts); + if (error != 0) { + prison_deref(pr, created + ? PD_LIST_XLOCKED + : PD_DEREF | PD_LIST_XLOCKED); + goto done_releroot; + } + mtx_lock(&pr->pr_mtx); + + /* At this point, all valid parameters should have been noted. */ + TAILQ_FOREACH(opt, opts, link) { + if (!opt->seen && strcmp(opt->name, "errmsg")) { + error = EINVAL; + vfs_opterror(opts, "unknown parameter: %s", opt->name); + goto done_deref_locked; + } + } + /* Set the parameters of the prison. */ #ifdef INET redo_ip4 = 0; @@ -1685,12 +1696,12 @@ kern_jail_set(struct thread *td, struct FOREACH_PRISON_DESCENDANT_LOCKED(pr, tpr, descend) tpr->pr_devfs_rsnum = rsnum; } - if (name != NULL) { + if (namelc != NULL) { if (ppr == &prison0) - strlcpy(pr->pr_name, name, sizeof(pr->pr_name)); + strlcpy(pr->pr_name, namelc, sizeof(pr->pr_name)); else snprintf(pr->pr_name, sizeof(pr->pr_name), "%s.%s", - ppr->pr_name, name); + ppr->pr_name, namelc); /* Change this component of child names. */ FOREACH_PRISON_DESCENDANT_LOCKED(pr, tpr, descend) { bcopy(tpr->pr_name + onamelen, tpr->pr_name + namelen,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201604250427.u3P4RxCf093894>