svn commit: r261266 - in head: sys/dev/drm sys/kern sys/sys usr.sbin/jail

[Robert Watson]

Hi Jamie:

As these privileges basically allows root processes in jail to break out of 
jail, I think this needs a much more clear signpost that this is a very unsafe 
thing to turn on.  I can imagine scenarios where this might be useful, but 
can't really imagine any where it is 'safe' with respect to the jail model.

Can we put a very large and very clear warning in the jail(8) man page, as 
well as a comment in the kernel source code about this?

Robert

On Wed, 29 Jan 2014, Jamie Gritton wrote:

> Author: jamie
> Date: Wed Jan 29 13:41:13 2014
> New Revision: 261266
> URL: http://svnweb.freebsd.org/changeset/base/261266
>
> Log:
>  Add a jail parameter, allow.kmem, which lets jailed processes access
>  /dev/kmem and related devices (i.e. grants PRIV_IO and PRIV_KMEM_WRITE).
>  This in conjunction with changing the drm driver's permission check from
>  PRIV_DRIVER to PRIV_KMEM_WRITE will allow a jailed Xorg server.
>
>  Submitted by:	netchild
>  MFC after:	1 week
>
> Modified:
>  head/sys/dev/drm/drmP.h
>  head/sys/kern/kern_jail.c
>  head/sys/sys/jail.h
>  head/usr.sbin/jail/jail.8
>
> Modified: head/sys/dev/drm/drmP.h
> ==============================================================================
> --- head/sys/dev/drm/drmP.h	Wed Jan 29 13:35:12 2014	(r261265)
> +++ head/sys/dev/drm/drmP.h	Wed Jan 29 13:41:13 2014	(r261266)
> @@ -227,7 +227,9 @@ enum {
>
> #define PAGE_ALIGN(addr) round_page(addr)
> /* DRM_SUSER returns true if the user is superuser */
> -#if __FreeBSD_version >= 700000
> +#if __FreeBSD_version >= 1000000
> +#define DRM_SUSER(p)		(priv_check(p, PRIV_KMEM_WRITE) == 0)
> +#elif __FreeBSD_version >= 700000
> #define DRM_SUSER(p)		(priv_check(p, PRIV_DRIVER) == 0)
> #else
> #define DRM_SUSER(p)		(suser(p) == 0)
>
> Modified: head/sys/kern/kern_jail.c
> ==============================================================================
> --- head/sys/kern/kern_jail.c	Wed Jan 29 13:35:12 2014	(r261265)
> +++ head/sys/kern/kern_jail.c	Wed Jan 29 13:41:13 2014	(r261266)
> @@ -208,6 +208,7 @@ static char *pr_allow_names[] = {
> 	"allow.mount.zfs",
> 	"allow.mount.procfs",
> 	"allow.mount.tmpfs",
> +	"allow.kmem",
> };
> const size_t pr_allow_names_size = sizeof(pr_allow_names);
>
> @@ -224,6 +225,7 @@ static char *pr_allow_nonames[] = {
> 	"allow.mount.nozfs",
> 	"allow.mount.noprocfs",
> 	"allow.mount.notmpfs",
> +	"allow.nokmem",
> };
> const size_t pr_allow_nonames_size = sizeof(pr_allow_nonames);
>
> @@ -3951,6 +3953,27 @@ prison_priv_check(struct ucred *cred, in
> 		return (0);
>
> 		/*
> +		 * Allow access to /dev/io in a jail if the non-jailed admin
> +		 * requests this and if /dev/io exists in the jail. This
> +		 * allows Xorg to probe a card.
> +		 */
> +	case PRIV_IO:
> +		if (cred->cr_prison->pr_allow & PR_ALLOW_KMEM)
> +			return (0);
> +		else
> +			return (EPERM);
> +
> +		/*
> +		 * Allow low level access to KMEM-like devices (e.g. to
> +		 * allow Xorg to use DRI).
> +		 */
> +	case PRIV_KMEM_WRITE:
> +		if (cred->cr_prison->pr_allow & PR_ALLOW_KMEM)
> +			return (0);
> +		else
> +			return (EPERM);
> +
> +		/*
> 		 * Allow jailed root to set loginclass.
> 		 */
> 	case PRIV_PROC_SETLOGINCLASS:
> @@ -4384,6 +4407,8 @@ SYSCTL_JAIL_PARAM(_allow, quotas, CTLTYP
>     "B", "Jail may set file quotas");
> SYSCTL_JAIL_PARAM(_allow, socket_af, CTLTYPE_INT | CTLFLAG_RW,
>     "B", "Jail may create sockets other than just UNIX/IPv4/IPv6/route");
> +SYSCTL_JAIL_PARAM(_allow, kmem, CTLTYPE_INT | CTLFLAG_RW,
> +    "B", "Jail may access kmem-like devices (io, dri) if they exist");
>
> SYSCTL_JAIL_PARAM_SUBNODE(allow, mount, "Jail mount/unmount permission flags");
> SYSCTL_JAIL_PARAM(_allow_mount, , CTLTYPE_INT | CTLFLAG_RW,
>
> Modified: head/sys/sys/jail.h
> ==============================================================================
> --- head/sys/sys/jail.h	Wed Jan 29 13:35:12 2014	(r261265)
> +++ head/sys/sys/jail.h	Wed Jan 29 13:41:13 2014	(r261266)
> @@ -228,7 +228,8 @@ struct prison_racct {
> #define	PR_ALLOW_MOUNT_ZFS		0x0200
> #define	PR_ALLOW_MOUNT_PROCFS		0x0400
> #define	PR_ALLOW_MOUNT_TMPFS		0x0800
> -#define	PR_ALLOW_ALL			0x0fff
> +#define	PR_ALLOW_KMEM			0x1000
> +#define	PR_ALLOW_ALL			0x1fff
>
> /*
>  * OSD methods
>
> Modified: head/usr.sbin/jail/jail.8
> ==============================================================================
> --- head/usr.sbin/jail/jail.8	Wed Jan 29 13:35:12 2014	(r261265)
> +++ head/usr.sbin/jail/jail.8	Wed Jan 29 13:41:13 2014	(r261266)
> @@ -573,6 +573,17 @@ with non-jailed parts of the system.
> Sockets within a jail are normally restricted to IPv4, IPv6, local
> (UNIX), and route.  This allows access to other protocol stacks that
> have not had jail functionality added to them.
> +.It Va allow.kmem
> +Jailed processes may access
> +.Pa /dev/kmem
> +and similar devices (e.g. io, dri) if they have sufficient permission
> +(via the usual file permissions).
> +Note that the device files must exist within the jail for this parameter
> +to be of any use;
> +the default devfs ruleset for jails does not include any such devices.
> +Giving a jail access to kernel memory obviates much of the security that
> +jails offer, but can still be useful for other purposes.
> +For example, this would allow the Xorg server to run inside a jail.
> .El
> .El
> .Pp
>