On 01/11/2012 09:33 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list