Re: [libvirt] [PATCH] qemu: Copy SELinux labels for namespace too

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.