[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 6/7] Use cgroups for block device whitelisting in QEMU guests



On Fri, Jul 17, 2009 at 05:15:15PM +0200, Daniel Veillard wrote:
> > +
> > +int virCgroupDenyDevicePath(virCgroupPtr group,
> > +                             const char *path)
> > +{
> > +    struct stat sb;
> > +
> > +    if (stat(path, &sb) < 0)
> > +        return -errno;
> > +
> > +    if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode))
> > +        return -EINVAL;
> > +
> > +    return virCgroupDenyDevice(group,
> > +                               S_ISCHR(sb.st_mode) ? 'c' : 'b',
> > +                               major(sb.st_rdev),
> > +                               minor(sb.st_rdev));
> > +}
> [...]
> > diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> > index f6d3f52..33e9cfa 100644
> > --- a/src/qemu_driver.c
> > +++ b/src/qemu_driver.c
> > @@ -1378,12 +1378,19 @@ error:
> >      return -1;
> >  }
> >  
> > +static const char *const devs[] = {
> > +    "/dev/null", "/dev/full", "/dev/zero",
> > +    "/dev/random", "/dev/urandom",
> > +    "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
> > +};
> 
>   Hum, that list sounds a bit arbitrary, this could break for random
> reasons  maybe this should be extended through the configuration, I
> assume a mismatch may result in domain failing to start or operate
> properly, right ?

Yes, QEMU will get -EPERM if it attempts to access a block device
we've not permitted, and hopefully exit.

Looking at it again, I've missed a couple of devices I should have
allowed. I'll check the QEMU source to match up the list properly

> [...]
> > +
> > +    rc = virCgroupAllowDeviceMajor(cgroup, 'c', 136);
> 
>   Hum, that's a magic number, can we get a meaningful #define
> 
> 
>    The idea sounds good but I'm a bit afraid of the inflexibility,
> this has the potential of making qemu/kvm far more fragile without
> a way to fix this by patching and recompiling.

Perhaps we should make it a config in /etc/libvirt/qemu.conf too

>    Again I'm not a cgroup expert but I feel a bit uneasy, can we get
> at least an option to disable it at runtime in the configuration (sorry
> if I missed that !) ?

Well the simplest way to disable it at runtime, is to simply not
mount the cgroups device ACL controller on the host. If this is
not mounted, then libvirt will just skip this functionality.

NB, no cgroups stuff is enabled by default on any current distro I'm
aware of. Arguably this should be changed, but that's not libvirt's
problem


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]