[libvirt] [RFC PATCH] cgroup: allow fine-tuning of device ACL permissions
Eric Blake
eblake at redhat.com
Wed Mar 9 00:31:25 UTC 2011
Adding audit points showed that we were granting too much privilege
to qemu; it should not need any mknod rights to recreate any
devices other than ptys. On the other hand, lxc should have all
device privileges. The solution is adding a flag parameter.
* src/util/cgroup.h (virCgroup*Device*): Adjust prototypes.
* src/util/cgroup.c (virCgroupAllowDevice)
(virCgroupAllowDeviceMajor, virCgroupAllowDevicePath)
(virCgroupDenyDevice, virCgroupDenyDeviceMajor)
(virCgroupDenyDevicePath): Add parameter.
* src/qemu/qemu_cgroup.c: Update clients.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Likewise.
* src/lxc/lxc_controller.c (lxcSetContainerResources): Likewise.
---
https://bugzilla.redhat.com/show_bug.cgi?id=683163 was created
based on my audit patches.
RFC for two reasons:
This patch won't apply unless you have my other pending qemu audit
patches in place (I still need to post those, but ran out of time
for now...)
I need to do a followup patch to also restrict write access to
a disk explicitly marked read-only from qemu's point of view.
But I wanted to at least get this email posted to start feedback.
src/lxc/lxc_controller.c | 9 +++++--
src/qemu/qemu_cgroup.c | 23 +++++++++++-------
src/qemu/qemu_driver.c | 9 +++++--
src/util/cgroup.c | 55 +++++++++++++++++++++++++++++++++------------
src/util/cgroup.h | 26 ++++++++++++++++-----
5 files changed, 86 insertions(+), 36 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index d2b113c..296b302 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1,5 +1,6 @@
/*
- * Copyright (C) 2010 Red Hat, Inc. Copyright IBM Corp. 2008
+ * Copyright (C) 2010-2011 Red Hat, Inc.
+ * Copyright IBM Corp. 2008
*
* lxc_controller.c: linux container process controller
*
@@ -168,7 +169,8 @@ static int lxcSetContainerResources(virDomainDefPtr def)
rc = virCgroupAllowDevice(cgroup,
dev->type,
dev->major,
- dev->minor);
+ dev->minor,
+ VIR_CGROUP_DEVICE_RWM);
if (rc != 0) {
virReportSystemError(-rc,
_("Unable to allow device %c:%d:%d for domain %s"),
@@ -177,7 +179,8 @@ static int lxcSetContainerResources(virDomainDefPtr def)
}
}
- rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY);
+ rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY,
+ VIR_CGROUP_DEVICE_RWM);
if (rc != 0) {
virReportSystemError(-rc,
_("Unable to allow PYT devices for domain %s"),
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ebf9ad5..3c124fd 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -66,7 +66,8 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path %s for disk", path);
/* XXX RO vs RW */
- rc = virCgroupAllowDevicePath(data->cgroup, path);
+ rc = virCgroupAllowDevicePath(data->cgroup, path,
+ VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
@@ -106,8 +107,8 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
int rc;
VIR_DEBUG("Process path %s for disk", path);
- /* XXX RO vs RW */
- rc = virCgroupDenyDevicePath(data->cgroup, path);
+ rc = virCgroupDenyDevicePath(data->cgroup, path,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupPath(data->vm, data->cgroup, "deny", path, rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
@@ -150,7 +151,8 @@ qemuSetupChardevCgroup(virDomainDefPtr def,
VIR_DEBUG("Process path '%s' for disk", dev->source.data.file.path);
- rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path);
+ rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path,
+ VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(data->vm, data->cgroup, "allow",
dev->source.data.file.path, rc);
if (rc < 0) {
@@ -172,7 +174,8 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
int rc;
VIR_DEBUG("Process path '%s' for USB device", path);
- rc = virCgroupAllowDevicePath(data->cgroup, path);
+ rc = virCgroupAllowDevicePath(data->cgroup, path,
+ VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) {
virReportSystemError(-rc,
@@ -226,7 +229,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
goto cleanup;
}
- rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
+ rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
"pty", rc == 0);
if (rc != 0) {
@@ -240,7 +244,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
((vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
driver->vncAllowHostAudio) ||
(vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)))) {
- rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
+ rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR,
"sound", rc == 0);
if (rc != 0) {
@@ -251,8 +256,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
for (i = 0; deviceACL[i] != NULL ; i++) {
- rc = virCgroupAllowDevicePath(cgroup,
- deviceACL[i]);
+ rc = virCgroupAllowDevicePath(cgroup, deviceACL[i],
+ VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(vm, cgroup, "allow", deviceACL[i], rc);
if (rc < 0 &&
rc != -ENOENT) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1981cdf..7b4edc5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1962,7 +1962,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
vm->def->name);
goto endjob;
}
- rc = virCgroupAllowDevicePath(cgroup, path);
+ rc = virCgroupAllowDevicePath(cgroup, path,
+ VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(vm, cgroup, "allow", path, rc);
if (rc < 0) {
virReportSystemError(-rc,
@@ -2012,7 +2013,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
VIR_WARN("failed to restore save state label on %s", path);
if (cgroup != NULL) {
- rc = virCgroupDenyDevicePath(cgroup, path);
+ rc = virCgroupDenyDevicePath(cgroup, path,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s %d",
@@ -2044,7 +2046,8 @@ endjob:
}
if (cgroup != NULL) {
- rc = virCgroupDenyDevicePath(cgroup, path);
+ rc = virCgroupDenyDevicePath(cgroup, path,
+ VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s: %d",
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index a16b1ab..10e3143 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -1081,7 +1081,7 @@ int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb)
/**
* virCgroupDenyAllDevices:
*
- * @group: The cgroup to deny devices for
+ * @group: The cgroup to deny all permissions, for all devices
*
* Returns: 0 on success
*/
@@ -1100,15 +1100,20 @@ int virCgroupDenyAllDevices(virCgroupPtr group)
* @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device
* @minor: The minor number of the device
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to allow
*
* Returns: 0 on success
*/
-int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor)
+int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor,
+ int perms)
{
int rc;
char *devstr = NULL;
- if (virAsprintf(&devstr, "%c %i:%i rwm", type, major, minor) == -1) {
+ if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor,
+ perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
+ perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
+ perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) {
rc = -ENOMEM;
goto out;
}
@@ -1129,15 +1134,20 @@ out:
* @group: The cgroup to allow an entire device major type for
* @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device type
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to allow
*
* Returns: 0 on success
*/
-int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major)
+int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major,
+ int perms)
{
int rc;
char *devstr = NULL;
- if (virAsprintf(&devstr, "%c %i:* rwm", type, major) == -1) {
+ if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major,
+ perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
+ perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
+ perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) {
rc = -ENOMEM;
goto out;
}
@@ -1157,6 +1167,7 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major)
*
* @group: The cgroup to allow the device for
* @path: the device to allow
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to allow
*
* Queries the type of device and its major/minor number, and
* adds that to the cgroup ACL
@@ -1165,7 +1176,7 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major)
* negative errno value on failure
*/
#if defined(major) && defined(minor)
-int virCgroupAllowDevicePath(virCgroupPtr group, const char *path)
+int virCgroupAllowDevicePath(virCgroupPtr group, const char *path, int perms)
{
struct stat sb;
@@ -1178,11 +1189,13 @@ int virCgroupAllowDevicePath(virCgroupPtr group, const char *path)
return virCgroupAllowDevice(group,
S_ISCHR(sb.st_mode) ? 'c' : 'b',
major(sb.st_rdev),
- minor(sb.st_rdev));
+ minor(sb.st_rdev),
+ perms);
}
#else
int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED,
- const char *path ATTRIBUTE_UNUSED)
+ const char *path ATTRIBUTE_UNUSED,
+ int perms ATTRIBUTE_UNUSED)
{
return -ENOSYS;
}
@@ -1196,15 +1209,20 @@ int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED,
* @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device
* @minor: The minor number of the device
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to deny
*
* Returns: 0 on success
*/
-int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor)
+int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor,
+ int perms)
{
int rc;
char *devstr = NULL;
- if (virAsprintf(&devstr, "%c %i:%i rwm", type, major, minor) == -1) {
+ if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor,
+ perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
+ perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
+ perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) {
rc = -ENOMEM;
goto out;
}
@@ -1225,15 +1243,20 @@ out:
* @group: The cgroup to deny an entire device major type for
* @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device type
+ * @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to deny
*
* Returns: 0 on success
*/
-int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major)
+int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major,
+ int perms)
{
int rc;
char *devstr = NULL;
- if (virAsprintf(&devstr, "%c %i:* rwm", type, major) == -1) {
+ if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major,
+ perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
+ perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
+ perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) {
rc = -ENOMEM;
goto out;
}
@@ -1249,7 +1272,7 @@ int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major)
}
#if defined(major) && defined(minor)
-int virCgroupDenyDevicePath(virCgroupPtr group, const char *path)
+int virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int perms)
{
struct stat sb;
@@ -1262,11 +1285,13 @@ int virCgroupDenyDevicePath(virCgroupPtr group, const char *path)
return virCgroupDenyDevice(group,
S_ISCHR(sb.st_mode) ? 'c' : 'b',
major(sb.st_rdev),
- minor(sb.st_rdev));
+ minor(sb.st_rdev),
+ perms);
}
#else
int virCgroupDenyDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED,
- const char *path ATTRIBUTE_UNUSED)
+ const char *path ATTRIBUTE_UNUSED,
+ int perms ATTRIBUTE_UNUSED)
{
return -ENOSYS;
}
diff --git a/src/util/cgroup.h b/src/util/cgroup.h
index b3c5f27..16ffb46 100644
--- a/src/util/cgroup.h
+++ b/src/util/cgroup.h
@@ -60,27 +60,41 @@ int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb);
int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb);
int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb);
+enum {
+ VIR_CGROUP_DEVICE_READ = 1,
+ VIR_CGROUP_DEVICE_WRITE = 2,
+ VIR_CGROUP_DEVICE_MKNOD = 4,
+ VIR_CGROUP_DEVICE_RW = VIR_CGROUP_DEVICE_READ | VIR_CGROUP_DEVICE_WRITE,
+ VIR_CGROUP_DEVICE_RWM = VIR_CGROUP_DEVICE_RW | VIR_CGROUP_DEVICE_MKNOD,
+};
+
int virCgroupDenyAllDevices(virCgroupPtr group);
int virCgroupAllowDevice(virCgroupPtr group,
char type,
int major,
- int minor);
+ int minor,
+ int perms);
int virCgroupAllowDeviceMajor(virCgroupPtr group,
char type,
- int major);
+ int major,
+ int perms);
int virCgroupAllowDevicePath(virCgroupPtr group,
- const char *path);
+ const char *path,
+ int perms);
int virCgroupDenyDevice(virCgroupPtr group,
char type,
int major,
- int minor);
+ int minor,
+ int perms);
int virCgroupDenyDeviceMajor(virCgroupPtr group,
char type,
- int major);
+ int major,
+ int perms);
int virCgroupDenyDevicePath(virCgroupPtr group,
- const char *path);
+ const char *path,
+ int perms);
int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares);
int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares);
--
1.7.4
More information about the libvir-list
mailing list