[libvirt] [PATCH 5/6] Cleanup sec driver error reporting to use virReportSystemError
Daniel P. Berrange
berrange at redhat.com
Tue Sep 1 15:28:58 UTC 2009
* src/security_selinux.c: Use virReportSystemError whereever an
errno is involved
* src/qemu_driver.c: Don't overwrite error message from the
security driver
---
src/qemu_driver.c | 13 +++--
src/security_selinux.c | 121 ++++++++++++++++++++++++++++--------------------
2 files changed, 77 insertions(+), 57 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index d75e28e..fad518c 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1695,6 +1695,8 @@ static int qemuDomainSetHostdevUSBOwnershipActor(virConnectPtr conn,
{
struct qemuFileOwner *owner = opaque;
+ VIR_DEBUG("Setting ownership on %s to %d:%d", file, owner->uid, owner->gid);
+
if (chown(file, owner->uid, owner->gid) < 0) {
virReportSystemError(conn, errno, _("cannot set ownership on %s"), file);
return -1;
@@ -1736,6 +1738,8 @@ static int qemuDomainSetHostdevPCIOwnershipActor(virConnectPtr conn,
{
struct qemuFileOwner *owner = opaque;
+ VIR_DEBUG("Setting ownership on %s to %d:%d", file, owner->uid, owner->gid);
+
if (chown(file, owner->uid, owner->gid) < 0) {
virReportSystemError(conn, errno, _("cannot set ownership on %s"), file);
return -1;
@@ -1900,18 +1904,15 @@ static int qemudSecurityHook(void *data) {
if (qemuAddToCgroup(h->driver, h->vm->def) < 0)
return -1;
- if (qemudDomainSetSecurityLabel(h->conn, h->driver, h->vm) < 0) {
- qemudReportError(h->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- "%s", _("Failed to set security label"));
+ if (qemudDomainSetSecurityLabel(h->conn, h->driver, h->vm) < 0)
return -1;
- }
if (h->driver->privileged) {
- DEBUG("Dropping privileges of VM to %d:%d", h->driver->user, h->driver->group);
-
if (qemuDomainSetAllDeviceOwnership(h->conn, h->driver, h->vm->def, 0) < 0)
return -1;
+ DEBUG("Dropping privileges of VM to %d:%d", h->driver->user, h->driver->group);
+
if (h->driver->group) {
if (setregid(h->driver->group, h->driver->group) < 0) {
virReportSystemError(NULL, errno,
diff --git a/src/security_selinux.c b/src/security_selinux.c
index 5b7b038..bc295b1 100644
--- a/src/security_selinux.c
+++ b/src/security_selinux.c
@@ -106,24 +106,21 @@ SELinuxInitialize(virConnectPtr conn)
{
char *ptr = NULL;
int fd = 0;
- char ebuf[1024];
virRandomInitialize(time(NULL) ^ getpid());
fd = open(selinux_virtual_domain_context_path(), O_RDONLY);
if (fd < 0) {
- virSecurityReportError(conn, VIR_ERR_ERROR,
- _("%s: cannot open SELinux virtual domain context file %s: %s"),
- __func__,selinux_virtual_domain_context_path(),
- virStrerror(errno, ebuf, sizeof ebuf));
+ virReportSystemError(conn, errno,
+ _("cannot open SELinux virtual domain context file '%s'"),
+ selinux_virtual_domain_context_path());
return -1;
}
if (saferead(fd, default_domain_context, sizeof(default_domain_context)) < 0) {
- virSecurityReportError(conn, VIR_ERR_ERROR,
- _("%s: cannot read SELinux virtual domain context file %s: %s"),
- __func__,selinux_virtual_domain_context_path(),
- virStrerror(errno, ebuf, sizeof ebuf));
+ virReportSystemError(conn, errno,
+ _("cannot read SELinux virtual domain context file %s"),
+ selinux_virtual_domain_context_path());
close(fd);
return -1;
}
@@ -133,18 +130,16 @@ SELinuxInitialize(virConnectPtr conn)
*ptr = '\0';
if ((fd = open(selinux_virtual_image_context_path(), O_RDONLY)) < 0) {
- virSecurityReportError(conn, VIR_ERR_ERROR,
- _("%s: cannot open SELinux virtual image context file %s: %s"),
- __func__,selinux_virtual_image_context_path(),
- virStrerror(errno, ebuf, sizeof ebuf));
+ virReportSystemError(conn, errno,
+ _("cannot open SELinux virtual image context file %s"),
+ selinux_virtual_image_context_path());
return -1;
}
if (saferead(fd, default_image_context, sizeof(default_image_context)) < 0) {
- virSecurityReportError(conn, VIR_ERR_ERROR,
- _("%s: cannot read SELinux virtual image context file %s: %s"),
- __func__,selinux_virtual_image_context_path(),
- virStrerror(errno, ebuf, sizeof ebuf));
+ virReportSystemError(conn, errno,
+ _("cannot read SELinux virtual image context file %s"),
+ selinux_virtual_image_context_path());
close(fd);
return -1;
}
@@ -232,10 +227,8 @@ SELinuxReserveSecurityLabel(virConnectPtr conn,
const char *mcs;
if (getpidcon(vm->pid, &pctx) == -1) {
- char ebuf[1024];
- virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: error calling "
- "getpidcon(): %s"), __func__,
- virStrerror(errno, ebuf, sizeof ebuf));
+ virReportSystemError(conn, errno,
+ _("unable to get PID %d security context"), vm->pid);
return -1;
}
@@ -286,17 +279,16 @@ SELinuxGetSecurityLabel(virConnectPtr conn,
security_context_t ctx;
if (getpidcon(vm->pid, &ctx) == -1) {
- char ebuf[1024];
- virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: error calling "
- "getpidcon(): %s"), __func__,
- virStrerror(errno, ebuf, sizeof ebuf));
+ virReportSystemError(conn, errno,
+ _("unable to get PID %d security context"),
+ vm->pid);
return -1;
}
if (strlen((char *) ctx) >= VIR_SECURITY_LABEL_BUFLEN) {
virSecurityReportError(conn, VIR_ERR_ERROR,
- _("%s: security label exceeds "
- "maximum lenth: %d"), __func__,
+ _("security label exceeds "
+ "maximum lenth: %d"),
VIR_SECURITY_LABEL_BUFLEN - 1);
return -1;
}
@@ -306,10 +298,8 @@ SELinuxGetSecurityLabel(virConnectPtr conn,
sec->enforcing = security_getenforce();
if (sec->enforcing == -1) {
- char ebuf[1024];
- virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: error calling "
- "security_getenforce(): %s"), __func__,
- virStrerror(errno, ebuf, sizeof ebuf));
+ virReportSystemError(conn, errno, "%s",
+ _("error calling security_getenforce()"));
return -1;
}
@@ -319,7 +309,6 @@ SELinuxGetSecurityLabel(virConnectPtr conn,
static int
SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon)
{
- char ebuf[1024];
security_context_t econ;
VIR_INFO("Setting SELinux context on '%s' to '%s'", path, tcon);
@@ -343,14 +332,14 @@ SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon)
* virt_use_{nfs,usb,pci} boolean tunables to allow it...
*/
if (setfilecon_errno != EOPNOTSUPP) {
- virSecurityReportError(conn, VIR_ERR_ERROR,
- _("%s: unable to set security context "
- "'\%s\' on %s: %s."), __func__,
- tcon,
- path,
- virStrerror(errno, ebuf, sizeof ebuf));
+ virReportSystemError(conn, setfilecon_errno,
+ _("unable to set security context '%s' on '%s'"),
+ tcon, path);
if (security_getenforce() == 1)
return -1;
+ } else {
+ VIR_INFO("Setting security context '%s' on '%s' not supported",
+ tcon, path);
}
}
return 0;
@@ -366,6 +355,8 @@ SELinuxRestoreSecurityFileLabel(virConnectPtr conn,
int err;
char *newpath = NULL;
+ VIR_INFO("Restoring SELinux context on '%s'", path);
+
if ((err = virFileResolveLink(path, &newpath)) < 0) {
virReportSystemError(conn, err,
_("cannot resolve symlink %s"), path);
@@ -440,6 +431,17 @@ SELinuxSetSecurityPCILabel(virConnectPtr conn,
}
static int
+SELinuxSetSecurityUSBLabel(virConnectPtr conn,
+ usbDevice *dev ATTRIBUTE_UNUSED,
+ const char *file, void *opaque)
+{
+ virDomainObjPtr vm = opaque;
+ const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
+
+ return SELinuxSetFilecon(conn, file, secdef->imagelabel);
+}
+
+static int
SELinuxSetSecurityHostdevLabel(virConnectPtr conn,
virDomainObjPtr vm,
virDomainHostdevDefPtr dev)
@@ -451,8 +453,24 @@ SELinuxSetSecurityHostdevLabel(virConnectPtr conn,
return 0;
switch (dev->source.subsys.type) {
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
- break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
+ if (dev->source.subsys.u.usb.bus && dev->source.subsys.u.usb.device) {
+ usbDevice *usb = usbGetDevice(conn,
+ dev->source.subsys.u.usb.bus,
+ dev->source.subsys.u.usb.device);
+
+ if (!usb)
+ goto done;
+
+ ret = usbDeviceFileIterate(conn, usb, SELinuxSetSecurityUSBLabel, vm);
+ usbFreeDevice(conn, usb);
+
+ break;
+ } else {
+ /* XXX deal with product/vendor better */
+ ret = 0;
+ }
+ }
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
pciDevice *pci = pciGetDevice(conn,
@@ -554,6 +572,9 @@ SELinuxRestoreSecurityLabel(virConnectPtr conn,
const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
int i;
int rc = 0;
+
+ VIR_DEBUG("Restoring security label on %s", vm->def->name);
+
if (secdef->imagelabel) {
for (i = 0 ; i < vm->def->nhostdevs ; i++) {
if (SELinuxRestoreSecurityHostdevLabel(conn, vm->def->hostdevs[i]) < 0)
@@ -597,25 +618,23 @@ SELinuxSetSecurityLabel(virConnectPtr conn,
/* TODO: verify DOI */
const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
int i;
- char ebuf[1024];
if (!STREQ(drv->name, secdef->model)) {
virSecurityReportError(conn, VIR_ERR_ERROR,
- _("%s: security label driver mismatch: "
- "\'%s\' model configured for domain, but "
- "hypervisor driver is \'%s\'."),
- __func__, secdef->model, drv->name);
+ _("security label driver mismatch: "
+ "'%s' model configured for domain, but "
+ "hypervisor driver is '%s'."),
+ secdef->model, drv->name);
if (security_getenforce() == 1)
- return -1;
+ return -1;
}
if (setexeccon(secdef->label) == -1) {
- virSecurityReportError(conn, VIR_ERR_ERROR,
- _("%s: unable to set security context "
- "'\%s\': %s."), __func__, secdef->label,
- virStrerror(errno, ebuf, sizeof ebuf));
+ virReportSystemError(conn, errno,
+ _("unable to set security context '%s'"),
+ secdef->label);
if (security_getenforce() == 1)
- return -1;
+ return -1;
}
if (secdef->imagelabel) {
--
1.6.2.5
More information about the libvir-list
mailing list