Re: [libvirt] [PATCH 7/7] Set a security context on /dev and /dev/pts mounts
On 01/11/2012 09:33 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
To allow the container to access /dev and /dev/pts when under
sVirt, set an explicit mount option. Also set a max size on
the /dev mount to prevent DOS on memory usage
* src/lxc/lxc_container.c: Set /dev mount context
* src/lxc/lxc_controller.c: Set /dev/pts mount context
---
src/lxc/lxc_container.c | 75 +++++++++++++++++++++++++++++++++++----------
src/lxc/lxc_controller.c | 43 +++++++++++++++++++++++---
2 files changed, 96 insertions(+), 22 deletions(-)
+ } else {
+#endif
+ /*
+ * tmpfs is limited to 64kb, since we only have device nodes in there
+ * and don't want to DOS the entire OS RAM usage
+ */
+ if (virAsprintf(&opts, "mode=755,size=65536%%%s%s%s",
Ouch. size=65536% is _not_ what you want; you either want size=65536 or
something like size=10%.
+ con ? ",context=\"" :
"",
+ con ? (const char *)con : "",
+ con ? "\"" : "") < 0) {
I would have split this:
if (virAsprintf(&opts, "mode=755,size=65536") < 0 ||
(con && virAsprintf(&opts, ",context=\"%s\"",
(const char *)con) < 0)) {
+ virReportOOMError();
+ goto cleanup;
+ }
+#if HAVE_SELINUX
+ }
+#endif
You don't need this second #if. That is, instead of writing:
#if HAVE_SELINUX
if (condition) {
goto cleanup;
} else {
#endif
stuff;
#if HAVE_SELINUX
}
#endif
I would have done:
#if HAVE_SELINUX
if (condition) {
goto cleanup;
}
#endif
stuff;
@@ -1373,16 +1380,42 @@ lxcControllerRun(virDomainDefPtr def,
goto cleanup;
}
- /* XXX should we support gid=X for X!=5 for distros which use
- * a different gid for tty? */
- VIR_DEBUG("Mounting 'devpts' on %s", devpts);
- if (mount("devpts", devpts, "devpts", 0,
- "newinstance,ptmxmode=0666,mode=0620,gid=5") < 0) {
+#if HAVE_SELINUX
+ if (getfilecon(root->src, &con) < 0 &&
+ errno != ENOTSUP) {
+ virReportSystemError(errno,
+ _("Failed to query file context on %s"),
+ root->src);
+ goto cleanup;
+ } else {
+#endif
+ /*
+ * tmpfs is limited to 64kb, since we only have device nodes in there
+ * and don't want to DOS the entire OS RAM usage
+ */
Is this comment really relative to the devpts mount point?
> + /* XXX should we support gid=X for X!=5 for distros which use
> + * a different gid for tty? */
> + if (virAsprintf(&opts,
"newinstance,ptmxmode=0666,mode=0620,gid=5%s%s%s",
+ con ? ",context=\"" :
"",
+ con ? (const char *)con : "",
+ con ? "\"" : "") < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+#if HAVE_SELINUX
+ }
+#endif
Same formatting nit about not needing a second #if.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)