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

Re: [libvirt] Release candidate 2 of 1.0.3 is available



Hey,

On Wed, Feb 27, 2013 at 07:20:56PM +0800, Daniel Veillard wrote:
>    I have just tagged git and pushed the tarball. The rpms for F17
> are at the usual place too:
>      ftp://libvirt.org/libvirt/
> 
>    I gave a try to the set of rpms and this seems to work fine for
> relatively simple tests.
>    Please give it more testing and let's keep change to git to
> bug fixes to minimize risks for 1.0.3.
> If everything goes well I will push 1.0.3 on Friday,

I've found a pretty bad issue with rc2/git head and Boxes, creating new
boxes fails with
2013-03-02 14:04:22.028+0000: 15681: error :
qemuDomainCheckDiskPresence:1837 : cannot access file
'/home/teuf/isos/msdn/win7/fr_windows_7_home_premium_n_with_sp1_x64_dvd_u_676833.iso':
Opération non permise

Looking more into it, this seems to be caused by

commit f506a4c115c44003455cb956861836a46425f97b
Date:   Thu Jan 31 13:18:45 2013 -0500

    util: make virSetUIDGID a NOP only when uid or gid is -1

qemuDomainCheckDiskPresence calls virFileAccessibleAs with (user, group)
being (0, 0) as Boxes is using qemu:///session (these are set to 0 by
virQEMUDriverConfigNew in the session case).

virFileAccessibleAs calls virSetUIDGID(0, 0) which used to be a noop before
the commit above, but is now trying to call setreuid/setregid, which fails.

I tried reverting this patch, but then probing qemu binaries during
libvirtd startup fails so not a good workaround :)

The patch below works for me, but it needs careful review as this is code I
don't know at all and I've only lightly tested it. This issue is a 1.0.3
blocker as far as Boxes is concerned.

Thanks,

Christophe


From 6474eb9dc04dcaa29450116ddfb76aefdaffd4f6 Mon Sep 17 00:00:00 2001
From: Christophe Fergeau <cfergeau redhat com>
Date: Sat, 2 Mar 2013 15:19:47 +0100
Subject: [PATCH] qemu: Use -1 as unpriviledged uid/gid

Commit f506a4c1 changed virSetUIDGID() to be a noop
when uid/gid are -1, while it used to be a noop when
they are <= 0.

The changes in this commit broke creating new VMs in GNOME Boxes
as qemuDomainCheckDiskPresence gets called during domain creation/startup,
which in turn calls virFileAccessibleAs which fails after calling
virSetUIDGID(0, 0) (Boxes uses session libvirtd).

This commit changes virQEMUDriverConfigNew to use -1 as the unpriviledged
uid/gid, and adjusts one code path that expected 0 in this case. I've also
looked at the various places where cfg->user is used, and they all
seem to handle -1 correctly.
---
 src/qemu/qemu_conf.c   | 4 ++--
 src/qemu/qemu_domain.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index f3b09cf..3ef3499 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -129,8 +129,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
         if (virGetGroupID(QEMU_GROUP, &cfg->group) < 0)
             goto error;
     } else {
-        cfg->user = 0;
-        cfg->group = 0;
+        cfg->user = (uid_t)-1;
+        cfg->group = (gid_t)-1;
     }
     cfg->dynamicOwnership = privileged;
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0e56596..1ecc8fa 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1301,8 +1301,8 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver,
 
     if (cfg->privileged &&
         (!cfg->clearEmulatorCapabilities ||
-         cfg->user == 0 ||
-         cfg->group == 0))
+         cfg->user == (uid_t)-1 ||
+         cfg->group == (gid_t)-1))
         qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, logFD);
 
     if (obj->def->namespaceData) {
-- 
1.8.1.2

Attachment: pgpry1uGOT72j.pgp
Description: PGP signature


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