From owner-freebsd-bugs@FreeBSD.ORG Sun Mar 30 20:40:04 2008 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4D3AC1065671 for ; Sun, 30 Mar 2008 20:40:04 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 1FA058FC22 for ; Sun, 30 Mar 2008 20:40:04 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.2/8.14.2) with ESMTP id m2UKe48n004370 for ; Sun, 30 Mar 2008 20:40:04 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.2/8.14.1/Submit) id m2UKe41b004369; Sun, 30 Mar 2008 20:40:04 GMT (envelope-from gnats) Resent-Date: Sun, 30 Mar 2008 20:40:04 GMT Resent-Message-Id: <200803302040.m2UKe41b004369@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Ed Schouten Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F2FFA1065671 for ; Sun, 30 Mar 2008 20:34:50 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: from palm.hoeg.nl (mx0.hoeg.nl [IPv6:2001:610:652::211]) by mx1.freebsd.org (Postfix) with ESMTP id 6E4D08FC17 for ; Sun, 30 Mar 2008 20:34:50 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: by palm.hoeg.nl (Postfix, from userid 1000) id CA6481CCDE; Sun, 30 Mar 2008 22:34:48 +0200 (CEST) Message-Id: <20080330203448.CA6481CCDE@palm.hoeg.nl> Date: Sun, 30 Mar 2008 22:34:48 +0200 (CEST) From: Ed Schouten To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: kern/122270: [Patch] Jail numbers keep incrementing X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Ed Schouten List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 30 Mar 2008 20:40:04 -0000 >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 #include #include +#include #include #include #include @@ -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 #include -#define JAIL_MAX 999999 - #ifdef MALLOC_DECLARE MALLOC_DECLARE(M_PRISON); #endif >Release-Note: >Audit-Trail: >Unformatted: