Split run_command up into an outer function (next_command) that chooses

 a single command string to run, and an inner function (run_command) that
 runs that single string.
Move the list of start/stop commands to run from a switch statement into
 an array, with a new placeholder parameter IP__OP for actually creating
 or removing the jail.
When jail creation fails, revert all non-exec commands in reverse order.

1 files changed, 171 insertions, 145 deletions

diff --git a/usr.sbin/jail/command.c b/usr.sbin/jail/command.c
index f757a83..ce90797 100644
--- a/usr.sbin/jail/command.c
+++ b/usr.sbin/jail/command.c
@@ -50,7 +50,6 @@ __FBSDID("$FreeBSD$");
 
 #include "jailp.h"
 
-#define COMSTRING_DUMMY		((struct cfstring *)1)
 #define DEFAULT_STOP_TIMEOUT	10
 #define PHASH_SIZE		256
 
@@ -66,12 +65,13 @@ int paralimit = -1;
 
 extern char **environ;
 
-static int get_user_info(struct cfjail *j, const char *username,
-    const struct passwd **pwdp, login_cap_t **lcapp);
+static int run_command(struct cfjail *j);
 static void add_proc(struct cfjail *j, pid_t pid);
 static void clear_procs(struct cfjail *j);
 static struct cfjail *find_proc(pid_t pid);
 static int term_procs(struct cfjail *j);
+static int get_user_info(struct cfjail *j, const char *username,
+    const struct passwd **pwdp, login_cap_t **lcapp);
 static int check_path(struct cfjail *j, const char *pname, const char *path,
     int isfile, const char *umount_type);
 
@@ -81,58 +81,188 @@ static struct phhead phash[PHASH_SIZE];
 static int kq;
 
 /*
- * Run a command associated with a jail, possibly inside the jail.
+ * Run the next command associated with a jail.
+ */
+int
+next_command(struct cfjail *j)
+{
+	const struct cfstring *comstring;
+	enum intparam comparam;
+	int rval, create_failed;
+
+	static struct cfstring dummystring = { .len = 1 };
+
+	rval = 0;
+	create_failed = (j->flags & (JF_STOP | JF_FAILED)) == JF_FAILED;
+	for (; (comparam = *j->comparam) && comparam != IP__OP;
+	     j->comparam += create_failed ? -1 : 1) {
+		if (j->comstring == NULL) {
+			switch (comparam) {
+			case IP_MOUNT_DEVFS:
+				if (!bool_param(j->intparams[IP_MOUNT_DEVFS]))
+					continue;
+				/* FALLTHROUGH */
+			case IP_STOP_TIMEOUT:
+				j->comstring = &dummystring;
+				break;
+			default:
+				if (j->intparams[comparam] == NULL)
+					continue;
+				j->comstring = create_failed
+				    ? TAILQ_LAST(&j->intparams[comparam]->val,
+					cfstrings)
+				    : TAILQ_FIRST(&j->intparams[comparam]->val);
+			}
+		}
+		for (; j->comstring != NULL;
+		     j->comstring = create_failed
+			? TAILQ_PREV(j->comstring, cfstrings, tq)
+			: TAILQ_NEXT(j->comstring, tq)) {
+			if (rval != 0)
+				return rval;
+			if (j->comstring->len == 0 || (create_failed &&
+			    (comparam == IP_EXEC_PRESTART || comparam ==
+			    IP_EXEC_START || comparam == IP_COMMAND ||
+			    comparam == IP_EXEC_POSTSTART)))
+				continue;
+			if (paralimit == 0) {
+				requeue(j, &runnable);
+				return 1;
+			}
+			rval = run_command(j);
+			create_failed =
+			    (j->flags & (JF_STOP | JF_FAILED)) == JF_FAILED;
+		}
+	}
+	return rval;
+}
+
+/*
+ * Check command exit status
  */
 int
-run_command(struct cfjail *j, enum intparam comparam)
+finish_command(struct cfjail *j)
+{
+	int error;
+
+	if (!(j->flags & JF_SLEEPQ))
+		return 0;
+	j->flags &= ~JF_SLEEPQ;
+	if (*j->comparam != IP_STOP_TIMEOUT) {
+		paralimit++;
+		if (!TAILQ_EMPTY(&runnable))
+			requeue(TAILQ_FIRST(&runnable), &ready);
+	}
+	error = 0;
+	if (j->flags & JF_TIMEOUT) {
+		j->flags &= ~JF_TIMEOUT;
+		if (*j->comparam != IP_STOP_TIMEOUT) {
+			jail_warnx(j, "%s: timed out", j->comline);
+			failed(j);
+			error = -1;
+		} else if (verbose > 0)
+			jail_note(j, "timed out\n");
+	} else if (j->pstatus != 0) {
+		if (WIFSIGNALED(j->pstatus))
+			jail_warnx(j, "%s: exited on signal %d",
+			    j->comline, WTERMSIG(j->pstatus));
+		else
+			jail_warnx(j, "%s: failed", j->comline);
+		j->pstatus = 0;
+		failed(j);
+		error = -1;
+	}
+	free(j->comline);
+	j->comline = NULL;
+	return error;
+}
+
+/*
+ * Check for finished processed or timeouts.
+ */
+struct cfjail *
+next_proc(int nonblock)
+{
+	struct kevent ke;
+	struct timespec ts;
+	struct timespec *tsp;
+	struct cfjail *j;
+
+	if (!TAILQ_EMPTY(&sleeping)) {
+	again:
+		tsp = NULL;
+		if ((j = TAILQ_FIRST(&sleeping)) && j->timeout.tv_sec) {
+			clock_gettime(CLOCK_REALTIME, &ts);
+			ts.tv_sec = j->timeout.tv_sec - ts.tv_sec;
+			ts.tv_nsec = j->timeout.tv_nsec - ts.tv_nsec;
+			if (ts.tv_nsec < 0) {
+				ts.tv_sec--;
+				ts.tv_nsec += 1000000000;
+			}
+			if (ts.tv_sec < 0 ||
+			    (ts.tv_sec == 0 && ts.tv_nsec == 0)) {
+				j->flags |= JF_TIMEOUT;
+				clear_procs(j);
+				return j;
+			}
+			tsp = &ts;
+		}
+		if (nonblock) {
+			ts.tv_sec = 0;
+			ts.tv_nsec = 0;
+			tsp = &ts;
+		}
+		switch (kevent(kq, NULL, 0, &ke, 1, tsp)) {
+		case -1:
+			if (errno != EINTR)
+				err(1, "kevent");
+			goto again;
+		case 0:
+			if (!nonblock) {
+				j = TAILQ_FIRST(&sleeping);
+				j->flags |= JF_TIMEOUT;
+				clear_procs(j);
+				return j;
+			}
+			break;
+		case 1:
+			(void)waitpid(ke.ident, NULL, WNOHANG);
+			if ((j = find_proc(ke.ident))) {
+				j->pstatus = ke.data;
+				return j;
+			}
+			goto again;
+		}
+	}
+	return NULL;
+}
+
+/*
+ * Run a single command for a jail, possible inside the jail.
+ */
+int
+run_command(struct cfjail *j)
 {
 	const struct passwd *pwd;
-	struct cfstring *comstring, *s;
+	const struct cfstring *comstring, *s;
 	login_cap_t *lcap;
 	char **argv;
 	char *cs, *addr, *comcs, *devpath;
 	const char *jidstr, *conslog, *path, *ruleset, *term, *username;
+	enum intparam comparam;
 	size_t comlen;
 	pid_t pid;
 	int argc, bg, clean, consfd, down, fib, i, injail, sjuser, timeout;
 
 	static char *cleanenv;
 
-	if (comparam) {
-		switch (comparam) {
-		case IP_MOUNT_DEVFS:
-			if (!bool_param(j->intparams[IP_MOUNT_DEVFS]))
-				return 0;
-			/* FALLTHROUGH */
-		case IP_STOP_TIMEOUT:
-			j->comstring = COMSTRING_DUMMY;
-			break;
-		default:
-			if (j->intparams[comparam] == NULL)
-				return 0;
-			j->comstring =
-			    TAILQ_FIRST(&j->intparams[comparam]->val);
-		}
-		j->comparam = comparam;
-	} else
-		comparam = j->comparam;
- next_comstring:
-	comstring = j->comstring;
-	if (comstring == NULL)
-		return 0;
-	if (paralimit == 0) {
-		requeue(j, &runnable);
-		return 1;
-	}
-	j->comstring =
-	    comstring == COMSTRING_DUMMY ? NULL : TAILQ_NEXT(comstring, tq);
-	if (comstring != COMSTRING_DUMMY && comstring->len == 0)
-		goto next_comstring;
 	/*
 	 * Collect exec arguments.  Internal commands for network and
 	 * mounting build their own argument lists.
 	 */
-	bg = j->flags & JF_FAILED;
+	comparam = *j->comparam;
+	comstring = j->comstring;
+	bg = 0;
 	down = j->flags & (JF_STOP | JF_FAILED);
 	switch (comparam) {
 	case IP_STOP_TIMEOUT:
@@ -169,7 +299,6 @@ run_command(struct cfjail *j, enum intparam comparam)
 		}
 		*(const char **)&argv[argc] = down ? "-alias" : "alias";
 		argv[argc + 1] = NULL;
-		j->flags |= JF_IFUP;
 		break;
 
 #ifdef INET6
@@ -195,7 +324,6 @@ run_command(struct cfjail *j, enum intparam comparam)
 			argc = 4;
 		*(const char **)&argv[argc] = down ? "-alias" : "alias";
 		argv[argc + 1] = NULL;
-		j->flags |= JF_IFUP;
 		break;	
 #endif
 
@@ -208,7 +336,6 @@ run_command(struct cfjail *j, enum intparam comparam)
 		*(const char **)&argv[3] =
 			jidstr ? jidstr : string_param(j->intparams[KP_NAME]);
 		argv[4] = NULL;
-		j->flags |= JF_IFUP;
 		break;
 
 	case IP_MOUNT:
@@ -221,7 +348,7 @@ run_command(struct cfjail *j, enum intparam comparam)
 		     cs = strtok(NULL, " \t\f\v\r\n"))
 			argv[argc++] = cs;
 		if (argc == 0)
-			goto next_comstring;
+			return 0;
 		if (argc < 3) {
 			jail_warnx(j, "%s: %s: missing information",
 			    j->intparams[comparam]->name, comstring->s);
@@ -252,7 +379,6 @@ run_command(struct cfjail *j, enum intparam comparam)
 			*(const char **)&argv[0] = _PATH_MOUNT;
 		}
 		*(const char **)&argv[1] = "-t";
-		j->flags |= JF_MOUNTED;
 		break;
 
 	case IP_MOUNT_DEVFS:
@@ -287,7 +413,6 @@ run_command(struct cfjail *j, enum intparam comparam)
 			    ruleset ? " " : "", ruleset ? ruleset : "");
 			argv[3] = NULL;
 		}
-		j->flags |= JF_MOUNTED;
 		break;
 
 	case IP_COMMAND:
@@ -333,9 +458,9 @@ run_command(struct cfjail *j, enum intparam comparam)
 			argv[argc] = NULL;
 		}
 	}
-
 	if (argv[0] == NULL)
-		goto next_comstring;
+		return 0;
+
 	if (int_param(j->intparams[IP_EXEC_TIMEOUT], &timeout) &&
 	    timeout != 0) {
 		clock_gettime(CLOCK_REALTIME, &j->timeout);
@@ -399,6 +524,7 @@ run_command(struct cfjail *j, enum intparam comparam)
 	if (bg)
 		setsid();
 
+	/* Set up the environment and run the command */
 	pwd = NULL;
 	lcap = NULL;
 	if ((clean || username) && injail && sjuser &&
@@ -462,106 +588,6 @@ run_command(struct cfjail *j, enum intparam comparam)
 }
 
 /*
- * Check command exit status
- */
-int
-finish_command(struct cfjail *j)
-{
-	int error;
-
-	if (!(j->flags & JF_SLEEPQ))
-		return 0;
-	j->flags &= ~JF_SLEEPQ;
-	if (j->comparam != IP_STOP_TIMEOUT) {
-		paralimit++;
-		if (!TAILQ_EMPTY(&runnable))
-			requeue(TAILQ_FIRST(&runnable), &ready);
-	}
-	error = 0;
-	if (j->flags & JF_TIMEOUT) {
-		j->flags &= ~JF_TIMEOUT;
-		if (j->comparam != IP_STOP_TIMEOUT) {
-			jail_warnx(j, "%s: timed out", j->comline);
-			failed(j);
-			error = -1;
-		} else if (verbose > 0)
-			jail_note(j, "timed out\n");
-	} else if (j->pstatus != 0) {
-		if (WIFSIGNALED(j->pstatus))
-			jail_warnx(j, "%s: exited on signal %d",
-			    j->comline, WTERMSIG(j->pstatus));
-		else
-			jail_warnx(j, "%s: failed", j->comline);
-		j->pstatus = 0;
-		failed(j);
-		error = -1;
-	}
-	free(j->comline);
-	j->comline = NULL;
-	return error;
-}
-
-/*
- * Check for finished processed or timeouts.
- */
-struct cfjail *
-next_proc(int nonblock)
-{
-	struct kevent ke;
-	struct timespec ts;
-	struct timespec *tsp;
-	struct cfjail *j;
-
-	if (!TAILQ_EMPTY(&sleeping)) {
-	again:
-		tsp = NULL;
-		if ((j = TAILQ_FIRST(&sleeping)) && j->timeout.tv_sec) {
-			clock_gettime(CLOCK_REALTIME, &ts);
-			ts.tv_sec = j->timeout.tv_sec - ts.tv_sec;
-			ts.tv_nsec = j->timeout.tv_nsec - ts.tv_nsec;
-			if (ts.tv_nsec < 0) {
-				ts.tv_sec--;
-				ts.tv_nsec += 1000000000;
-			}
-			if (ts.tv_sec < 0 ||
-			    (ts.tv_sec == 0 && ts.tv_nsec == 0)) {
-				j->flags |= JF_TIMEOUT;
-				clear_procs(j);
-				return j;
-			}
-			tsp = &ts;
-		}
-		if (nonblock) {
-			ts.tv_sec = 0;
-			ts.tv_nsec = 0;
-			tsp = &ts;
-		}
-		switch (kevent(kq, NULL, 0, &ke, 1, tsp)) {
-		case -1:
-			if (errno != EINTR)
-				err(1, "kevent");
-			goto again;
-		case 0:
-			if (!nonblock) {
-				j = TAILQ_FIRST(&sleeping);
-				j->flags |= JF_TIMEOUT;
-				clear_procs(j);
-				return j;
-			}
-			break;
-		case 1:
-			(void)waitpid(ke.ident, NULL, WNOHANG);
-			if ((j = find_proc(ke.ident))) {
-				j->pstatus = ke.data;
-				return j;
-			}
-			goto again;
-		}
-	}
-	return NULL;
-}
-
-/*
  * Add a process to the hash, tied to a jail.
  */
 static void