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

Re: [libvirt] [PATCH 3/3] Replace setuid/setgid/initgroups with virSetUIDGID()

On 12/23/2010 04:05 PM, Eric Blake wrote:
On 12/23/2010 11:39 AM, Laine Stump wrote:
This patch fixes https://bugzilla.redhat.com/show_bug.cgi?id=664406

If qemu is run as a different uid, it has been unable to access mode
0660 files that are owned by a different user, but with a group that
the qemu is a member of (aside from the one group listed in the passwd
file), because initgroups() is not being called prior to the
exec. initgroups will change the group membership of the process (and
its children) to match the new uid.

To make this happen, the setregid()/setreuid() code in
qemuSecurityDACSetProcessLabel has been replaced with a call to
virSetUIDGID(), which does both of those, plus calls initgroups.

Similar, but not identical, code in qemudOpenAsUID() has been replaced
with virSetUIDGID(). This not only consolidates the functionality to a
single location, but also potentially fixes some as-yet unreported
  src/qemu/qemu_driver.c       |   44 +++++++++++++----------------------------
  src/qemu/qemu_security_dac.c |   18 +---------------
  2 files changed, 16 insertions(+), 46 deletions(-)
+    if (virSetUIDGID(uid, gid)<  0) {
+       exit_code = errno;
+       goto child_cleanup;
Ah, I see why you needed patch 1 - patch two calls virReportSystemError
in between the setting of errno and the return.

However, one glitch - VIR_FREE(x) does not (yet) guarantee that errno is
preserved (some poorly-written free() implementations can modify errno;
glibc is nicer in trying to avoid that).  You'll need to modify 1/3

Which means ACK to this patch as perfect, but the prerequisites aren't
there yet :)


Thanks again for all the last minute pre-holiday reviews!

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