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.