o Introduce pr_mtx into struct prison, providing protection for the

mutable contents of struct prison (hostname, securelevel, refcount,
  pr_linux, ...)
o Generally introduce mtx_lock()/mtx_unlock() calls throughout kern/
  so as to enforce these protections, in particular, in kern_mib.c
  protection sysctl access to the hostname and securelevel, as well as
  kern_prot.c access to the securelevel for access control purposes.
o Rewrite linux emulator abstractions for accessing per-jail linux
  mib entries (osname, osrelease, osversion) so that they don't return
  a pointer to the text in the struct linux_prison, rather, a copy
  to an array passed into the calls.  Likewise, update linprocfs to
  use these primitives.
o Update in_pcb.c to always use prison_getip() rather than directly
  accessing struct prison.

Reviewed by:	jhb

sys/compat/linprocfs/linprocfs.c

@@ -90,6 +90,7 @@ extern int ncpus;
 #include <machine/md_var.h>
 #endif /* __i386__ */
 
+#include <machine/../linux/linux.h>
 #include <compat/linux/linux_ioctl.h>
 #include <compat/linux/linux_mib.h>
 #include <compat/linux/linux_util.h>
@@ -444,11 +445,15 @@ linprocfs_douptime(PFS_FILL_ARGS)
 static int
 linprocfs_doversion(PFS_FILL_ARGS)
 {
+	char osname[LINUX_MAX_UTSNAME];
+	char osrelease[LINUX_MAX_UTSNAME];
+
+	linux_get_osname(td->td_proc, osname);
+	linux_get_osrelease(td->td_proc, osrelease);
+
 	sbuf_printf(sb,
 	    "%s version %s (des@freebsd.org) (gcc version " __VERSION__ ")"
-	    " #4 Sun Dec 18 04:30:00 CET 1977\n",
-	    linux_get_osname(td->td_proc),
-	    linux_get_osrelease(td->td_proc));
+	    " #4 Sun Dec 18 04:30:00 CET 1977\n", osname, osrelease);
 	return (0);
 }
 

sys/compat/linux/linux_mib.c

@@ -35,6 +35,8 @@
 #include <sys/proc.h>
 #include <sys/malloc.h>
 #include <sys/jail.h>
+#include <sys/lock.h>
+#include <sys/mutex.h>
 
 #include <machine/../linux/linux.h>
 #include <compat/linux/linux_mib.h>
@@ -56,7 +58,7 @@ linux_sysctl_osname(SYSCTL_HANDLER_ARGS)
 	char osname[LINUX_MAX_UTSNAME];
 	int error;
 
-	strcpy(osname, linux_get_osname(req->td->td_proc));
+	linux_get_osname(req->td->td_proc, osname);
 	error = sysctl_handle_string(oidp, osname, LINUX_MAX_UTSNAME, req);
 	if (error || req->newptr == NULL)
 		return (error);
@@ -77,7 +79,7 @@ linux_sysctl_osrelease(SYSCTL_HANDLER_ARGS)
 	char osrelease[LINUX_MAX_UTSNAME];
 	int error;
 
-	strcpy(osrelease, linux_get_osrelease(req->td->td_proc));
+	linux_get_osrelease(req->td->td_proc, osrelease);
 	error = sysctl_handle_string(oidp, osrelease, LINUX_MAX_UTSNAME, req);
 	if (error || req->newptr == NULL)
 		return (error);
@@ -111,8 +113,11 @@ SYSCTL_PROC(_compat_linux, OID_AUTO, oss_version,
 	    0, 0, linux_sysctl_oss_version, "I",
 	    "Linux OSS version");
 
+/*
+ * Returns holding the prison mutex if return non-NULL.
+ */
 static struct linux_prison *
-get_prison(struct proc *p)
+linux_get_prison(struct proc *p)
 {
 	register struct prison *pr;
 	register struct linux_prison *lpr;
@@ -122,30 +127,58 @@ get_prison(struct proc *p)
 
 	pr = p->p_ucred->cr_prison;
 
+	/*
+	 * Rather than hold the prison mutex during allocation, check to
+	 * see if we need to allocate while holding the mutex, release it,
+	 * allocate, then once we've allocated the memory, check again to
+	 * see if it's still needed, and set if appropriate.  If it's not,
+	 * we release the mutex again to FREE(), and grab it again so as
+	 * to release holding the lock.
+	 */
+	mtx_lock(&pr->pr_mtx);
 	if (pr->pr_linux == NULL) {
+		mtx_unlock(&pr->pr_mtx);
 		MALLOC(lpr, struct linux_prison *, sizeof *lpr,
 		    M_PRISON, M_WAITOK|M_ZERO);
-		pr->pr_linux = lpr;
+		mtx_lock(&pr->pr_mtx);
+		if (pr->pr_linux == NULL) {
+			pr->pr_linux = lpr;
+		} else {
+			mtx_unlock(&pr->pr_mtx);
+			FREE(lpr, M_PRISON);
+			mtx_lock(&pr->pr_mtx);
+		}
 	}
 
 	return (pr->pr_linux);
 }
 
-char *
-linux_get_osname(p)
+void
+linux_get_osname(p, dst)
 	struct proc *p;
+	char *dst;
 {
 	register struct prison *pr;
 	register struct linux_prison *lpr;
 
-	pr = p->p_ucred->cr_prison;
-	if (pr != NULL && pr->pr_linux != NULL) {
-		lpr = pr->pr_linux;
-		if (lpr->pr_osname[0])
-			return (lpr->pr_osname);
+	if (p->p_ucred->cr_prison == NULL) {
+		bcopy(linux_osname, dst, LINUX_MAX_UTSNAME);
+		return;
 	}
 
-	return (linux_osname);
+	pr = p->p_ucred->cr_prison;
+
+	mtx_lock(&pr->pr_mtx);
+	if (pr->pr_linux != NULL) {
+		lpr = (struct linux_prison *)pr->pr_linux;
+		if (lpr->pr_osname[0]) {
+			bcopy(lpr->pr_osname, dst, LINUX_MAX_UTSNAME);
+			mtx_unlock(&pr->pr_mtx);
+			return;
+		}
+	}
+	mtx_unlock(&pr->pr_mtx);
+	bcopy(linux_osname, dst, LINUX_MAX_UTSNAME);
 }
 
 int
@@ -155,30 +188,43 @@ linux_set_osname(p, osname)
 {
 	register struct linux_prison *lpr;
 
-	lpr = get_prison(p);
-	if (lpr != NULL)
+	lpr = linux_get_prison(p);
+	if (lpr != NULL) {
 		strcpy(lpr->pr_osname, osname);
-	else
+		mtx_unlock(&p->p_ucred->cr_prison->pr_mtx);
+	} else {
 		strcpy(linux_osname, osname);
+	}
 
 	return (0);
 }
 
-char *
-linux_get_osrelease(p)
+void
+linux_get_osrelease(p, dst)
 	struct proc *p;
+	char *dst;
 {
 	register struct prison *pr;
-	register struct linux_prison *lpr;
+	struct linux_prison *lpr;
 
-	pr = p->p_ucred->cr_prison;
-	if (pr != NULL && pr->pr_linux != NULL) {
-		lpr = pr->pr_linux;
-		if (lpr->pr_osrelease[0])
-			return (lpr->pr_osrelease);
+	if (p->p_ucred->cr_prison == NULL) {
+		bcopy(linux_osrelease, dst, LINUX_MAX_UTSNAME);
+		return;
 	}
 
-	return (linux_osrelease);
+	pr = p->p_ucred->cr_prison;
+
+	mtx_lock(&pr->pr_mtx);
+	if (pr->pr_linux != NULL) {
+		lpr = (struct linux_prison *) pr->pr_linux;
+		if (lpr->pr_osrelease[0]) {
+			bcopy(lpr->pr_osrelease, dst, LINUX_MAX_UTSNAME);
+			mtx_unlock(&pr->pr_mtx);
+			return;
+		}
+	}
+	mtx_unlock(&pr->pr_mtx);
+	bcopy(linux_osrelease, dst, LINUX_MAX_UTSNAME);
 }
 
 int
@@ -188,11 +234,13 @@ linux_set_osrelease(p, osrelease)
 {
 	register struct linux_prison *lpr;
 
-	lpr = get_prison(p);
-	if (lpr != NULL)
+	lpr = linux_get_prison(p);
+	if (lpr != NULL) {
 		strcpy(lpr->pr_osrelease, osrelease);
-	else
+		mtx_unlock(&p->p_ucred->cr_prison->pr_mtx);
+	} else {
 		strcpy(linux_osrelease, osrelease);
+	}
 
 	return (0);
 }
@@ -203,15 +251,27 @@ linux_get_oss_version(p)
 {
 	register struct prison *pr;
 	register struct linux_prison *lpr;
+	int version;
+
+	if (p->p_ucred->cr_prison == NULL)
+		return (linux_oss_version);
 
 	pr = p->p_ucred->cr_prison;
-	if (pr != NULL && pr->pr_linux != NULL) {
-		lpr = pr->pr_linux;
-		if (lpr->pr_oss_version)
-			return (lpr->pr_oss_version);
-	}
 
-	return (linux_oss_version);
+	mtx_lock(&pr->pr_mtx);
+	if (pr->pr_linux != NULL) {
+		lpr = (struct linux_prison *) pr->pr_linux;
+		if (lpr->pr_oss_version) {
+			version = lpr->pr_oss_version;
+		} else {
+			version = linux_oss_version;
+		}
+	} else {
+		version = linux_oss_version;
+	}
+	mtx_unlock(&pr->pr_mtx);
+
+	return (version);
 }
 
 int
@@ -221,11 +281,13 @@ linux_set_oss_version(p, oss_version)
 {
 	register struct linux_prison *lpr;
 
-	lpr = get_prison(p);
-	if (lpr != NULL)
+	lpr = linux_get_prison(p);
+	if (lpr != NULL) {
 		lpr->pr_oss_version = oss_version;
-	else
+		mtx_unlock(&p->p_ucred->cr_prison->pr_mtx);
+	} else {
 		linux_oss_version = oss_version;
+	}
 
 	return (0);
 }

sys/compat/linux/linux_mib.h

@@ -31,10 +31,10 @@
 #ifndef _LINUX_MIB_H_
 #define _LINUX_MIB_H_
 
-char*	linux_get_osname	__P((struct proc *p));
+void	linux_get_osname	__P((struct proc *p, char *dst));
 int	linux_set_osname	__P((struct proc *p, char *osname));
 
-char*	linux_get_osrelease	__P((struct proc *p));
+void	linux_get_osrelease	__P((struct proc *p, char *dst));
 int	linux_set_osrelease	__P((struct proc *p, char *osrelease));
 
 int	linux_get_oss_version	__P((struct proc *p));

sys/compat/linux/linux_misc.c

@@ -688,15 +688,16 @@ int
 linux_newuname(struct thread *td, struct linux_newuname_args *args)
 {
 	struct l_new_utsname utsname;
-	char *osrelease, *osname;
+	char osname[LINUX_MAX_UTSNAME];
+	char osrelease[LINUX_MAX_UTSNAME];
 
 #ifdef DEBUG
 	if (ldebug(newuname))
 		printf(ARGS(newuname, "*"));
 #endif
 
-	osname = linux_get_osname(td->td_proc);
-	osrelease = linux_get_osrelease(td->td_proc);
+	linux_get_osname(td->td_proc, osname);
+	linux_get_osrelease(td->td_proc, osrelease);
 
 	bzero(&utsname, sizeof(utsname));
 	strncpy(utsname.sysname, osname, LINUX_MAX_UTSNAME-1);

sys/fs/procfs/procfs_status.c

@@ -43,10 +43,10 @@
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/exec.h>
-#include <sys/jail.h>
 #include <sys/lock.h>
-#include <sys/malloc.h>
 #include <sys/mutex.h>
+#include <sys/jail.h>
+#include <sys/malloc.h>
 #include <sys/proc.h>
 #include <sys/resourcevar.h>
 #include <sys/tty.h>
@@ -178,11 +178,14 @@ procfs_dostatus(curp, p, pfs, uio)
 		DOCHECK();
 	}
 
-	if (jailed(p->p_ucred))
+	if (jailed(p->p_ucred)) {
+		mtx_lock(&p->p_ucred->cr_prison->pr_mtx);
 		ps += snprintf(ps, psbuf + sizeof(psbuf) - ps,
 		    " %s", p->p_ucred->cr_prison->pr_host);
-	else
+		mtx_unlock(&p->p_ucred->cr_prison->pr_mtx);
+	} else {
 		ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, " -");
+	}
 	DOCHECK();
 	ps += snprintf(ps, psbuf + sizeof(psbuf) - ps, "\n");
 	DOCHECK();

sys/kern/kern_jail.c

@@ -19,6 +19,8 @@
 #include <sys/malloc.h>
 #include <sys/proc.h>
 #include <sys/jail.h>
+#include <sys/lock.h>
+#include <sys/mutex.h>
 #include <sys/socket.h>
 #include <sys/sysctl.h>
 #include <net/if.h>
@@ -71,6 +73,7 @@ jail(td, uap)
 
 	mtx_lock(&Giant);
 	MALLOC(pr, struct prison *, sizeof *pr , M_PRISON, M_WAITOK | M_ZERO);
+	mtx_init(&pr->pr_mtx, "jail mutex", MTX_DEF);
 	pr->pr_securelevel = securelevel;
 	error = copyinstr(j.hostname, &pr->pr_host, sizeof pr->pr_host, 0);
 	if (error)
@@ -108,19 +111,33 @@ void
 prison_free(struct prison *pr)
 {
 
+	mtx_lock(&pr->pr_mtx);
 	pr->pr_ref--;
 	if (pr->pr_ref == 0) {
+		mtx_unlock(&pr->pr_mtx);
+		mtx_destroy(&pr->pr_mtx);
 		if (pr->pr_linux != NULL)
 			FREE(pr->pr_linux, M_PRISON);
 		FREE(pr, M_PRISON);
+		return;
 	}
+	mtx_unlock(&pr->pr_mtx);
 }
 
 void
 prison_hold(struct prison *pr)
 {
 
+	mtx_lock(&pr->pr_mtx);
 	pr->pr_ref++;
+	mtx_unlock(&pr->pr_mtx);
+}
+
+u_int32_t
+prison_getip(struct ucred *cred)
+{
+
+	return (cred->cr_prison->pr_ip);
 }
 
 int

sys/kern/kern_mib.c

@@ -48,6 +48,8 @@
 #include <sys/systm.h>
 #include <sys/sysctl.h>
 #include <sys/proc.h>
+#include <sys/lock.h>
+#include <sys/mutex.h>
 #include <sys/jail.h>
 #include <sys/smp.h>
 
@@ -155,14 +157,34 @@ static int
 sysctl_hostname(SYSCTL_HANDLER_ARGS)
 {
 	struct prison *pr;
+	char tmphostname[MAXHOSTNAMELEN];
 	int error;
 
 	pr = req->td->td_proc->p_ucred->cr_prison;
 	if (pr != NULL) {
 		if (!jail_set_hostname_allowed && req->newptr)
 			return (EPERM);
-		error = sysctl_handle_string(oidp, pr->pr_host,
+		/*
+		 * Process is in jail, so make a local copy of jail
+		 * hostname to get/set so we don't have to hold the jail
+		 * mutex during the sysctl copyin/copyout activities.
+		 */
+		mtx_lock(&pr->pr_mtx);
+		bcopy(pr->pr_host, tmphostname, MAXHOSTNAMELEN);
+		mtx_unlock(&pr->pr_mtx);
+
+		error = sysctl_handle_string(oidp, tmphostname,
 		    sizeof pr->pr_host, req);
+
+		if (req->newptr != NULL && error == 0) {
+			/*
+			 * Copy the locally set hostname to the jail, if
+			 * appropriate.
+			 */
+			mtx_lock(&pr->pr_mtx);
+			bcopy(tmphostname, pr->pr_host, MAXHOSTNAMELEN);
+			mtx_unlock(&pr->pr_mtx);
+		}
 	} else
 		error = sysctl_handle_string(oidp,
 		    hostname, sizeof hostname, req);
@@ -194,9 +216,11 @@ sysctl_kern_securelvl(SYSCTL_HANDLER_ARGS)
 	 * If the process is in jail, return the maximum of the global and
 	 * local levels; otherwise, return the global level.
 	 */
-	if (pr != NULL)
+	if (pr != NULL) {
+		mtx_lock(&pr->pr_mtx);
 		level = imax(securelevel, pr->pr_securelevel);
-	else
+		mtx_unlock(&pr->pr_mtx);
+	} else
 		level = securelevel;
 	error = sysctl_handle_int(oidp, &level, 0, req);
 	if (error || !req->newptr)
@@ -206,10 +230,14 @@ sysctl_kern_securelvl(SYSCTL_HANDLER_ARGS)
 	 * global level, and local level if any.
 	 */
 	if (pr != NULL) {
+		mtx_lock(&pr->pr_mtx);
 		if (!regression_securelevel_nonmonotonic &&
-		    (level < imax(securelevel, pr->pr_securelevel)))
+		    (level < imax(securelevel, pr->pr_securelevel))) {
+			mtx_unlock(&pr->pr_mtx);
 			return (EPERM);
+		}
 		pr->pr_securelevel = level;
+		mtx_unlock(&pr->pr_mtx);
 	} else {
 		if (!regression_securelevel_nonmonotonic &&
 		    (level < securelevel))

sys/kern/kern_prot.c

@@ -1278,9 +1278,12 @@ securelevel_gt(struct ucred *cr, int level)
 	active_securelevel = securelevel;
 	if (cr == NULL)
 		printf("securelevel_gt: cr is NULL\n");
-	if (cr->cr_prison != NULL)
+	if (cr->cr_prison != NULL) {
+		mtx_lock(&cr->cr_prison->pr_mtx);
 		active_securelevel = imax(cr->cr_prison->pr_securelevel,
 		    active_securelevel);
+		mtx_unlock(&cr->cr_prison->pr_mtx);
+	}
 	return (active_securelevel > level ? EPERM : 0);
 }
 
@@ -1292,9 +1295,12 @@ securelevel_ge(struct ucred *cr, int level)
 	active_securelevel = securelevel;
 	if (cr == NULL)
 		printf("securelevel_gt: cr is NULL\n");
-	if (cr->cr_prison != NULL)
+	if (cr->cr_prison != NULL) {
+		mtx_lock(&cr->cr_prison->pr_mtx);
 		active_securelevel = imax(cr->cr_prison->pr_securelevel,
 		    active_securelevel);
+		mtx_unlock(&cr->cr_prison->pr_mtx);
+	}
 	return (active_securelevel >= level ? EPERM : 0);
 }
 

sys/netinet/in_pcb.c

@@ -505,7 +505,7 @@ in_pcbconnect(inp, nam, td)
 	cred = inp->inp_socket->so_cred;
 	if (inp->inp_laddr.s_addr == INADDR_ANY && jailed(cred)) {
 		bzero(&sa, sizeof (sa));
-		sa.sin_addr.s_addr = htonl(cred->cr_prison->pr_ip);
+		sa.sin_addr.s_addr = htonl(prison_getip(cred));
 		sa.sin_len=sizeof (sa);
 		sa.sin_family = AF_INET;
 		error = in_pcbbind(inp, (struct sockaddr *)&sa, td);
@@ -1032,7 +1032,7 @@ prison_xinpcb(struct proc *p, struct inpcb *inp)
 {
 	if (!jailed(p->p_ucred))
 		return (0);
-	if (ntohl(inp->inp_laddr.s_addr) == p->p_ucred->cr_prison->pr_ip)
+	if (ntohl(inp->inp_laddr.s_addr) == prison_getip(p->p_ucred))
 		return (0);
 	return (1);
 }

sys/sys/jail.h

@@ -26,6 +26,9 @@ int jail __P((struct jail *));
 
 #else /* _KERNEL */
 
+#include <sys/_lock.h>
+#include <sys/_mutex.h>
+
 #ifdef MALLOC_DECLARE
 MALLOC_DECLARE(M_PRISON);
 #endif
@@ -35,20 +38,25 @@ MALLOC_DECLARE(M_PRISON);
  * ucreds's of the inmates.  pr_ref keeps track of them and is used to
  * delete the struture when the last inmate is dead.
  *
- * XXX: Note: this structure needs a mutex to protect the reference count
- * and other mutable fields (pr_host, pr_linux).
+ * Lock key:
+ *   (p) locked by pr_mutex
+ *   (c) set only during creation before the structure is shared, no mutex
+ *       required to read
  */
-
+struct mtx;
 struct prison {
-	int		pr_ref;
-	char 		pr_host[MAXHOSTNAMELEN];
-	u_int32_t	pr_ip;
-	void		*pr_linux;
-	int		pr_securelevel;
+	int		 pr_ref;			/* (p) refcount */
+	char 		 pr_host[MAXHOSTNAMELEN];	/* (p) jail hostname */
+	u_int32_t	 pr_ip;				/* (c) ip addr host */
+	void		*pr_linux;			/* (p) linux abi */
+	int		 pr_securelevel;		/* (p) securelevel */
+	struct mtx	 pr_mtx;
 };
 
 /*
  * Sysctl-set variables that determine global jail policy
+ *
+ * XXX MIB entries will need to be protected by a mutex.
  */
 extern int	jail_set_hostname_allowed;
 extern int	jail_socket_unixiproute_only;
@@ -62,6 +70,7 @@ struct sockaddr;
 int jailed __P((struct ucred *cred));
 int prison_check __P((struct ucred *cred1, struct ucred *cred2));
 void prison_free __P((struct prison *pr));
+u_int32_t prison_getip __P((struct ucred *cred));
 void prison_hold __P((struct prison *pr));
 int prison_if __P((struct ucred *cred, struct sockaddr *sa));
 int prison_ip __P((struct ucred *cred, int flag, u_int32_t *ip));