[libvirt] [PATCH] qemu: Set umask before calling mknod()

[Andrea Bolognani]

On Tue, 2017-02-14 at 11:37 +0100, Michal Privoznik wrote:
> > @@ -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.

Noted.

> 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?

I think it would make sense, but then again, the default
umask is probably going to be 022 or something like that,
so in practice we'll still want to set it to 000 for this
specific task (copying the top-level device nodes as-is).

> 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?

Turns out mknod() honors the umask just fine, but something
after device node creation changes the file permission and
does so only on x86. More digging happening as we speak :)

-- 
Andrea Bolognani / Red Hat / Virtualization