On Fri, 2017-01-13 at 11:12 +0100, Michal Privoznik wrote: [...]
@@ -7023,10 +7029,34 @@ qemuDomainCreateDevice(const char *device, goto cleanup; } +#ifdef WITH_SELINUX + if (getfilecon_raw(canonDevicePath, &tcon) < 0 && + (errno != ENOTSUP && errno != ENODATA)) { + virReportSystemError(errno, + _("Unable to get SELinux label on %s"), canonDevicePath);
s/get SELinux label on/get SELinux label from/ One more occurrence in the patch.
+ goto cleanup; + } + + if (tcon && + setfilecon_raw(devicePath, (VIR_SELINUX_CTX_CONST char *) tcon) < 0) { + VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR + if (errno != EOPNOTSUPP && errno != ENOTSUP) { + VIR_WARNINGS_RESET + virReportSystemError(errno, + _("Unable to set SELinux label on %s"), + devicePath);
Please decide whether you want the argument to %s on the same line as the format string or on the next, and stick with it :) [...]
@@ -7571,6 +7617,9 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, cleanup: if (ret < 0 && delDevice) unlink(data->file); +#ifdef WITH_SELINUX + freecon(data->tcon); +#endif
I don't think you should free the SELinux context...
virFileFreeACLs(&data->acl);
... or the ACLs, for that matter, on failure: the caller will free them already if the helper fails, which is good because whoever allocates the memory should be responsible for releasing it. [...]
@@ -7619,6 +7677,9 @@ qemuDomainAttachDeviceMknod(virQEMUDriverPtr driver, ret = 0; cleanup: +#ifdef WITH_SELINUX + freecon(data.tcon); +#endif virFileFreeACLs(&data.acl); return 0;
Existing, but I'm pretty sure you want to return 'ret' rather than 0 here. ACK once you deal with the issues mentioned above, and we definitely want to have this in as soon as possible. -- Andrea Bolognani / Red Hat / Virtualization