Attachment 169452 Details for Bug 48471 – Add PR_METHOD_REMOVE to jails, pass prison to PR_METHOD_CREATE

[patch] Add PR_METHOD_REMOVE to jails, pass prison to PR_METHOD_CREATE

jail-create-remove.diff (text/plain), 9.91 KB, created by Jamie Gritton on 2016-04-18 19:42:46 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jamie Gritton

Created: 2016-04-18 19:42:46 UTC

Size: 9.91 KB

patch

obsolete

>Index: kern/kern_jail.c
>===================================================================
>--- kern/kern_jail.c	(revision 298132)
>+++ kern/kern_jail.c	(working copy)
>@@ -557,8 +557,9 @@
> 	void *op;
> #endif
> 	unsigned long hid;
>-	size_t namelen, onamelen;
>-	int created, cuflags, descend, enforce, error, errmsg_len, errmsg_pos;
>+	size_t namelen, onamelen, pnamelen;
>+	int born, created, cuflags, descend, enforce;
>+	int error, errmsg_len, errmsg_pos;
> 	int gotchildmax, gotenforce, gothid, gotrsnum, gotslevel;
> 	int fi, jid, jsys, len, level;
> 	int childmax, osreldt, rsnum, slevel;
>@@ -581,7 +582,7 @@
> 		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)
>@@ -608,6 +609,13 @@
> #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;
>@@ -1013,36 +1021,11 @@
> 	}
> 
> 	/*
>-	 * 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);
>@@ -1049,6 +1032,7 @@
> 		if (*p != '\0')
> 			jid = 0;
> 	}
>+	sx_xlock(&allprison_lock);
> 	if (jid != 0) {
> 		/*
> 		 * See if a requested jid already exists.  There is an
>@@ -1114,6 +1098,7 @@
> 	 * 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)
>@@ -1124,7 +1109,6 @@
> 			 * 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') {
>@@ -1135,6 +1119,7 @@
> 					goto done_unlock_list;
> 				}
> 			} else {
>+				*namelc = '\0';
> 				ppr = prison_find_name(mypr, name);
> 				if (ppr == NULL) {
> 					error = ENOENT;
>@@ -1143,17 +1128,18 @@
> 					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);
>@@ -1227,10 +1213,11 @@
> 			}
> 		created = 1;
> 		mtx_lock(&ppr->pr_mtx);
>-		if (ppr->pr_ref == 0 || (ppr->pr_flags & PR_REMOVE)) {
>+		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++;
>@@ -1284,8 +1271,8 @@
> 		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;
>@@ -1365,7 +1352,7 @@
> 		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 {
>@@ -1579,13 +1566,13 @@
> 	}
> #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;
>@@ -1597,9 +1584,10 @@
> 		 * 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;
> 		}
>@@ -1616,6 +1604,30 @@
> 		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;
>@@ -1689,12 +1701,12 @@
> 		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,
>@@ -1772,6 +1784,7 @@
> 	 * for now, so new ones will remain unseen until after the module
> 	 * handlers have completed.
> 	 */
>+	born = pr->pr_uref == 0;
> 	if (!created && (ch_flags & PR_PERSIST & (pr_flags ^ pr->pr_flags))) {
> 		if (pr_flags & PR_PERSIST) {
> 			pr->pr_ref++;
>@@ -1841,15 +1854,20 @@
> 
> 	/* Let the modules do their work. */
> 	sx_downgrade(&allprison_lock);
>-	if (created) {
>+	if (born) {
> 		error = osd_jail_call(pr, PR_METHOD_CREATE, opts);
> 		if (error) {
>-			prison_deref(pr, PD_LIST_SLOCKED);
>+			(void)osd_jail_call(pr, PR_METHOD_REMOVE, NULL);
>+			prison_deref(pr, created
>+			    ? PD_LIST_SLOCKED
>+			    : PD_DEREF | PD_LIST_SLOCKED);
> 			goto done_errmsg;
> 		}
> 	}
> 	error = osd_jail_call(pr, PR_METHOD_SET, opts);
> 	if (error) {
>+		if (born)
>+			(void)osd_jail_call(pr, PR_METHOD_REMOVE, NULL);
> 		prison_deref(pr, created
> 		    ? PD_LIST_SLOCKED
> 		    : PD_DEREF | PD_LIST_SLOCKED);
>@@ -1901,7 +1919,7 @@
> 			sx_sunlock(&allprison_lock);
> 	}
> 
>-	goto done_errmsg;
>+	goto done_free;
> 
>  done_deref_locked:
> 	prison_deref(pr, created
>@@ -2281,7 +2299,6 @@
> 
> 	/* Remove all descendants of this prison, then remove this prison. */
> 	pr->pr_ref++;
>-	pr->pr_flags |= PR_REMOVE;
> 	if (!LIST_EMPTY(&pr->pr_children)) {
> 		mtx_unlock(&pr->pr_mtx);
> 		lpr = NULL;
>@@ -2290,7 +2307,6 @@
> 			if (cpr->pr_ref > 0) {
> 				tpr = cpr;
> 				cpr->pr_ref++;
>-				cpr->pr_flags |= PR_REMOVE;
> 			} else {
> 				/* Already removed - do not do it again. */
> 				tpr = NULL;
>@@ -2606,6 +2622,7 @@
> prison_deref(struct prison *pr, int flags)
> {
> 	struct prison *ppr, *tpr;
>+	int ref, lasturef;
> 
> 	if (!(flags & PD_LOCKED))
> 		mtx_lock(&pr->pr_mtx);
>@@ -2612,13 +2629,39 @@
> 	for (;;) {
> 		if (flags & PD_DEUREF) {
> 			pr->pr_uref--;
>+			lasturef = pr->pr_uref == 0;
>+			if (lasturef)
>+				pr->pr_ref++;
> 			KASSERT(prison0.pr_uref != 0, ("prison0 pr_uref=0"));
>-		}
>+		} else
>+			lasturef = 0;
> 		if (flags & PD_DEREF)
> 			pr->pr_ref--;
>+		ref = pr->pr_ref;
>+		mtx_unlock(&pr->pr_mtx);
>+
>+		/*
>+		 * Tell the modules if the last user reference was removed
>+		 * (even it sticks around in dying state).
>+		 */
>+		if (lasturef) {
>+			if (!(flags & (PD_LIST_SLOCKED | PD_LIST_XLOCKED))) {
>+				if (ref > 1) {
>+					sx_slock(&allprison_lock);
>+					flags |= PD_LIST_SLOCKED;
>+				} else {
>+					sx_xlock(&allprison_lock);
>+					flags |= PD_LIST_XLOCKED;
>+				}
>+			}
>+			(void)osd_jail_call(pr, PR_METHOD_REMOVE, NULL);
>+			mtx_lock(&pr->pr_mtx);
>+			ref = --pr->pr_ref;
>+			mtx_unlock(&pr->pr_mtx);
>+		}
>+
> 		/* If the prison still has references, nothing else to do. */
>-		if (pr->pr_ref > 0) {
>-			mtx_unlock(&pr->pr_mtx);
>+		if (ref > 0) {
> 			if (flags & PD_LIST_SLOCKED)
> 				sx_sunlock(&allprison_lock);
> 			else if (flags & PD_LIST_XLOCKED)
>@@ -2626,7 +2669,6 @@
> 			return;
> 		}
> 
>-		mtx_unlock(&pr->pr_mtx);
> 		if (flags & PD_LIST_SLOCKED) {
> 			if (!sx_try_upgrade(&allprison_lock)) {
> 				sx_sunlock(&allprison_lock);

Actions: View | Diff

Attachments on bug 48471: 28925 | 28926 | 157658 | 157661 | 169450 | 169452 | 169480