Date: Sat, 18 May 2013 07:36:01 -0600 From: Jamie Gritton <jamie@FreeBSD.org> To: FreeBSD Current <freebsd-current@FreeBSD.org> Cc: Konstantin Belousov <kostikbel@gmail.com>, Alexander Leidinger <netchild@FreeBSD.org> Subject: Re: A PRIV_* flag for /dev/mem? Message-ID: <519783C1.2010601@FreeBSD.org> In-Reply-To: <20130518114357.GK3047@kib.kiev.ua> References: <5196818F.8080201@FreeBSD.org> <20130518114357.GK3047@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On 05/18/13 05:43, Konstantin Belousov wrote: > On Fri, May 17, 2013 at 01:14:23PM -0600, Jamie Gritton wrote: >> I'm considering Alexander Leidinger's patch to make X11 work inside a >> jail (http://leidinger.net/FreeBSD/current-patches/0_jail.diff). It >> allows a jail to optionally have access to /dev/io and DRI (provided the >> requisite device files are visible in the devfs ruleset). >> >> I'm planning on putting this under a single jail permission, which would >> group those two together as device access that allows messing with >> kernel memory. It seems more complete to put /dev/mem under that same >> umbrella, with the side benefit of letting me call it "allow.dev_mem". >> >> Currently, access is controlled only by device file permission and a >> securelevel check. Jail access is allowed as long as the /dev/mem is in >> the jail's ruleset (it isn't by default). Adding a prison_priv_check() >> call would allow some finer control over this. Something like: >> >> int >> memopen(struct cdev *dev __unused, int flags, int fmt __unused, >> struct thread *td) >> { >> int error; >> >> error = priv_check(td, PRIV_FOO); >> if (error != 0&& (flags& FWRITE)) >> error = securelevel_gt(td->td_ucred, 0); >> >> return (error); >> } >> >> The main question I'm coming up with here is, what PRIV_* flag should I >> use. Does PRIV_IO make sense? PRIV_DRIVER? Something new like >> PRIV_KMEM? Also, I'd appreciate if anyone familiar with this interface >> can tell me if memopen() is the right/only place to make this change. > > Why do we need the PRIV check there at all, esp. for DRM ? > Why the devfs rulesets are not enough ? At least for the reason Alexander's patch was first made, X11 working inside a jail, there's already a need to get past PRIV_IO and PRIV_DRIVER - those checks are already made so in that case the presence of device files isn't sufficient. His solution was to special-case PRIV_DRIVER for drm, and then add jail permission bits that allowed PRIV_IO and PRIV_DRI_DRIVER. A largish but apparently arbitrary set of of devices use PRIV_DRIVER, so it makes sense to separate out this one that's necessary. So while there may be a question as to why /dev/io and DRM should have PRIV checks, the fact of the matter is they already do. Now as to the change I'm considering: kmem. Since the main danger of the existing checks (io and drm) is that they can allow you to stomp on kernel memory, I thought it reasonable to combine them into a single jail flag that allowed that behavior. In coming up with a succinct name for it, I decided on allow.dev_mem (permission for devices that work with system memory), and that brought up the question for /dev/mem. No, I don't need to add a priv check to it; but it seems that if such checks as PRIV_IO and PRIV_DRIVER exist for devices already, then an architectural decision has already been made that device file access isn't the only level of control we'd like to have. Really I'm surprised something as potentially damaging as kmem didn't already have a priv_check associated with it. Now I could certainly add his patch with no changes (or with very few), and just put in a jail flag that's X11-specific. The /dev/mem change isn't necessary to this, but it just seemed a good time to add something that feels like a hole in the paradigm. Alexander: I've sort of put some words in your mouth, so feel free to correct anything. - Jamie
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?519783C1.2010601>