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