Device names can be manipulated, so it is better to also log
the major/minor device number corresponding to the cgroup ACL
changes that libvirt made. This required some refactoring
of the relatively new qemu cgroup audit code.
Also, qemuSetupChardevCgroup was only auditing on failure, not success.
* src/qemu/qemu_audit.h (qemuDomainCgroupAudit): Delete.
(qemuAuditCgroup, qemuAuditCgroupMajor, qemuAuditCgroupPath): New
prototypes.
* src/qemu/qemu_audit.c (qemuDomainCgroupAudit): Rename...
(qemuAuditCgroup): ...and drop a parameter.
(qemuAuditCgroupMajor, qemuAuditCgroupPath): New functions, to
allow listing device major/minor in audit.
(qemuAuditGetRdev): New helper function.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Adjust callers.
* src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow)
(qemuSetupHostUsbDeviceCgroup, qemuSetupCgroup)
(qemuTeardownDiskPathDeny): Likewise.
(qemuSetupChardevCgroup): Likewise, fixing missing audit.
---
v2: fix a logic bug I noticed, choose nicer function names
src/qemu/qemu_audit.c | 126 ++++++++++++++++++++++++++++++++++++++++--------
src/qemu/qemu_audit.h | 22 ++++++--
src/qemu/qemu_cgroup.c | 29 ++++-------
src/qemu/qemu_driver.c | 10 +---
4 files changed, 135 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 0f954c0..56b0b74 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -23,12 +23,43 @@
#include <config.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
#include "qemu_audit.h"
#include "virtaudit.h"
#include "uuid.h"
#include "logging.h"
#include "memory.h"
+/* Return rdev=nn:mm in hex for block and character devices, rdev=?
+ * for other file types or stat failure, or NULL on allocation
+ * failure. */
+#if defined major && defined minor
+static char *
+qemuAuditGetRdev(const char *path)
+{
+ char *ret;
+ struct stat sb;
+
+ if (stat(path, &sb) == 0 &&
+ (S_ISCHR(sb.st_mode) || S_ISBLK(sb.st_mode))) {
+ int maj = major(sb.st_rdev);
+ int min = minor(sb.st_rdev);
+ virAsprintf(&ret, "rdev=%02X:%02X", maj, min);
+ } else {
+ ret = strdup("rdev=?");
+ }
+ return ret;
+}
+#else
+static char *
+qemuAuditGetRdev(const char *path ATTRIBUTE_UNUSED)
+{
+ return strdup("rdev=?");
+}
+#endif
+
void qemuDomainDiskAudit(virDomainObjPtr vm,
virDomainDiskDefPtr oldDef,
virDomainDiskDefPtr newDef,
@@ -106,7 +137,7 @@ 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"
+ * @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.
@@ -172,51 +203,104 @@ cleanup:
/**
- * qemuDomainCgroupAudit:
+ * qemuAuditCgroup:
* @vm: domain making the cgroups ACL change
* @cgroup: cgroup that manages the devices
* @reason: either "allow" or "deny"
- * @item: one of "all", "path", or "major"
- * @name: NULL for @item of "all", device path for @item of "path", and
- * string describing major device type for @item of "major"
+ * @extra: additional details, in the form "all",
+ * "major category=xyz maj=nn", or "path path=xyz dev=nn:mm" (the
+ * latter two are generated by qemuAuditCgroupMajor and
+ * qemuAuditCgroupPath).
* @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)
+void
+qemuAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup ATTRIBUTE_UNUSED,
+ const char *reason, const char *extra, 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, "path") ? "path" : "category",
- name))) {
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "resrc=cgroup reason=%s %s uuid=%s class=%s",
+ reason, vmname, uuidstr, extra);
+
+ VIR_FREE(vmname);
+}
+
+/**
+ * qemuAuditCgroupMajor:
+ * @vm: domain making the cgroups ACL change
+ * @cgroup: cgroup that manages the devices
+ * @reason: either "allow" or "deny"
+ * @maj: the major number of the device category
+ * @name: a textual name for that device category, alphabetic only
+ * @success: true if the cgroup operation succeeded
+ *
+ * Log an audit message about an attempted cgroup device ACL change.
+ */
+void
+qemuAuditCgroupMajor(virDomainObjPtr vm, virCgroupPtr cgroup,
+ const char *reason, int maj, const char *name,
+ bool success)
+{
+ char *extra;
+
+ if (virAsprintf(&extra, "major category=%s maj=%02X", name, maj) < 0) {
+ VIR_WARN0("OOM while encoding audit message");
+ return;
+ }
+
+ qemuAuditCgroup(vm, cgroup, reason, extra, success);
+
+ VIR_FREE(extra);
+}
+
+/**
+ * qemuAuditCgroupPath:
+ * @vm: domain making the cgroups ACL change
+ * @cgroup: cgroup that manages the devices
+ * @reason: either "allow" or "deny"
+ * @path: the device being adjusted
+ * @rc: > 0 if not a device, 0 if success, < 0 if failure
+ *
+ * Log an audit message about an attempted cgroup device ACL change to
+ * a specific device.
+ */
+void
+qemuAuditCgroupPath(virDomainObjPtr vm, virCgroupPtr cgroup,
+ const char *reason, const char *path, int rc)
+{
+ char *detail;
+ char *rdev;
+ char *extra;
+
+ /* Nothing to audit for regular files. */
+ if (rc > 0)
+ return;
+
+ if (!(detail = virAuditEncode("path", path)) ||
+ !(rdev = qemuAuditGetRdev(path)) ||
+ virAsprintf(&extra, "path path=%s %s", path, rdev) < 0) {
VIR_WARN0("OOM while encoding audit message");
goto cleanup;
}
- VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "resrc=cgroup reason=%s %s uuid=%s class=%s%s%s",
- reason, vmname, uuidstr,
- item, detail ? " " : "", detail ? detail : "");
+ qemuAuditCgroup(vm, cgroup, reason, extra, rc == 0);
cleanup:
- VIR_FREE(vmname);
+ VIR_FREE(extra);
VIR_FREE(detail);
+ VIR_FREE(rdev);
}
-
/**
* qemuDomainResourceAudit:
* @vm: domain making an integer resource change
diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h
index 247edde..53855e2 100644
--- a/src/qemu/qemu_audit.h
+++ b/src/qemu/qemu_audit.h
@@ -43,12 +43,22 @@ void qemuDomainHostdevAudit(virDomainObjPtr vm,
virDomainHostdevDefPtr def,
const char *reason,
bool success);
-void qemuDomainCgroupAudit(virDomainObjPtr vm,
- virCgroupPtr group,
- const char *reason,
- const char *item,
- const char *name,
- bool success);
+void qemuAuditCgroup(virDomainObjPtr vm,
+ virCgroupPtr group,
+ const char *reason,
+ const char *extra,
+ bool success);
+void qemuAuditCgroupMajor(virDomainObjPtr vm,
+ virCgroupPtr group,
+ const char *reason,
+ int maj,
+ const char *name,
+ bool success);
+void qemuAuditCgroupPath(virDomainObjPtr vm,
+ virCgroupPtr group,
+ const char *reason,
+ const char *path,
+ int rc);
void qemuDomainMemoryAudit(virDomainObjPtr vm,
unsigned long long oldmem,
unsigned long long newmem,
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e71d3fa..ebf9ad5 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -67,9 +67,7 @@ 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", "path", path,
- rc == 0);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -110,9 +108,7 @@ 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", "path", path,
- rc == 0);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "deny", path, rc);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -155,9 +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);
- if (rc < 0)
- qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "path",
- dev->source.data.file.path, rc == 0);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "allow",
+ dev->source.data.file.path, rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -178,9 +173,7 @@ 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", "path", path,
- rc == 0);
+ qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s"),
@@ -216,7 +209,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);
+ qemuAuditCgroup(vm, cgroup, "deny", "all", rc == 0);
if (rc != 0) {
if (rc == -EPERM) {
VIR_WARN0("Group devices ACL is not accessible, disabling whitelisting");
@@ -234,7 +227,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
- qemuDomainCgroupAudit(vm, cgroup, "allow", "major", "pty", rc == 0);
+ qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
+ "pty", rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/pts/ devices"));
@@ -247,8 +241,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
driver->vncAllowHostAudio) ||
(vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)))) {
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
- qemuDomainCgroupAudit(vm, cgroup, "allow", "major", "sound",
- rc == 0);
+ qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR,
+ "sound", rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/snd/ devices"));
@@ -259,8 +253,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
for (i = 0; deviceACL[i] != NULL ; i++) {
rc = virCgroupAllowDevicePath(cgroup,
deviceACL[i]);
- qemuDomainCgroupAudit(vm, cgroup, "allow", "path",
- deviceACL[i], rc == 0);
+ qemuAuditCgroupPath(vm, cgroup, "allow", deviceACL[i], rc);
if (rc < 0 &&
rc != -ENOENT) {
virReportSystemError(-rc,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0f7cbad..0f7b5ec 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1964,8 +1964,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
goto endjob;
}
rc = virCgroupAllowDevicePath(cgroup, path);
- if (rc <= 0)
- qemuDomainCgroupAudit(vm, cgroup, "allow", "path", path, rc == 0);
+ qemuAuditCgroupPath(vm, cgroup, "allow", path, rc);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -2015,8 +2014,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path);
- if (rc <= 0)
- qemuDomainCgroupAudit(vm, cgroup, "deny", "path", path, rc == 0);
+ qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s %d",
path, vm->def->name, rc);
@@ -2048,9 +2046,7 @@ endjob:
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path);
- if (rc <= 0)
- qemuDomainCgroupAudit(vm, cgroup, "deny", "path", path,
- rc == 0);
+ qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s: %d",
path, vm->def->name, rc);
--
1.7.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list