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

Re: [libvirt] [PATCH 2/3] Set SELinux context label of pipes used for qemu migration



On Tue, Jan 25, 2011 at 04:24:19AM -0500, Laine Stump wrote:
> This patch is a partial resolution to the following bug:
> 
>    https://bugzilla.redhat.com/show_bug.cgi?id=667756
> 
> (to complete the fix, an updated selinux-policy package is required,
> to add the policy that allows libvirt to set the context of a fifo,
> which was previously not allowed).
> 
> Explanation : When an incoming migration is over a pipe (for example,
> if the image was compressed and is being fed through gzip, or was on a
> root-squash nfs server, so needed to be opened by a child process
> running as a different uid), qemu cannot read it unless the selinux
> context label for the pipe has been set properly.
> 
> The solution is to check the fd used as the source of the migration
> just before passing it to qemu; if it's a fifo (implying that it's a
> pipe), we call the newly added virSecurityManagerSetFDLabel() function
> to set the context properly.
> ---
>  src/qemu/qemu_driver.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 34cc29f..985b062 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2667,6 +2667,24 @@ static int qemudStartVMDaemon(virConnectPtr conn,
>                                        vm, stdin_path) < 0)
>          goto cleanup;
>  
> +    if (stdin_fd != -1) {
> +        /* if there's an fd to migrate from, and it's a pipe, put the
> +         * proper security label on it
> +         */
> +        struct stat stdin_sb;
> +
> +        DEBUG0("setting security label on pipe used for migration");
> +
> +        if (fstat(stdin_fd, &stdin_sb) < 0) {
> +            virReportSystemError(errno,
> +                                 _("cannot stat fd %d"), stdin_fd);
> +            goto cleanup;
> +        }
> +        if (S_ISFIFO(stdin_sb.st_mode) &&
> +            virSecurityManagerSetFDLabel(driver->securityManager, vm, stdin_fd) < 0)
> +            goto cleanup;
> +    }

This feels like the wrong place to put this call. The callers
of qemudStartVMDaemon() which opened 'stdin_fd' in the first
place will already know if it is a pipe or not. If we put
the virSecurityManagerSetFDLabel call in the appropriate
callers, then the fstat() complexity is avoided.

Daniel


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