[libvirt] [PATCH] qemu: Set umask before calling mknod()
Michal Privoznik
mprivozn at redhat.com
Tue Feb 14 10:37:41 UTC 2017
On 02/13/2017 09:18 PM, Andrea Bolognani wrote:
> When we populate the private /dev that's going to be used by
> an isolated QEMU process, we take care all metadata matches
> what's in the top-level namespace: in particular, we copy the
> file permissions directly.
>
> However, since the permissions passed to mknod() are still
> affected by the active umask, we need to set it to a very
> permissive value before creating device nodes to avoid file
> access issues.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1421036
> ---
> src/qemu/qemu_domain.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f62bf8f..7993acc 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7040,6 +7040,7 @@ qemuDomainCreateDeviceRecursive(const char *device,
> #ifdef WITH_SELINUX
> char *tcon = NULL;
> #endif
> + mode_t oldUmask = umask((mode_t) 0);
>
> if (!ttl) {
> virReportSystemError(ELOOP,
> @@ -7205,6 +7206,7 @@ qemuDomainCreateDeviceRecursive(const char *device,
> #ifdef WITH_SELINUX
> freecon(tcon);
> #endif
> + umask(oldUmask);
> return ret;
> }
There are couple of returns in between these two chunks so new umask
will be leaked.
Moreover, the current umask at this point should be 002 as a result of
virCommandSetUmask() called from qemuProcessLaunch(). virCommandRun()
then sets the umask also for the pre-exec hook. Does it make sense to
run the pre-exec hook with unchanged umask?
But as we are discussing in person right now, there is something fishy
going on: on ppc and aarch64 the umask is honoured when calling mknod()
but on x86_64 it is not. Maybe somebody on the list has a bright idea
why that might be?
Michal
More information about the libvir-list
mailing list