From owner-freebsd-current@FreeBSD.ORG Sat May 25 20:07:28 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 3518191A; Sat, 25 May 2013 20:07:28 +0000 (UTC) (envelope-from alexander@leidinger.net) Received: from mail.ebusiness-leidinger.de (mail.ebusiness-leidinger.de [217.11.53.44]) by mx1.freebsd.org (Postfix) with ESMTP id 905E5307; Sat, 25 May 2013 20:07:26 +0000 (UTC) Received: from outgoing.leidinger.net (p5DD45863.dip0.t-ipconnect.de [93.212.88.99]) by mail.ebusiness-leidinger.de (Postfix) with ESMTPSA id 934A38440CD; Sat, 25 May 2013 22:07:12 +0200 (CEST) Received: from unknown (Titan.Leidinger.net [192.168.1.17]) by outgoing.leidinger.net (Postfix) with ESMTP id EC695147D; Sat, 25 May 2013 22:07:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=leidinger.net; s=outgoing-alex; t=1369512430; bh=0WXIve6q6xQqiGurxfaX4PGfypZ6bO8MaNAdluj2UYw=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=tWrXV+f5ZP3rOQ3m3w+qXmfX7Mebb0VYxELX/ZOokje9rwsjOOVdKCLp/GtLniQ4C Noo2py1zKMU+GS+mE1Io9YQ3Q2b7FgPOq62sqpK5++PTzFXUeHZJBkS5vx9H+9R5pB g64PXn7/440NajHEuYGl1T1LalsDvUQBgWKycgEX3xOhf9YPQGj2UTJog+mECrvjMv MWftfGjS4ogrVCtobyC1BTngo/E061T7jdtINYY8yDGZSVAzTqrZ7BgG/9NVEqubpw FLOnWzJbCzx0xIkJVbRIZrrX9mPqtHBWquhnDOIsPTmKG/uGM3ngC1bF1EH0F8PYNQ fF6pNTnKucWHQ== Date: Sat, 25 May 2013 22:07:10 +0200 From: Alexander Leidinger To: Jamie Gritton Subject: Re: A PRIV_* flag for /dev/mem? Message-ID: <20130525220710.000041b5@unknown> In-Reply-To: <519783C1.2010601@FreeBSD.org> References: <5196818F.8080201@FreeBSD.org> <20130518114357.GK3047@kib.kiev.ua> <519783C1.2010601@FreeBSD.org> X-Mailer: Claws Mail 3.9.1-2-g66aa06 (GTK+ 2.16.6; i586-pc-mingw32msvc) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-EBL-MailScanner-Information: Please contact the ISP for more information X-EBL-MailScanner-ID: 934A38440CD.A15A6 X-EBL-MailScanner: Found to be clean X-EBL-MailScanner-SpamCheck: not spam, spamhaus-ZEN, SpamAssassin (not cached, score=-1.187, required 6, autolearn=disabled, ALL_TRUSTED -1.00, AWL -0.15, DKIM_SIGNED 0.10, DKIM_VALID -0.10, DKIM_VALID_AU -0.10, TW_EV 0.08, T_RP_MATCHES_RCVD -0.01, URIBL_BLOCKED 0.00) X-EBL-MailScanner-From: alexander@leidinger.net X-EBL-MailScanner-Watermark: 1370117233.14226@lLbjdF+i2oFNaIyggGSxTQ X-EBL-Spam-Status: No Cc: Konstantin Belousov , FreeBSD Current X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 May 2013 20:07:28 -0000 On Sat, 18 May 2013 07:36:01 -0600 Jamie Gritton wrote: > 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 Info: I spoke with the author of the dri1 driver loooong ago, and it was OK for him if I would change the PRIV_DRIVER in DRI to something else. > 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 I wouldn't be happy if my patch is committed as is. Your suggestion sounds much better. I would suggest "allow.kmem" or "allow.kmem_devs". The reason is that "dev_mem" could be seen as "/dev/mem" only. > change isn't necessary to this, but it just seemed a good time to add > something that feels like a hole in the paradigm. Bye, Alexander. -- http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID = B0063FE7 http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID = 72077137