run_command (mostly) cleanup:

Make the parallelism limit a global instead of always passing it
 to run_command and finish_command.
In the case of an empty command string, try to run any other strings
 the command may have.
Replace JF_BACKGROUND with its sort-of opposite JF_SLEEPQ.
Change j->comstring earlier to render JF_RUNQ unncessary.
Change the if-else series to a more readable switch statement.
Treat IP_STOP_TIMEOUT like a command, calling run_command which then
 calls term_procs.
When the IP_STOP_TIMEOUT "command" finishes, it shouldn't mess with
 the parallelism limit.
Make sufficient checks in finish_command and run_command so that
 the nonintuitive j->comstring null check isn't necessary to run them.
Rename the "waiting" queue to "depend", because the "sleeping" and
 "runnable" queues are also used to wait for something.

1 files changed, 150 insertions, 111 deletions

diff --git a/usr.sbin/jail/command.c b/usr.sbin/jail/command.c
index ac766cb..ca69474 100644
--- a/usr.sbin/jail/command.c
+++ b/usr.sbin/jail/command.c
@@ -62,6 +62,8 @@ struct phash {
 	pid_t			pid;
 };
 
+int paralimit = -1;
+
 extern char **environ;
 
 static int get_user_info(struct cfjail *j, const char *username,
@@ -69,6 +71,7 @@ static int get_user_info(struct cfjail *j, const char *username,
 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 check_path(struct cfjail *j, const char *pname, const char *path,
     int isfile, const char *umount_type);
 
@@ -81,7 +84,7 @@ static int kq;
  * Run a command associated with a jail, possibly inside the jail.
  */
 int
-run_command(struct cfjail *j, int *plimit, enum intparam comparam)
+run_command(struct cfjail *j, enum intparam comparam)
 {
 	const struct passwd *pwd;
 	struct cfstring *comstring, *s;
@@ -96,37 +99,47 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 	static char *cleanenv;
 
 	if (comparam) {
-		if (comparam == IP_MOUNT_DEVFS
-		    ? !bool_param(j->intparams[IP_MOUNT_DEVFS])
-		    : j->intparams[comparam] == NULL)
-			return 0;
+		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 =
+			    STAILQ_FIRST(&j->intparams[comparam]->val);
+		}
 		j->comparam = comparam;
-		j->comstring = comparam == IP_MOUNT_DEVFS ? COMSTRING_DUMMY
-		    : STAILQ_FIRST(&j->intparams[comparam]->val);
-	} else {
+	} else
 		comparam = j->comparam;
-		if (!(j->flags & JF_RUNQ))
-			j->comstring = j->comstring == COMSTRING_DUMMY
-			    ? NULL : STAILQ_NEXT(j->comstring, tq);
-	}
+ next_comstring:
 	comstring = j->comstring;
-	if (comstring == NULL ||
-	    (comstring != COMSTRING_DUMMY && comstring->len == 0))
+	if (comstring == NULL)
 		return 0;
-	if (plimit && *plimit == 0) {
-		j->flags |= JF_RUNQ;
+	if (paralimit == 0) {
 		requeue(j, &runnable);
 		return 1;
 	}
-	j->flags &= ~(JF_RUNQ | JF_BACKGROUND);
+	j->comstring =
+	    comstring == COMSTRING_DUMMY ? NULL : STAILQ_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 (XXX they should be
-	 * truly internal).
+	 * mounting build their own argument lists.
 	 */
 	bg = j->flags & JF_FAILED;
 	down = j->flags & (JF_STOP | JF_FAILED);
-	if (comparam == IP__IP4_IFADDR) {
+	switch (comparam) {
+	case IP_STOP_TIMEOUT:
+		/* This isn't really a command */
+		return term_procs(j);
+
+	case IP__IP4_IFADDR:
 		argv = alloca(8 * sizeof(char *));
 		*(const char **)&argv[0] = _PATH_IFCONFIG;
 		if ((cs = strchr(comstring->s, '|'))) {
@@ -157,8 +170,10 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 		*(const char **)&argv[argc] = down ? "-alias" : "alias";
 		argv[argc + 1] = NULL;
 		j->flags |= JF_IFUP;
+		break;
+
 #ifdef INET6
-	} else if (comparam == IP__IP6_IFADDR) {
+	case IP__IP6_IFADDR:
 		argv = alloca(8 * sizeof(char *));
 		*(const char **)&argv[0] = _PATH_IFCONFIG;
 		if ((cs = strchr(comstring->s, '|'))) {
@@ -181,8 +196,10 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 		*(const char **)&argv[argc] = down ? "-alias" : "alias";
 		argv[argc + 1] = NULL;
 		j->flags |= JF_IFUP;
+		break;	
 #endif
-	} else if (comparam == IP_VNET_INTERFACE) {
+
+	case IP_VNET_INTERFACE:
 		argv = alloca(5 * sizeof(char *));
 		*(const char **)&argv[0] = _PATH_IFCONFIG;
 		argv[1] = comstring->s;
@@ -192,7 +209,10 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 			jidstr ? jidstr : string_param(j->intparams[KP_NAME]);
 		argv[4] = NULL;
 		j->flags |= JF_IFUP;
-	} else if (comparam == IP_MOUNT || comparam == IP__MOUNT_FROM_FSTAB) {
+		break;
+
+	case IP_MOUNT:
+	case IP__MOUNT_FROM_FSTAB:
 		argv = alloca(8 * sizeof(char *));
 		comcs = alloca(comstring->len + 1);
 		strcpy(comcs, comstring->s);
@@ -201,7 +221,7 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 		     cs = strtok(NULL, " \t\f\v\r\n"))
 			argv[argc++] = cs;
 		if (argc == 0)
-			return 0;
+			goto next_comstring;
 		if (argc < 3) {
 			jail_warnx(j, "%s: %s: missing information",
 			    j->intparams[comparam]->name, comstring->s);
@@ -233,7 +253,9 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 		}
 		*(const char **)&argv[1] = "-t";
 		j->flags |= JF_MOUNTED;
-	} else if (comparam == IP_MOUNT_DEVFS) {
+		break;
+
+	case IP_MOUNT_DEVFS:
 		path = string_param(j->intparams[KP_PATH]);
 		if (path == NULL) {
 			jail_warnx(j, "mount.devfs: no path");
@@ -266,7 +288,11 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 			argv[3] = NULL;
 		}
 		j->flags |= JF_MOUNTED;
-	} else if (comparam == IP_COMMAND && j->name == NULL) {
+		break;
+
+	case IP_COMMAND:
+		if (j->name != NULL)
+			goto default_command;
 		argc = 0;
 		STAILQ_FOREACH(s, &j->intparams[IP_COMMAND]->val, tq)
 			argc++;
@@ -276,36 +302,40 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 			argv[argc++] = s->s;
 		argv[argc] = NULL;
 		j->comstring = NULL;
-	} else if ((cs = strpbrk(comstring->s, "!\"$&'()*;<>?[\\]`{|}~")) &&
-		   !(cs[0] == '&' && cs[1] == '\0')) {
-		argv = alloca(4 * sizeof(char *));
-		*(const char **)&argv[0] = _PATH_BSHELL;
-		*(const char **)&argv[1] = "-c";
-		argv[2] = comstring->s;
-		argv[3] = NULL;
-	} else {
-		if (cs) {
-			*cs = 0;
-			bg = 1;
+		break;
+
+	default:
+	default_command:
+		if ((cs = strpbrk(comstring->s, "!\"$&'()*;<>?[\\]`{|}~")) &&
+		    !(cs[0] == '&' && cs[1] == '\0')) {
+			argv = alloca(4 * sizeof(char *));
+			*(const char **)&argv[0] = _PATH_BSHELL;
+			*(const char **)&argv[1] = "-c";
+			argv[2] = comstring->s;
+			argv[3] = NULL;
+		} else {
+			if (cs) {
+				*cs = 0;
+				bg = 1;
+			}
+			comcs = alloca(comstring->len + 1);
+			strcpy(comcs, comstring->s);	
+			argc = 0;
+			for (cs = strtok(comcs, " \t\f\v\r\n"); cs;
+			     cs = strtok(NULL, " \t\f\v\r\n"))
+				argc++;
+			argv = alloca((argc + 1) * sizeof(char *));
+			strcpy(comcs, comstring->s);	
+			argc = 0;
+			for (cs = strtok(comcs, " \t\f\v\r\n"); cs;
+			     cs = strtok(NULL, " \t\f\v\r\n"))
+				argv[argc++] = cs;
+			argv[argc] = NULL;
 		}
-		comcs = alloca(comstring->len + 1);
-		strcpy(comcs, comstring->s);	
-		argc = 0;
-		for (cs = strtok(comcs, " \t\f\v\r\n"); cs;
-		     cs = strtok(NULL, " \t\f\v\r\n"))
-			argc++;
-		argv = alloca((argc + 1) * sizeof(char *));
-		strcpy(comcs, comstring->s);	
-		argc = 0;
-		for (cs = strtok(comcs, " \t\f\v\r\n"); cs;
-		     cs = strtok(NULL, " \t\f\v\r\n"))
-			argv[argc++] = cs;
-		argv[argc] = NULL;
 	}
-	if (argv[0] == NULL)
-		return 0;
 
-	j->pstatus = 0;
+	if (argv[0] == NULL)
+		goto next_comstring;
 	if (int_param(j->intparams[IP_EXEC_TIMEOUT], &timeout) &&
 	    timeout != 0) {
 		clock_gettime(CLOCK_REALTIME, &j->timeout);
@@ -357,10 +387,11 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
 		err(1, "fork");
 	if (pid > 0) {
 		if (bg) {
-			j->flags |= JF_BACKGROUND;
+			free(j->comline);
+			j->comline = NULL;
 			requeue(j, &ready);
 		} else {
-			--*plimit;
+			paralimit--;
 			add_proc(j, pid);
 		}
 		return 1;
@@ -434,15 +465,18 @@ run_command(struct cfjail *j, int *plimit, enum intparam comparam)
  * Check command exit status
  */
 int
-finish_command(struct cfjail *j, int *plimit)
+finish_command(struct cfjail *j)
 {
 	int error;
 
-	if (j->flags & (JF_RUNQ | JF_BACKGROUND))
+	if (!(j->flags & JF_SLEEPQ))
 		return 0;
-	++*plimit;
-	if (!TAILQ_EMPTY(&runnable))
-		requeue(TAILQ_FIRST(&runnable), &ready);
+	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;
@@ -458,10 +492,12 @@ finish_command(struct cfjail *j, int *plimit)
 			    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;
 }
 
@@ -526,54 +562,6 @@ next_proc(int nonblock)
 }
 
 /*
- * Send SIGTERM to all processes in a jail and wait for them to die.
- */
-int
-term_procs(struct cfjail *j)
-{
-	struct kinfo_proc *ki;
-	int i, noted, pcnt, timeout;
-
-	static kvm_t *kd;
-
-	if (!int_param(j->intparams[IP_STOP_TIMEOUT], &timeout))
-		timeout = DEFAULT_STOP_TIMEOUT;
-	else if (timeout == 0)
-		return 0;
-
-	if (kd == NULL) {
-		kd = kvm_open(NULL, NULL, NULL, O_RDONLY, "jail");
-		if (kd == NULL)
-			exit(1);
-	}
-
-	ki = kvm_getprocs(kd, KERN_PROC_PROC, 0, &pcnt);
-	if (ki == NULL)
-		exit(1);
-	noted = 0;
-	for (i = 0; i < pcnt; i++)
-		if (ki[i].ki_jid == j->jid &&
-		    kill(ki[i].ki_pid, SIGTERM) == 0) {
-			add_proc(j, ki[i].ki_pid);
-			if (verbose > 0) {
-				if (!noted) {
-					noted = 1;
-					jail_note(j, "sent SIGTERM to:");
-				}
-				printf(" %d", ki[i].ki_pid);
-			}
-		}
-	if (noted)
-		printf("\n");
-	if (j->nprocs > 0) {
-		clock_gettime(CLOCK_REALTIME, &j->timeout);
-		j->timeout.tv_sec += timeout;
-		return 1;
-	}
-	return 0;
-}
-
-/*
  * Add a process to the hash, tied to a jail.
  */
 static void
@@ -593,7 +581,11 @@ add_proc(struct cfjail *j, pid_t pid)
 	ph->pid = pid;
 	LIST_INSERT_HEAD(&phash[pid % PHASH_SIZE], ph, le);
 	j->nprocs++;
-	if (j->timeout.tv_sec) {
+	j->flags |= JF_SLEEPQ;
+	if (j->timeout.tv_sec == 0)
+		requeue(j, &sleeping);
+	else {
+		/* File the jail in the sleep queue acording to its timeout. */
 		TAILQ_REMOVE(j->queue, j, tq);
 		TAILQ_FOREACH(tj, &sleeping, tq) {
 			if (!tj->timeout.tv_sec ||
@@ -607,8 +599,7 @@ add_proc(struct cfjail *j, pid_t pid)
 		if (tj == NULL)
 			TAILQ_INSERT_TAIL(&sleeping, j, tq);
 		j->queue = &sleeping;
-	} else
-		requeue(j, &sleeping);
+	}
 }
 
 /*
@@ -653,6 +644,54 @@ find_proc(pid_t pid)
 }
 
 /*
+ * Send SIGTERM to all processes in a jail and wait for them to die.
+ */
+static int
+term_procs(struct cfjail *j)
+{
+	struct kinfo_proc *ki;
+	int i, noted, pcnt, timeout;
+
+	static kvm_t *kd;
+
+	if (!int_param(j->intparams[IP_STOP_TIMEOUT], &timeout))
+		timeout = DEFAULT_STOP_TIMEOUT;
+	else if (timeout == 0)
+		return 0;
+
+	if (kd == NULL) {
+		kd = kvm_open(NULL, NULL, NULL, O_RDONLY, "jail");
+		if (kd == NULL)
+			exit(1);
+	}
+
+	ki = kvm_getprocs(kd, KERN_PROC_PROC, 0, &pcnt);
+	if (ki == NULL)
+		exit(1);
+	noted = 0;
+	for (i = 0; i < pcnt; i++)
+		if (ki[i].ki_jid == j->jid &&
+		    kill(ki[i].ki_pid, SIGTERM) == 0) {
+			add_proc(j, ki[i].ki_pid);
+			if (verbose > 0) {
+				if (!noted) {
+					noted = 1;
+					jail_note(j, "sent SIGTERM to:");
+				}
+				printf(" %d", ki[i].ki_pid);
+			}
+		}
+	if (noted)
+		printf("\n");
+	if (j->nprocs > 0) {
+		clock_gettime(CLOCK_REALTIME, &j->timeout);
+		j->timeout.tv_sec += timeout;
+		return 1;
+	}
+	return 0;
+}
+
+/*
  * Look up a user in the passwd and login.conf files.
  */
 static int