[libvirt] [PATCH] util: fix clear_emulator_capabilities=0

[Laine Stump]

On 03/14/2013 12:26 PM, Laine Stump wrote:
> On 03/13/2013 06:09 PM, Eric Blake wrote:
>> On 03/13/2013 01:37 PM, Laine Stump wrote:
>>> My commit 7a2e845a865dc7fa82d2393ea2a770cfc8cf00b4 (and its
>>> prerequisites) managed to effectively ignore the
>>> clear_emulator_capabilities setting in qemu.conf (visible in the code
>>> as the VIR_EXEC_CLEAR_CAPS flag when qemu is being exec'ed), with the
>>> result that the capabilities are always cleared regardless of the
>>> qemu.conf setting. This patch fixes it by passing the flag through to
>>> virSetUIDGIDWithCaps(), which uses it to decide whether or not to
>>> clear existing capabilities before adding in those that were
>>> requested.
>>>
>>> Note that the existing capabilities are *always* cleared if the new
>>> process is going to run as non-root, since the whole point of running
>>> non-root is to have the capabilities removed (it's still possible to
>>> add back individual capabilities as needed though).
>>> ---
>>> This will need to be backported to v1.0.3-maint.
>> Yeah, now that Fedora 19 has branched and settled on 1.0.3 as its
>> starting point, it looks like v1.0.3-maint will be getting lots of fixes :)
>>
>>> +        if (virSetUIDGIDWithCaps(cmd->uid, cmd->gid, cmd->capabilities,
>>> +                                 (cmd->flags & VIR_EXEC_CLEAR_CAPS)) < 0) {
>> While gnulib guarantees that we have <stdbool.h>, it also states that we
>> cannot rely on C99 rules for slamming random integers into 1 when
>> converting into a bool context (especially true for C89 compilers using
>> gnulib's emulation, but apparently there are also buggy C99 compilers
>> that miscompile things).  This should use '(cmd->flags &
>> VIR_EXEC_CLEAR_CAPS) != 0' (or !! if you don't like != 0), just to be safe.
>>
>>> +    /* First drop all caps (unless the requested uid is "unchanged" or
>>> +     * root and clearExistingCaps wasn't requested), then add back
>>> +     * those in capBits + the extra ones we need to change uid/gid and
>>> +     * change the capabilities bounding set.
>>>       */
>>>  
>>> -    capng_clear(CAPNG_SELECT_BOTH);
>>> +    if (clearExistingCaps || (uid != 1 && uid != 0))
>> Did you mean uid != 0?
> Well, actually I meant "uid != -1 && uid != 0" (and I had to look at it
> 5 times to see that the negative sign was missing).
>
> I want to clear the existing caps if:
>
>    A) clearExistingCaps is true (duh)
>
> or
>
>    B) The new uid will be different, and it will be something other than
> root
>
>
>> ACK with those problems addressed.
>>

Okay, I pushed it after adding "!!" to the bool arg and fixing the "1"
to "-1". I also pushed the patch to the v1.0.3-maint branch.