[libvirt] [PATCHv2 0/5] audit: add some more audit hooks

I'm following up on danpb's patches to add initial audit support to qemu actions (see around commit 8dc136b in Oct 2010). This series adds the following additional audit points: All changes to the device ACL whitelist via the cgroup device controller All changes to memory balloon and vcpu sizes All changes to pci and usb device passthrough Here's an example audit, using audit-2.0.6-1.el6.x86_64 from RHEL, where I hot-unplugged a PCI device from a guest: type=VIRT_RESOURCE msg=audit(1298504227.432:914): user pid=13400 uid=0 auid=500 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=dev reason=detach vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 type=pci device="0000:0a:0a.0": exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=? terminal=pts/0 res=success' And one where I reduced memory via ballooning: type=VIRT_RESOURCE msg=audit(1298505060.916:927): user pid=13400 uid=0 auid=500 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=mem reason=update vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 old-mem=786432 new-mem=524288: exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=? terminal=pts/0 res=success' Changes since v1[1]: Rebased (assumes that Jirka's patch series[2] to clean up qemuCmdFlags will go in first, otherwise you will get minor conflicts when applying) Added some patches Reworked the cgroup ACL patches to avoid spamming the audit log when visiting a regular file instead of a device [1] https://www.redhat.com/archives/libvir-list/2011-February/msg00565.html [2] https://www.redhat.com/archives/libvir-list/2011-February/msg00985.html Eric Blake (5): cgroup: determine when skipping non-devices audit: prepare qemu for listing vm in cgroup audits audit: add qemu hooks for auditing cgroup events audit: audit qemu memory and vcpu adjusments audit: audit qemu pci and usb device passthrough src/qemu/qemu_audit.c | 178 ++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_audit.h | 23 ++++++- src/qemu/qemu_cgroup.c | 95 +++++++++++++++---------- src/qemu/qemu_cgroup.h | 21 +++--- src/qemu/qemu_driver.c | 28 +++++-- src/qemu/qemu_hotplug.c | 35 ++++----- src/util/cgroup.c | 7 +- 7 files changed, 305 insertions(+), 82 deletions(-) -- 1.7.4

* src/util/cgroup.c (virCgroupAllowDevicePath) (virCgroupDenyDevicePath): Don't fail with EINVAL for non-devices. * src/qemu/qemu_driver.c (qemudDomainSaveFlag): Update caller. * src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow) (qemuSetupChardevCgroup, qemuSetupHostUsbDeviceCgroup) (qemuSetupCgroup, qemuTeardownDiskPathDeny): Likewise. --- v2: new patch, reduces audit log clutter in later patches src/qemu/qemu_cgroup.c | 18 ++++++------------ src/qemu/qemu_driver.c | 6 +++--- src/util/cgroup.c | 7 ++++--- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 8cd6ce9..3907a09 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -66,11 +66,8 @@ int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, VIR_DEBUG("Process path %s for disk", path); /* XXX RO vs RW */ rc = virCgroupAllowDevicePath(cgroup, path); - if (rc != 0) { - /* Get this for non-block devices */ - if (rc == -EINVAL) { - VIR_DEBUG("Ignoring EINVAL for %s", path); - } else if (rc == -EACCES) { /* Get this for root squash NFS */ + if (rc < 0) { + if (rc == -EACCES) { /* Get this for root squash NFS */ VIR_DEBUG("Ignoring EACCES for %s", path); } else { virReportSystemError(-rc, @@ -106,11 +103,8 @@ int qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, VIR_DEBUG("Process path %s for disk", path); /* XXX RO vs RW */ rc = virCgroupDenyDevicePath(cgroup, path); - if (rc != 0) { - /* Get this for non-block devices */ - if (rc == -EINVAL) { - VIR_DEBUG("Ignoring EINVAL for %s", path); - } else if (rc == -EACCES) { /* Get this for root squash NFS */ + if (rc < 0) { + if (rc == -EACCES) { /* Get this for root squash NFS */ VIR_DEBUG("Ignoring EACCES for %s", path); } else { virReportSystemError(-rc, @@ -148,7 +142,7 @@ int qemuSetupChardevCgroup(virDomainDefPtr def, VIR_DEBUG("Process path '%s' for disk", dev->source.data.file.path); rc = virCgroupAllowDevicePath(cgroup, dev->source.data.file.path); - if (rc != 0) { + if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), dev->source.data.file.path, def->name); @@ -168,7 +162,7 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED, VIR_DEBUG("Process path '%s' for USB device", path); rc = virCgroupAllowDevicePath(cgroup, path); - if (rc != 0) { + if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s"), path); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c58c20e..15b9bc0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1963,7 +1963,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, goto endjob; } rc = virCgroupAllowDevicePath(cgroup, path); - if (rc != 0) { + if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), path, vm->def->name); @@ -2012,7 +2012,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, if (cgroup != NULL) { rc = virCgroupDenyDevicePath(cgroup, path); - if (rc != 0) + if (rc < 0) VIR_WARN("Unable to deny device %s for %s %d", path, vm->def->name, rc); } @@ -2043,7 +2043,7 @@ endjob: if (cgroup != NULL) { rc = virCgroupDenyDevicePath(cgroup, path); - if (rc != 0) + if (rc < 0) VIR_WARN("Unable to deny device %s for %s: %d", path, vm->def->name, rc); } diff --git a/src/util/cgroup.c b/src/util/cgroup.c index b71eef9..00c8828 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -1147,7 +1147,8 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major) * Queries the type of device and its major/minor number, and * adds that to the cgroup ACL * - * Returns: 0 on success + * Returns: 0 on success, 1 if path exists but is not a device, or + * negative errno value on failure */ #if defined(major) && defined(minor) int virCgroupAllowDevicePath(virCgroupPtr group, const char *path) @@ -1158,7 +1159,7 @@ int virCgroupAllowDevicePath(virCgroupPtr group, const char *path) return -errno; if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode)) - return -EINVAL; + return 1; return virCgroupAllowDevice(group, S_ISCHR(sb.st_mode) ? 'c' : 'b', @@ -1242,7 +1243,7 @@ int virCgroupDenyDevicePath(virCgroupPtr group, const char *path) return -errno; if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode)) - return -EINVAL; + return 1; return virCgroupDenyDevice(group, S_ISCHR(sb.st_mode) ? 'c' : 'b', -- 1.7.4

On Wed, Feb 23, 2011 at 05:02:59PM -0700, Eric Blake wrote:
* src/util/cgroup.c (virCgroupAllowDevicePath) (virCgroupDenyDevicePath): Don't fail with EINVAL for non-devices. * src/qemu/qemu_driver.c (qemudDomainSaveFlag): Update caller. * src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow) (qemuSetupChardevCgroup, qemuSetupHostUsbDeviceCgroup) (qemuSetupCgroup, qemuTeardownDiskPathDeny): Likewise. ---
v2: new patch, reduces audit log clutter in later patches
src/qemu/qemu_cgroup.c | 18 ++++++------------ src/qemu/qemu_driver.c | 6 +++--- src/util/cgroup.c | 7 ++++--- 3 files changed, 13 insertions(+), 18 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

* src/qemu/qemu_cgroup.h (struct qemuCgroupData): New helper type. (qemuSetupDiskPathAllow, qemuSetupChardevCgroup) (qemuTeardownDiskPathDeny): Drop unneeded prototypes. (qemuSetupDiskCgroup, qemuTeardownDiskCgroup): Adjust prototype. * src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow, qemuSetupChardevCgroup) (qemuTeardownDiskPathDeny): Mark static and use new type. (qemuSetupHostUsbDeviceCgroup): Use new type. (qemuSetupDiskCgroup): Alter signature. (qemuSetupCgroup): Adjust caller. * src/qemu/qemu_hotplug.c (qemuDomainAttachHostUsbDevice) (qemuDomainDetachPciDiskDevice, qemuDomainDetachSCSIDiskDevice): Likewise. * src/qemu/qemu_driver.c (qemudDomainAttachDevice) (qemuDomainUpdateDeviceFlags): Likewise. --- v2: rebase, no functional change src/qemu/qemu_cgroup.c | 58 ++++++++++++++++++++++++++-------------------- src/qemu/qemu_cgroup.h | 21 +++++++--------- src/qemu/qemu_driver.c | 8 +++--- src/qemu/qemu_hotplug.c | 7 +++-- 4 files changed, 50 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3907a09..49ec473 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -54,18 +54,18 @@ int qemuCgroupControllerActive(struct qemud_driver *driver, return 0; } - -int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, - const char *path, - size_t depth ATTRIBUTE_UNUSED, - void *opaque) +static int +qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, + const char *path, + size_t depth ATTRIBUTE_UNUSED, + void *opaque) { - virCgroupPtr cgroup = opaque; + qemuCgroupData *data = opaque; int rc; VIR_DEBUG("Process path %s for disk", path); /* XXX RO vs RW */ - rc = virCgroupAllowDevicePath(cgroup, path); + rc = virCgroupAllowDevicePath(data->cgroup, path); if (rc < 0) { if (rc == -EACCES) { /* Get this for root squash NFS */ VIR_DEBUG("Ignoring EACCES for %s", path); @@ -81,28 +81,31 @@ int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, int qemuSetupDiskCgroup(struct qemud_driver *driver, + virDomainObjPtr vm, virCgroupPtr cgroup, virDomainDiskDefPtr disk) { + qemuCgroupData data = { vm, cgroup }; return virDomainDiskDefForeachPath(disk, driver->allowDiskFormatProbing, true, qemuSetupDiskPathAllow, - cgroup); + &data); } -int qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, - const char *path, - size_t depth ATTRIBUTE_UNUSED, - void *opaque) +static int +qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, + const char *path, + size_t depth ATTRIBUTE_UNUSED, + void *opaque) { - virCgroupPtr cgroup = opaque; + qemuCgroupData *data = opaque; int rc; VIR_DEBUG("Process path %s for disk", path); /* XXX RO vs RW */ - rc = virCgroupDenyDevicePath(cgroup, path); + rc = virCgroupDenyDevicePath(data->cgroup, path); if (rc < 0) { if (rc == -EACCES) { /* Get this for root squash NFS */ VIR_DEBUG("Ignoring EACCES for %s", path); @@ -118,22 +121,25 @@ int qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, int qemuTeardownDiskCgroup(struct qemud_driver *driver, + virDomainObjPtr vm, virCgroupPtr cgroup, virDomainDiskDefPtr disk) { + qemuCgroupData data = { vm, cgroup }; return virDomainDiskDefForeachPath(disk, driver->allowDiskFormatProbing, true, qemuTeardownDiskPathDeny, - cgroup); + &data); } -int qemuSetupChardevCgroup(virDomainDefPtr def, - virDomainChrDefPtr dev, - void *opaque) +static int +qemuSetupChardevCgroup(virDomainDefPtr def, + virDomainChrDefPtr dev, + void *opaque) { - virCgroupPtr cgroup = opaque; + qemuCgroupData *data = opaque; int rc; if (dev->source.type != VIR_DOMAIN_CHR_TYPE_DEV) @@ -141,7 +147,7 @@ int qemuSetupChardevCgroup(virDomainDefPtr def, VIR_DEBUG("Process path '%s' for disk", dev->source.data.file.path); - rc = virCgroupAllowDevicePath(cgroup, dev->source.data.file.path); + rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), @@ -157,11 +163,11 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED, const char *path, void *opaque) { - virCgroupPtr cgroup = opaque; + qemuCgroupData *data = opaque; int rc; VIR_DEBUG("Process path '%s' for USB device", path); - rc = virCgroupAllowDevicePath(cgroup, path); + rc = virCgroupAllowDevicePath(data->cgroup, path); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s"), @@ -195,6 +201,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, } if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + qemuCgroupData data = { vm, cgroup }; rc = virCgroupDenyAllDevices(cgroup); if (rc != 0) { if (rc == -EPERM) { @@ -208,7 +215,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, } for (i = 0; i < vm->def->ndisks ; i++) { - if (qemuSetupDiskCgroup(driver, cgroup, vm->def->disks[i]) < 0) + if (qemuSetupDiskCgroup(driver, vm, cgroup, vm->def->disks[i]) < 0) goto cleanup; } @@ -243,7 +250,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, if (virDomainChrDefForeach(vm->def, true, qemuSetupChardevCgroup, - cgroup) < 0) + &data) < 0) goto cleanup; for (i = 0; i < vm->def->nhostdevs; i++) { @@ -259,7 +266,8 @@ int qemuSetupCgroup(struct qemud_driver *driver, hostdev->source.subsys.u.usb.device)) == NULL) goto cleanup; - if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, cgroup) < 0 ) + if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, + &data) < 0) goto cleanup; } } diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 0a9692b..299bd2d 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -1,7 +1,7 @@ /* * qemu_cgroup.h: QEMU cgroup management * - * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -28,25 +28,22 @@ # include "domain_conf.h" # include "qemu_conf.h" +struct _qemuCgroupData { + virDomainObjPtr vm; + virCgroupPtr cgroup; +}; +typedef struct _qemuCgroupData qemuCgroupData; + int qemuCgroupControllerActive(struct qemud_driver *driver, int controller); -int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk, - const char *path, - size_t depth, - void *opaque); int qemuSetupDiskCgroup(struct qemud_driver *driver, + virDomainObjPtr vm, virCgroupPtr cgroup, virDomainDiskDefPtr disk); -int qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk, - const char *path, - size_t depth, - void *opaque); int qemuTeardownDiskCgroup(struct qemud_driver *driver, + virDomainObjPtr vm, virCgroupPtr cgroup, virDomainDiskDefPtr disk); -int qemuSetupChardevCgroup(virDomainDefPtr def, - virDomainChrDefPtr dev, - void *opaque); int qemuSetupHostUsbDeviceCgroup(usbDevice *dev, const char *path, void *opaque); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 15b9bc0..1916ed6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3989,7 +3989,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom, vm->def->name); goto endjob; } - if (qemuSetupDiskCgroup(driver, cgroup, dev->data.disk) < 0) + if (qemuSetupDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) goto endjob; } @@ -4035,7 +4035,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom, /* Fallthrough */ } if (ret != 0 && cgroup) { - if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0) + if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(dev->data.disk->src)); } @@ -4161,7 +4161,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, vm->def->name); goto endjob; } - if (qemuSetupDiskCgroup(driver, cgroup, dev->data.disk) < 0) + if (qemuSetupDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) goto endjob; } @@ -4185,7 +4185,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, } if (ret != 0 && cgroup) { - if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0) + if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(dev->data.disk->src)); } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0002af0..8090b90 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -893,6 +893,7 @@ int qemuDomainAttachHostUsbDevice(struct qemud_driver *driver, if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { virCgroupPtr cgroup = NULL; usbDevice *usb; + qemuCgroupData data = { vm, cgroup }; if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -905,7 +906,7 @@ int qemuDomainAttachHostUsbDevice(struct qemud_driver *driver, hostdev->source.subsys.u.usb.device)) == NULL) goto error; - if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, cgroup) < 0 ) + if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, &data) < 0) goto error; } @@ -1206,7 +1207,7 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); if (cgroup != NULL) { - if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0) + if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(dev->data.disk->src)); } @@ -1284,7 +1285,7 @@ int qemuDomainDetachSCSIDiskDevice(struct qemud_driver *driver, VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); if (cgroup != NULL) { - if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0) + if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(dev->data.disk->src)); } -- 1.7.4

On Wed, Feb 23, 2011 at 05:03:00PM -0700, Eric Blake wrote:
* src/qemu/qemu_cgroup.h (struct qemuCgroupData): New helper type. (qemuSetupDiskPathAllow, qemuSetupChardevCgroup) (qemuTeardownDiskPathDeny): Drop unneeded prototypes. (qemuSetupDiskCgroup, qemuTeardownDiskCgroup): Adjust prototype. * src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow, qemuSetupChardevCgroup) (qemuTeardownDiskPathDeny): Mark static and use new type. (qemuSetupHostUsbDeviceCgroup): Use new type. (qemuSetupDiskCgroup): Alter signature. (qemuSetupCgroup): Adjust caller. * src/qemu/qemu_hotplug.c (qemuDomainAttachHostUsbDevice) (qemuDomainDetachPciDiskDevice, qemuDomainDetachSCSIDiskDevice): Likewise. * src/qemu/qemu_driver.c (qemudDomainAttachDevice) (qemuDomainUpdateDeviceFlags): Likewise. ---
v2: rebase, no functional change
src/qemu/qemu_cgroup.c | 58 ++++++++++++++++++++++++++-------------------- src/qemu/qemu_cgroup.h | 21 +++++++--------- src/qemu/qemu_driver.c | 8 +++--- src/qemu/qemu_hotplug.c | 7 +++-- 4 files changed, 50 insertions(+), 44 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

* src/qemu/qemu_audit.h (qemuDomainCgroupAudit): New prototype. * src/qemu/qemu_audit.c (qemuDomainCgroupAudit): Implement it. * src/qemu/qemu_driver.c (qemudDomainSaveFlag): Add audit. * src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow) (qemuSetupChardevCgroup, qemuSetupHostUsbDeviceCgroup) (qemuSetupCgroup, qemuTeardownDiskPathDeny): Likewise. --- v2: updated to avoid audit messages about regular files, which never affect cgroup ACLs src/qemu/qemu_audit.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_audit.h | 9 ++++++++- src/qemu/qemu_cgroup.c | 19 +++++++++++++++++++ src/qemu/qemu_driver.c | 7 +++++++ 4 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c index f03f984..c76d49e 100644 --- a/src/qemu/qemu_audit.c +++ b/src/qemu/qemu_audit.c @@ -1,7 +1,7 @@ /* * qemu_audit.c: QEMU audit management * - * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -102,6 +102,52 @@ void qemuDomainNetAudit(virDomainObjPtr vm, } +/** + * qemuDomainCgroupAudit: + * @vm: domain making the cgroups ACL change + * @cgroup: cgroup that manages the devices + * @reason: either "allow" or "deny" + * @item: one of "all", "file", or "major" + * @name: NULL for @item of "all", path for @item of "file", and + * string describing major device type for @item of "major" + * @success: true if the cgroup operation succeeded + * + * Log an audit message about an attempted cgroup device ACL change. + */ +void qemuDomainCgroupAudit(virDomainObjPtr vm, + virCgroupPtr cgroup ATTRIBUTE_UNUSED, + const char *reason, + const char *item, + const char *name, + bool success) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *vmname; + char *detail = NULL; + + virUUIDFormat(vm->def->uuid, uuidstr); + if (!(vmname = virAuditEncode("vm", vm->def->name))) { + VIR_WARN0("OOM while encoding audit message"); + return; + } + if (name && + !(detail = virAuditEncode(STREQ(item, "file") ? "path" : "type", + name))) { + VIR_WARN0("OOM while encoding audit message"); + goto cleanup; + } + + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, + "resrc=cgroup reason=%s %s uuid=%s item=%s%s%s", + reason, vmname, uuidstr, + item, detail ? " " : "", detail ? detail : ""); + +cleanup: + VIR_FREE(vmname); + VIR_FREE(detail); +} + + static void qemuDomainLifecycleAudit(virDomainObjPtr vm, const char *op, const char *reason, diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h index 5b5a5d3..40f4591 100644 --- a/src/qemu/qemu_audit.h +++ b/src/qemu/qemu_audit.h @@ -1,7 +1,7 @@ /* * qemu_audit.h: QEMU audit management * - * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -25,6 +25,7 @@ # define __QEMU_AUDIT_H__ # include "domain_conf.h" +# include "cgroup.h" void qemuDomainStartAudit(virDomainObjPtr vm, const char *reason, bool success); void qemuDomainStopAudit(virDomainObjPtr vm, const char *reason); @@ -38,6 +39,12 @@ void qemuDomainNetAudit(virDomainObjPtr vm, virDomainNetDefPtr newDef, const char *reason, bool success); +void qemuDomainCgroupAudit(virDomainObjPtr vm, + virCgroupPtr group, + const char *reason, + const char *item, + const char *name, + bool success); void qemuDomainSecurityLabelAudit(virDomainObjPtr vm, bool success); #endif /* __QEMU_AUDIT_H__ */ diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 49ec473..5d4f35e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -29,6 +29,7 @@ #include "memory.h" #include "virterror_internal.h" #include "util.h" +#include "qemu_audit.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -66,6 +67,9 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, VIR_DEBUG("Process path %s for disk", path); /* XXX RO vs RW */ rc = virCgroupAllowDevicePath(data->cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "file", path, + rc == 0); if (rc < 0) { if (rc == -EACCES) { /* Get this for root squash NFS */ VIR_DEBUG("Ignoring EACCES for %s", path); @@ -106,6 +110,9 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, VIR_DEBUG("Process path %s for disk", path); /* XXX RO vs RW */ rc = virCgroupDenyDevicePath(data->cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(data->vm, data->cgroup, "deny", "file", path, + rc == 0); if (rc < 0) { if (rc == -EACCES) { /* Get this for root squash NFS */ VIR_DEBUG("Ignoring EACCES for %s", path); @@ -148,6 +155,9 @@ qemuSetupChardevCgroup(virDomainDefPtr def, VIR_DEBUG("Process path '%s' for disk", dev->source.data.file.path); rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path); + if (rc < 0) + qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "file", + dev->source.data.file.path, rc == 0); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), @@ -168,6 +178,9 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED, VIR_DEBUG("Process path '%s' for USB device", path); rc = virCgroupAllowDevicePath(data->cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "file", path, + rc == 0); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s"), @@ -203,6 +216,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { qemuCgroupData data = { vm, cgroup }; rc = virCgroupDenyAllDevices(cgroup); + qemuDomainCgroupAudit(vm, cgroup, "deny", "all", NULL, rc == 0); if (rc != 0) { if (rc == -EPERM) { VIR_WARN0("Group devices ACL is not accessible, disabling whitelisting"); @@ -220,6 +234,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, } rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR); + qemuDomainCgroupAudit(vm, cgroup, "allow", "major", "pty", rc == 0); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to allow /dev/pts/ devices")); @@ -228,6 +243,8 @@ int qemuSetupCgroup(struct qemud_driver *driver, if (vm->def->nsounds) { rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR); + qemuDomainCgroupAudit(vm, cgroup, "allow", "major", "sound", + rc == 0); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to allow /dev/snd/ devices")); @@ -238,6 +255,8 @@ int qemuSetupCgroup(struct qemud_driver *driver, for (i = 0; deviceACL[i] != NULL ; i++) { rc = virCgroupAllowDevicePath(cgroup, deviceACL[i]); + qemuDomainCgroupAudit(vm, cgroup, "allow", "file", + deviceACL[i], rc == 0); if (rc < 0 && rc != -ENOENT) { virReportSystemError(-rc, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1916ed6..c2ddd34 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1963,6 +1963,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, goto endjob; } rc = virCgroupAllowDevicePath(cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(vm, cgroup, "allow", "file", path, rc == 0); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), @@ -2012,6 +2014,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, if (cgroup != NULL) { rc = virCgroupDenyDevicePath(cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(vm, cgroup, "deny", "file", path, rc == 0); if (rc < 0) VIR_WARN("Unable to deny device %s for %s %d", path, vm->def->name, rc); @@ -2043,6 +2047,9 @@ endjob: if (cgroup != NULL) { rc = virCgroupDenyDevicePath(cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(vm, cgroup, "deny", "file", path, + rc == 0); if (rc < 0) VIR_WARN("Unable to deny device %s for %s: %d", path, vm->def->name, rc); -- 1.7.4

On Wed, Feb 23, 2011 at 05:03:01PM -0700, Eric Blake wrote:
* src/qemu/qemu_audit.h (qemuDomainCgroupAudit): New prototype. * src/qemu/qemu_audit.c (qemuDomainCgroupAudit): Implement it. * src/qemu/qemu_driver.c (qemudDomainSaveFlag): Add audit. * src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow) (qemuSetupChardevCgroup, qemuSetupHostUsbDeviceCgroup) (qemuSetupCgroup, qemuTeardownDiskPathDeny): Likewise. ---
v2: updated to avoid audit messages about regular files, which never affect cgroup ACLs
src/qemu/qemu_audit.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_audit.h | 9 ++++++++- src/qemu/qemu_cgroup.c | 19 +++++++++++++++++++ src/qemu/qemu_driver.c | 7 +++++++ 4 files changed, 81 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c index f03f984..c76d49e 100644 --- a/src/qemu/qemu_audit.c +++ b/src/qemu/qemu_audit.c @@ -1,7 +1,7 @@ /* * qemu_audit.c: QEMU audit management * - * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -102,6 +102,52 @@ void qemuDomainNetAudit(virDomainObjPtr vm, }
+/** + * qemuDomainCgroupAudit: + * @vm: domain making the cgroups ACL change + * @cgroup: cgroup that manages the devices + * @reason: either "allow" or "deny" + * @item: one of "all", "file", or "major"
I'd be inclined to rename 'file' to 'path', because to me 'file' implies a plain file, while we're actually dealing with block devices.
+ * @name: NULL for @item of "all", path for @item of "file", and + * string describing major device type for @item of "major" + * @success: true if the cgroup operation succeeded + * + * Log an audit message about an attempted cgroup device ACL change. + */ +void qemuDomainCgroupAudit(virDomainObjPtr vm, + virCgroupPtr cgroup ATTRIBUTE_UNUSED, + const char *reason, + const char *item, + const char *name, + bool success) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *vmname; + char *detail = NULL; + + virUUIDFormat(vm->def->uuid, uuidstr); + if (!(vmname = virAuditEncode("vm", vm->def->name))) { + VIR_WARN0("OOM while encoding audit message"); + return; + } + if (name && + !(detail = virAuditEncode(STREQ(item, "file") ? "path" : "type", + name))) { + VIR_WARN0("OOM while encoding audit message"); + goto cleanup; + } + + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, + "resrc=cgroup reason=%s %s uuid=%s item=%s%s%s", + reason, vmname, uuidstr, + item, detail ? " " : "", detail ? detail : ""); + +cleanup: + VIR_FREE(vmname); + VIR_FREE(detail); +} + + static void qemuDomainLifecycleAudit(virDomainObjPtr vm, const char *op, const char *reason, diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h index 5b5a5d3..40f4591 100644 --- a/src/qemu/qemu_audit.h +++ b/src/qemu/qemu_audit.h @@ -1,7 +1,7 @@ /* * qemu_audit.h: QEMU audit management * - * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -25,6 +25,7 @@ # define __QEMU_AUDIT_H__
# include "domain_conf.h" +# include "cgroup.h"
void qemuDomainStartAudit(virDomainObjPtr vm, const char *reason, bool success); void qemuDomainStopAudit(virDomainObjPtr vm, const char *reason); @@ -38,6 +39,12 @@ void qemuDomainNetAudit(virDomainObjPtr vm, virDomainNetDefPtr newDef, const char *reason, bool success); +void qemuDomainCgroupAudit(virDomainObjPtr vm, + virCgroupPtr group, + const char *reason, + const char *item, + const char *name, + bool success); void qemuDomainSecurityLabelAudit(virDomainObjPtr vm, bool success);
#endif /* __QEMU_AUDIT_H__ */ diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 49ec473..5d4f35e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -29,6 +29,7 @@ #include "memory.h" #include "virterror_internal.h" #include "util.h" +#include "qemu_audit.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -66,6 +67,9 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, VIR_DEBUG("Process path %s for disk", path); /* XXX RO vs RW */ rc = virCgroupAllowDevicePath(data->cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "file", path, + rc == 0); if (rc < 0) { if (rc == -EACCES) { /* Get this for root squash NFS */ VIR_DEBUG("Ignoring EACCES for %s", path); @@ -106,6 +110,9 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, VIR_DEBUG("Process path %s for disk", path); /* XXX RO vs RW */ rc = virCgroupDenyDevicePath(data->cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(data->vm, data->cgroup, "deny", "file", path, + rc == 0); if (rc < 0) { if (rc == -EACCES) { /* Get this for root squash NFS */ VIR_DEBUG("Ignoring EACCES for %s", path); @@ -148,6 +155,9 @@ qemuSetupChardevCgroup(virDomainDefPtr def,
VIR_DEBUG("Process path '%s' for disk", dev->source.data.file.path); rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path); + if (rc < 0) + qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "file", + dev->source.data.file.path, rc == 0); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), @@ -168,6 +178,9 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path '%s' for USB device", path); rc = virCgroupAllowDevicePath(data->cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "file", path, + rc == 0); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s"), @@ -203,6 +216,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { qemuCgroupData data = { vm, cgroup }; rc = virCgroupDenyAllDevices(cgroup); + qemuDomainCgroupAudit(vm, cgroup, "deny", "all", NULL, rc == 0); if (rc != 0) { if (rc == -EPERM) { VIR_WARN0("Group devices ACL is not accessible, disabling whitelisting"); @@ -220,6 +234,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, }
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR); + qemuDomainCgroupAudit(vm, cgroup, "allow", "major", "pty", rc == 0); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to allow /dev/pts/ devices")); @@ -228,6 +243,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
if (vm->def->nsounds) { rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR); + qemuDomainCgroupAudit(vm, cgroup, "allow", "major", "sound", + rc == 0); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to allow /dev/snd/ devices")); @@ -238,6 +255,8 @@ int qemuSetupCgroup(struct qemud_driver *driver, for (i = 0; deviceACL[i] != NULL ; i++) { rc = virCgroupAllowDevicePath(cgroup, deviceACL[i]); + qemuDomainCgroupAudit(vm, cgroup, "allow", "file", + deviceACL[i], rc == 0); if (rc < 0 && rc != -ENOENT) { virReportSystemError(-rc, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1916ed6..c2ddd34 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1963,6 +1963,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, goto endjob; } rc = virCgroupAllowDevicePath(cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(vm, cgroup, "allow", "file", path, rc == 0); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), @@ -2012,6 +2014,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
if (cgroup != NULL) { rc = virCgroupDenyDevicePath(cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(vm, cgroup, "deny", "file", path, rc == 0); if (rc < 0) VIR_WARN("Unable to deny device %s for %s %d", path, vm->def->name, rc); @@ -2043,6 +2047,9 @@ endjob:
if (cgroup != NULL) { rc = virCgroupDenyDevicePath(cgroup, path); + if (rc <= 0) + qemuDomainCgroupAudit(vm, cgroup, "deny", "file", path, + rc == 0); if (rc < 0) VIR_WARN("Unable to deny device %s for %s: %d", path, vm->def->name, rc);
ACK if s/file/path/ Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/24/2011 03:08 AM, Daniel P. Berrange wrote:
+/** + * qemuDomainCgroupAudit: + * @vm: domain making the cgroups ACL change + * @cgroup: cgroup that manages the devices + * @reason: either "allow" or "deny" + * @item: one of "all", "file", or "major"
I'd be inclined to rename 'file' to 'path', because to me 'file' implies a plain file, while we're actually dealing with block devices.
Fair enough; I've made that change.
@@ -1615,6 +1617,7 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { goto endjob; }
+ /* XXX update vm->def->mem.cur_balloon? */ The reason we don't update cur_balloon, is that all we're doing here is making a *request* to guest OS change its balloon level. The guest is under no obligation to comply and if it does comply it may not reach the requested level immediately. Hence we always talk to the guest to get an update at time of query instead.
Good point; I've removed this comment. However, does that mean that the audit should be querying the guest for the current usage rather than relying on a (possibly-stale) vm->def->mem.cur_balloon as the oldmem parameter in the audit call? I can prepare a followup patch for that, if you think it is worth it.
ACK
I've gone ahead and pushed the amended series. There may be some followup patches next week based from reviews by people involved with Common Criteria testing (basically, where the request for audit points originated in the first place), but they can be independent patches without holding up this part of the series. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

* src/qemu/qemu_audit.h (qemuDomainMemoryAudit) (qemuDomainVcpuAudit): New prototypes. * src/qemu/qemu_audit.c (qemuDomainResourceAudit) (qemuDomainMemoryAudit, qemuDomainVcpuAudit): New functions. (qemuDomainStartAudit): Call as appropriate. * src/qemu/qemu_driver.c (qemudDomainSetMemory) (qemudDomainHotplugVcpus): Likewise. --- v2: fix logic bug (qemuMonitorSet{Balloon,Cpu} returns -1 on monitor failure, 0 on unsupported, and 1 on success) src/qemu/qemu_audit.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_audit.h | 10 ++++++++ src/qemu/qemu_driver.c | 7 +++++- 3 files changed, 72 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c index c76d49e..6ea31c9 100644 --- a/src/qemu/qemu_audit.c +++ b/src/qemu/qemu_audit.c @@ -148,6 +148,59 @@ cleanup: } +/** + * qemuDomainResourceAudit: + * @vm: domain making an integer resource change + * @resource: name of the resource: "mem" or "vcpu" + * @oldval: the old value of the resource + * @newval: the new value of the resource + * @reason: either "start" or "update" + * @success: true if the resource change succeeded + * + * Log an audit message about an attempted resource change. + */ +static void +qemuDomainResourceAudit(virDomainObjPtr vm, + const char *resource, + unsigned long long oldval, + unsigned long long newval, + const char *reason, + bool success) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *vmname; + + virUUIDFormat(vm->def->uuid, uuidstr); + if (!(vmname = virAuditEncode("vm", vm->def->name))) { + VIR_WARN0("OOM while encoding audit message"); + return; + } + + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, + "resrc=%s reason=%s %s uuid=%s old-%s=%lld new-%s=%lld", + resource, reason, vmname, uuidstr, + resource, oldval, resource, newval); + + VIR_FREE(vmname); +} + +void +qemuDomainMemoryAudit(virDomainObjPtr vm, + unsigned long long oldmem, unsigned long long newmem, + const char *reason, bool success) +{ + return qemuDomainResourceAudit(vm, "mem", oldmem, newmem, reason, success); +} + +void +qemuDomainVcpuAudit(virDomainObjPtr vm, + unsigned int oldvcpu, unsigned int newvcpu, + const char *reason, bool success) +{ + return qemuDomainResourceAudit(vm, "vcpu", oldvcpu, newvcpu, reason, + success); +} + static void qemuDomainLifecycleAudit(virDomainObjPtr vm, const char *op, const char *reason, @@ -185,6 +238,9 @@ void qemuDomainStartAudit(virDomainObjPtr vm, const char *reason, bool success) qemuDomainNetAudit(vm, NULL, net, "start", true); } + qemuDomainMemoryAudit(vm, 0, vm->def->mem.cur_balloon, "start", true); + qemuDomainVcpuAudit(vm, 0, vm->def->vcpus, "start", true); + qemuDomainLifecycleAudit(vm, "start", reason, success); } diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h index 40f4591..cdbb957 100644 --- a/src/qemu/qemu_audit.h +++ b/src/qemu/qemu_audit.h @@ -45,6 +45,16 @@ void qemuDomainCgroupAudit(virDomainObjPtr vm, const char *item, const char *name, bool success); +void qemuDomainMemoryAudit(virDomainObjPtr vm, + unsigned long long oldmem, + unsigned long long newmem, + const char *reason, + bool success); +void qemuDomainVcpuAudit(virDomainObjPtr vm, + unsigned int oldvcpu, + unsigned int newvcpu, + const char *reason, + bool success); void qemuDomainSecurityLabelAudit(virDomainObjPtr vm, bool success); #endif /* __QEMU_AUDIT_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c2ddd34..65dfa5f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1605,6 +1605,8 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { qemuDomainObjEnterMonitor(vm); r = qemuMonitorSetBalloon(priv->mon, newmem); qemuDomainObjExitMonitor(vm); + qemuDomainMemoryAudit(vm, vm->def->mem.cur_balloon, newmem, "update", + r == 1); if (r < 0) goto endjob; @@ -1615,6 +1617,7 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { goto endjob; } + /* XXX update vm->def->mem.cur_balloon? */ ret = 0; endjob: if (qemuDomainObjEndJob(vm) == 0) @@ -2518,8 +2521,9 @@ static void processWatchdogEvent(void *data, void *opaque) static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) { qemuDomainObjPrivatePtr priv = vm->privateData; - int i, rc; + int i, rc = 1; int ret = -1; + int oldvcpus = vm->def->vcpus; qemuDomainObjEnterMonitor(vm); @@ -2554,6 +2558,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) cleanup: qemuDomainObjExitMonitor(vm); + qemuDomainVcpuAudit(vm, oldvcpus, nvcpus, "update", rc == 1); return ret; unsupported: -- 1.7.4

On Wed, Feb 23, 2011 at 05:03:02PM -0700, Eric Blake wrote:
* src/qemu/qemu_audit.h (qemuDomainMemoryAudit) (qemuDomainVcpuAudit): New prototypes. * src/qemu/qemu_audit.c (qemuDomainResourceAudit) (qemuDomainMemoryAudit, qemuDomainVcpuAudit): New functions. (qemuDomainStartAudit): Call as appropriate. * src/qemu/qemu_driver.c (qemudDomainSetMemory) (qemudDomainHotplugVcpus): Likewise. ---
v2: fix logic bug (qemuMonitorSet{Balloon,Cpu} returns -1 on monitor failure, 0 on unsupported, and 1 on success)
src/qemu/qemu_audit.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_audit.h | 10 ++++++++ src/qemu/qemu_driver.c | 7 +++++- 3 files changed, 72 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c index c76d49e..6ea31c9 100644 --- a/src/qemu/qemu_audit.c +++ b/src/qemu/qemu_audit.c @@ -148,6 +148,59 @@ cleanup: }
+/** + * qemuDomainResourceAudit: + * @vm: domain making an integer resource change + * @resource: name of the resource: "mem" or "vcpu" + * @oldval: the old value of the resource + * @newval: the new value of the resource + * @reason: either "start" or "update" + * @success: true if the resource change succeeded + * + * Log an audit message about an attempted resource change. + */ +static void +qemuDomainResourceAudit(virDomainObjPtr vm, + const char *resource, + unsigned long long oldval, + unsigned long long newval, + const char *reason, + bool success) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *vmname; + + virUUIDFormat(vm->def->uuid, uuidstr); + if (!(vmname = virAuditEncode("vm", vm->def->name))) { + VIR_WARN0("OOM while encoding audit message"); + return; + } + + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, + "resrc=%s reason=%s %s uuid=%s old-%s=%lld new-%s=%lld", + resource, reason, vmname, uuidstr, + resource, oldval, resource, newval); + + VIR_FREE(vmname); +} + +void +qemuDomainMemoryAudit(virDomainObjPtr vm, + unsigned long long oldmem, unsigned long long newmem, + const char *reason, bool success) +{ + return qemuDomainResourceAudit(vm, "mem", oldmem, newmem, reason, success); +} + +void +qemuDomainVcpuAudit(virDomainObjPtr vm, + unsigned int oldvcpu, unsigned int newvcpu, + const char *reason, bool success) +{ + return qemuDomainResourceAudit(vm, "vcpu", oldvcpu, newvcpu, reason, + success); +} + static void qemuDomainLifecycleAudit(virDomainObjPtr vm, const char *op, const char *reason, @@ -185,6 +238,9 @@ void qemuDomainStartAudit(virDomainObjPtr vm, const char *reason, bool success) qemuDomainNetAudit(vm, NULL, net, "start", true); }
+ qemuDomainMemoryAudit(vm, 0, vm->def->mem.cur_balloon, "start", true); + qemuDomainVcpuAudit(vm, 0, vm->def->vcpus, "start", true); + qemuDomainLifecycleAudit(vm, "start", reason, success); }
diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h index 40f4591..cdbb957 100644 --- a/src/qemu/qemu_audit.h +++ b/src/qemu/qemu_audit.h @@ -45,6 +45,16 @@ void qemuDomainCgroupAudit(virDomainObjPtr vm, const char *item, const char *name, bool success); +void qemuDomainMemoryAudit(virDomainObjPtr vm, + unsigned long long oldmem, + unsigned long long newmem, + const char *reason, + bool success); +void qemuDomainVcpuAudit(virDomainObjPtr vm, + unsigned int oldvcpu, + unsigned int newvcpu, + const char *reason, + bool success); void qemuDomainSecurityLabelAudit(virDomainObjPtr vm, bool success);
#endif /* __QEMU_AUDIT_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c2ddd34..65dfa5f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1605,6 +1605,8 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { qemuDomainObjEnterMonitor(vm); r = qemuMonitorSetBalloon(priv->mon, newmem); qemuDomainObjExitMonitor(vm); + qemuDomainMemoryAudit(vm, vm->def->mem.cur_balloon, newmem, "update", + r == 1); if (r < 0) goto endjob;
@@ -1615,6 +1617,7 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { goto endjob; }
+ /* XXX update vm->def->mem.cur_balloon? */
The reason we don't update cur_balloon, is that all we're doing here is making a *request* to guest OS change its balloon level. The guest is under no obligation to comply and if it does comply it may not reach the requested level immediately. Hence we always talk to the guest to get an update at time of query instead.
ret = 0; endjob: if (qemuDomainObjEndJob(vm) == 0) @@ -2518,8 +2521,9 @@ static void processWatchdogEvent(void *data, void *opaque) static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) { qemuDomainObjPrivatePtr priv = vm->privateData; - int i, rc; + int i, rc = 1; int ret = -1; + int oldvcpus = vm->def->vcpus;
qemuDomainObjEnterMonitor(vm);
@@ -2554,6 +2558,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus)
cleanup: qemuDomainObjExitMonitor(vm); + qemuDomainVcpuAudit(vm, oldvcpus, nvcpus, "update", rc == 1); return ret;
unsupported:
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

* src/qemu/qemu_audit.h (qemuDomainHostdevAudit): New prototype. * src/qemu/qemu_audit.c (qemuDomainHostdevAudit): New function. (qemuDomainStartAudit): Call as appropriate. * src/qemu/qemu_hotplug.c (qemuDomainAttachHostPciDevice) (qemuDomainAttachHostUsbDevice, qemuDomainDetachHostPciDevice) (qemuDomainDetachHostUsbDevice): Likewise. --- v2: new patch; covers the case of pci and usb host device passthrough, worth auditing since this can involve IOMMU mappings to have the guest manage the device I/O. src/qemu/qemu_audit.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_audit.h | 4 ++ src/qemu/qemu_hotplug.c | 28 +++++++----------- 3 files changed, 89 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c index 6ea31c9..26247a6 100644 --- a/src/qemu/qemu_audit.c +++ b/src/qemu/qemu_audit.c @@ -103,6 +103,75 @@ void qemuDomainNetAudit(virDomainObjPtr vm, /** + * qemuDomainHostdevAudit: + * @vm: domain making a change in pass-through host device + * @hostdev: device being attached or removed + * @reason: one of "start, "attach", or "detach" + * @success: true if the device passthrough operation succeeded + * + * Log an audit message about an attempted device passthrough change. + */ +void +qemuDomainHostdevAudit(virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev, + const char *reason, + bool success) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *vmname; + char *address; + char *device; + + virUUIDFormat(vm->def->uuid, uuidstr); + if (!(vmname = virAuditEncode("vm", vm->def->name))) { + VIR_WARN0("OOM while encoding audit message"); + return; + } + + switch (hostdev->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + if (virAsprintf(&address, "%.4x:%.2x:%.2x.%.1x", + hostdev->source.subsys.u.pci.domain, + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function) < 0) { + VIR_WARN0("OOM while encoding audit message"); + goto cleanup; + } + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + if (virAsprintf(&address, "%.3d.%.3d", + hostdev->source.subsys.u.usb.bus, + hostdev->source.subsys.u.usb.device) < 0) { + VIR_WARN0("OOM while encoding audit message"); + goto cleanup; + } + break; + default: + VIR_WARN("Unexpected hostdev type while encoding audit message: %d", + hostdev->source.subsys.type); + goto cleanup; + } + + if (!(device = virAuditEncode("device", VIR_AUDIT_STR(address)))) { + VIR_WARN0("OOM while encoding audit message"); + goto cleanup; + } + + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, + "resrc=dev reason=%s %s uuid=%s type=%s %s", + reason, vmname, uuidstr, + virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type), + device); + +cleanup: + VIR_FREE(vmname); + VIR_FREE(device); + VIR_FREE(address); +} + + +/** * qemuDomainCgroupAudit: * @vm: domain making the cgroups ACL change * @cgroup: cgroup that manages the devices @@ -238,6 +307,11 @@ void qemuDomainStartAudit(virDomainObjPtr vm, const char *reason, bool success) qemuDomainNetAudit(vm, NULL, net, "start", true); } + for (i = 0 ; i < vm->def->nhostdevs ; i++) { + virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; + qemuDomainHostdevAudit(vm, hostdev, "start", true); + } + qemuDomainMemoryAudit(vm, 0, vm->def->mem.cur_balloon, "start", true); qemuDomainVcpuAudit(vm, 0, vm->def->vcpus, "start", true); diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h index cdbb957..2f901be 100644 --- a/src/qemu/qemu_audit.h +++ b/src/qemu/qemu_audit.h @@ -39,6 +39,10 @@ void qemuDomainNetAudit(virDomainObjPtr vm, virDomainNetDefPtr newDef, const char *reason, bool success); +void qemuDomainHostdevAudit(virDomainObjPtr vm, + virDomainHostdevDefPtr def, + const char *reason, + bool success); void qemuDomainCgroupAudit(virDomainObjPtr vm, virCgroupPtr group, const char *reason, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8090b90..da07c29 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -842,6 +842,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, hostdev->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; memcpy(&hostdev->info.addr.pci, &guestAddr, sizeof(guestAddr)); } + qemuDomainHostdevAudit(vm, hostdev, "attach", ret == 0); if (ret < 0) goto error; @@ -918,6 +919,7 @@ int qemuDomainAttachHostUsbDevice(struct qemud_driver *driver, hostdev->source.subsys.u.usb.bus, hostdev->source.subsys.u.usb.device); qemuDomainObjExitMonitorWithDriver(driver, vm); + qemuDomainHostdevAudit(vm, hostdev, "attach", ret == 0); if (ret < 0) goto error; @@ -1607,20 +1609,14 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(vm); - return -1; - } + ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); } else { - if (qemuMonitorRemovePCIDevice(priv->mon, - &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - return -1; - } + ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci); } qemuDomainObjExitMonitorWithDriver(driver, vm); - - ret = 0; + qemuDomainHostdevAudit(vm, detach, "detach", ret == 0); + if (ret < 0) + return -1; pci = pciGetDevice(detach->source.subsys.u.pci.domain, detach->source.subsys.u.pci.bus, @@ -1715,13 +1711,11 @@ int qemuDomainDetachHostUsbDevice(struct qemud_driver *driver, } qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - return -1; - } + ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); qemuDomainObjExitMonitorWithDriver(driver, vm); - - ret = 0; + qemuDomainHostdevAudit(vm, detach, "detach", ret == 0); + if (ret < 0) + return -1; if (vm->def->nhostdevs > 1) { memmove(vm->def->hostdevs + i, -- 1.7.4

On Wed, Feb 23, 2011 at 05:03:03PM -0700, Eric Blake wrote:
* src/qemu/qemu_audit.h (qemuDomainHostdevAudit): New prototype. * src/qemu/qemu_audit.c (qemuDomainHostdevAudit): New function. (qemuDomainStartAudit): Call as appropriate. * src/qemu/qemu_hotplug.c (qemuDomainAttachHostPciDevice) (qemuDomainAttachHostUsbDevice, qemuDomainDetachHostPciDevice) (qemuDomainDetachHostUsbDevice): Likewise. ---
v2: new patch; covers the case of pci and usb host device passthrough, worth auditing since this can involve IOMMU mappings to have the guest manage the device I/O.
src/qemu/qemu_audit.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_audit.h | 4 ++ src/qemu/qemu_hotplug.c | 28 +++++++-----------
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange
-
Eric Blake