kern/122270: [Patch] Jail numbers keep incrementing

[Ed Schouten]

>Number:         122270
>Category:       kern
>Synopsis:       [Patch] Jail numbers keep incrementing
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sun Mar 30 20:40:03 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator:     Ed Schouten
>Release:        FreeBSD 8.0-CURRENT amd64
>Organization:
>Environment:
Today's CURRENT.
>Description:
The FreeBSD Jail code has some statements to make sure we never hand out
jails that share the same prison ID's. For some reason, it doesn't use
the unrhdr routines that exist inside the kernel.

If we just use unrhdr here, the jail numbers don't keep incrementing
forever, making jls/ps output a lot easier. No more 4-5 digit numbers
in your test setup, where you only have 10-20 jails.

It also removes some duplicate code from the kernel. We shouldn't roll
our own unit number allocators.
>How-To-Repeat:
Restart jails many times - jls will report very high digit jail numbers,
while you only have a small amount of jails.
>Fix:
--- sys/kern/kern_jail.c
+++ sys/kern/kern_jail.c
@@ -23,6 +23,7 @@
 #include <sys/proc.h>
 #include <sys/taskqueue.h>
 #include <sys/jail.h>
+#include <sys/limits.h>
 #include <sys/lock.h>
 #include <sys/mutex.h>
 #include <sys/sx.h>
@@ -78,11 +79,13 @@
     &jail_mount_allowed, 0,
     "Processes in jail can mount/unmount jail-friendly file systems");
 
-/* allprison, lastprid, and prisoncount are protected by allprison_lock. */
-struct	prisonlist allprison;
+/* allprison and prisoncount are protected by allprison_lock. */
 struct	sx allprison_lock;
-int	lastprid = 0;
+SX_SYSINIT(allprison_lock, &allprison_lock, "allprison");
+struct	prisonlist allprison = LIST_HEAD_INITIALIZER(allprison);
 int	prisoncount = 0;
+/* Prison number allocation */
+static struct unrhdr *prison_numpool;
 
 /*
  * List of jail services. Protected by allprison_lock.
@@ -108,8 +111,7 @@
 init_prison(void *data __unused)
 {
 
-	sx_init(&allprison_lock, "allprison");
-	LIST_INIT(&allprison);
+	prison_numpool = new_unrhdr(1, INT_MAX, NULL);
 }
 
 SYSINIT(prison, SI_SUB_INTRINSIC, SI_ORDER_ANY, init_prison, NULL);
@@ -123,11 +125,11 @@
 jail(struct thread *td, struct jail_args *uap)
 {
 	struct nameidata nd;
-	struct prison *pr, *tpr;
+	struct prison *pr;
 	struct prison_service *psrv;
 	struct jail j;
 	struct jail_attach_args jaa;
-	int vfslocked, error, tryprid;
+	int vfslocked, error, prid;
 
 	error = copyin(uap->jail, &j, sizeof(j));
 	if (error)
@@ -135,9 +137,15 @@
 	if (j.version != 0)
 		return (EINVAL);
 
+	/* Allocate prison number */
+	prid = alloc_unr(prison_numpool);
+	if (prid == -1)
+		return (EAGAIN);
+
 	MALLOC(pr, struct prison *, sizeof(*pr), M_PRISON, M_WAITOK | M_ZERO);
 	mtx_init(&pr->pr_mtx, "jail mutex", NULL, MTX_DEF);
 	pr->pr_ref = 1;
+	pr->pr_id = jaa.jid = prid;
 	error = copyinstr(j.path, &pr->pr_path, sizeof(pr->pr_path), 0);
 	if (error)
 		goto e_killmtx;
@@ -164,24 +172,8 @@
 		    M_PRISON, M_ZERO | M_WAITOK);
 	}
 
-	/* Determine next pr_id and add prison to allprison list. */
+	/* Add prison to allprison list. */
 	sx_xlock(&allprison_lock);
-	tryprid = lastprid + 1;
-	if (tryprid == JAIL_MAX)
-		tryprid = 1;
-next:
-	LIST_FOREACH(tpr, &allprison, pr_list) {
-		if (tpr->pr_id == tryprid) {
-			tryprid++;
-			if (tryprid == JAIL_MAX) {
-				sx_xunlock(&allprison_lock);
-				error = EAGAIN;
-				goto e_dropvnref;
-			}
-			goto next;
-		}
-	}
-	pr->pr_id = jaa.jid = lastprid = tryprid;
 	LIST_INSERT_HEAD(&allprison, pr, pr_list);
 	prisoncount++;
 	sx_downgrade(&allprison_lock);
@@ -213,6 +205,7 @@
 	VFS_UNLOCK_GIANT(vfslocked);
 e_killmtx:
 	mtx_destroy(&pr->pr_mtx);
+	free_unr(prison_numpool, pr->pr_id);
 	FREE(pr, M_PRISON);
 	return (error);
 }
@@ -346,6 +339,7 @@
 	mtx_destroy(&pr->pr_mtx);
 	if (pr->pr_linux != NULL)
 		FREE(pr->pr_linux, M_PRISON);
+	free_unr(prison_numpool, pr->pr_id);
 	FREE(pr, M_PRISON);
 }
 
--- sys/sys/jail.h
+++ sys/sys/jail.h
@@ -41,8 +41,6 @@
 #include <sys/_mutex.h>
 #include <sys/_task.h>
 
-#define JAIL_MAX	999999
-
 #ifdef MALLOC_DECLARE
 MALLOC_DECLARE(M_PRISON);
 #endif
>Release-Note:
>Audit-Trail:
>Unformatted: