[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 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
> bugs.
> ---
>  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
accordingly.

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

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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