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

Re: [libvirt] [PATCH] Call initgroups for qemu's uid prior to exec



On 12/21/2010 04:52 PM, Eric Blake wrote:
On 12/21/2010 01:45 PM, 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). initgroups will change the group membership of the process (and
its children) to match the new uid.
---
  src/qemu/qemu_security_dac.c |   27 +++++++++++++++++++++++++++
  1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c
index 55dc0c6..2e60aec 100644
--- a/src/qemu/qemu_security_dac.c
+++ b/src/qemu/qemu_security_dac.c
@@ -12,6 +12,8 @@
  #include<sys/types.h>
  #include<sys/stat.h>
  #include<fcntl.h>
+#include<pwd.h>
+#include<grp.h>

  #include "qemu_security_dac.h"
  #include "qemu_conf.h"
@@ -558,6 +560,30 @@ qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED,
          }
      }
      if (driver->user) {
+        struct passwd pwd, *pwd_result;
+        char *buf = NULL;
+        size_t bufsize = 16384;
qemu_driver.c sets this to 1024*1024.  Will that matter?

The usage in qemu_driver.c re-uses a buffer that was already conveniently allocated for something else. This buffer is used to contain all the strings encountered in a single entry from /etc/passwd. The manpage for getpwuid_r() has example code to learn the maximum possible size for all the strings in one line, but that example falls back to 16384 if it fails to learn the system limit, saying "this should be big enough for anything".

   For that
matter, can't you provide this functionality in a single .c file so that
both qemudOpenAsUID and qemuSecurityDACSetProcessLabel can share the
benefits of common code?

If I put it in a separate function, I won't feel so bad about adding in the extra code to detect the absolute max size to allocate. Of course, if I put it in a util file, we'll have to worry about making it compile on all you mother's favorite platforms, so... :-)

That refactoring probably deserves a v2.

Sure, I'll take a stab at it.

@@ -566,6 +592,7 @@ qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED,
          }
      }

+
      return 0;
Spurious whitespace change.

Oops.


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