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

Re: [libvirt] [PATCHv2 2/2] security_dac: compute supplemental groups before fork



On 18.07.2013 15:16, Eric Blake wrote:
> On 07/18/2013 06:46 AM, Michal Privoznik wrote:
>> On 18.07.2013 01:08, Eric Blake wrote:
>>> Commit 75c1256 states that virGetGroupList must not be called
>>> between fork and exec, then commit ee777e99 promptly violated
>>> that for lxc's use of virSecurityManagerSetProcessLabel.  Hoist
>>> the supplemental group detection to the time that the security
>>> manager is created.  Qemu is safe, as it uses
>>> virSecurityManagerSetChildProcessLabel which in turn uses
>>> virCommand to determine supplemental groups.
>>>
> 
>>> -    if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0)
>>> +    if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) {
>>> +        if (groups)
>>> +            *groups = NULL;
>>> +        if (ngroups)
>>> +            ngroups = 0;
>>
>> I believe you wanted *ngroups = 0; in here.
>>
> 
> Indeed.  I blame C for treating 0 and NULL interchangeably.
> 
>>
>> ACK series, but see the issue I'm raising in 2/2.
> 
> Thanks; I'll push after fixing that typo.
> 

In fact, gcc warned me about possibly unused variable:

security/security_dac.c: In function 'virSecurityDACSetProcessLabel':
security/security_dac.c:1038:21: error: 'ngroups' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     if (virSetUIDGID(user, group, groups, ngroups) < 0)
                     ^
security/security_dac.c: At top level:
cc1: error: unrecognized command line option "-Wno-unused-command-line-argument" [-Werror]
cc1: all warnings being treated as errors

Truth is, I'm using the most recent gcc (4.8.1). Which is both advantage and disadvantage. One hand, I can catch errors like these, on the other hand, the gcc bug in static analysis has been fixed. The bug, where local variable 'shadows' global function. This is NOT what I call shadowing.

Michal


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