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,
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to