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

[Laine Stump]

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
>> 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 :)
>

Pushed.

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