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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
