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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux