[libvirt] [PATCH] lxc_container: Don't call virGetGroupList during exec
Commit 75c1256 states that virGetGroupList must not be called between fork and exec, then commit ee777e99 promptly violated that for lxc. Patch originally posted by Eric Blake <eblake@redhat.com>. --- src/lxc/lxc_container.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index b51d7a2..37d2ba6 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -351,24 +351,18 @@ int lxcContainerWaitForContinue(int control) */ static int lxcContainerSetID(virDomainDefPtr def) { - gid_t *groups; - int ngroups; - /* Only call virSetUIDGID when user namespace is enabled * for this container. And user namespace is only enabled * when nuidmap&ngidmap is not zero */ VIR_DEBUG("Set UID/GID to 0/0"); if (def->idmap.nuidmap && - ((ngroups = virGetGroupList(0, 0, &groups) < 0) || - virSetUIDGID(0, 0, groups, ngroups) < 0)) { + virSetUIDGID(0, 0, groups, ngroups) < 0) { virReportSystemError(errno, "%s", _("setuid or setgid failed")); - VIR_FREE(groups); return -1; } - VIR_FREE(groups); return 0; } -- 1.8.1.5
On Wed, Jul 17, 2013 at 11:28:42AM +0200, Michal Privoznik wrote:
Commit 75c1256 states that virGetGroupList must not be called between fork and exec, then commit ee777e99 promptly violated that for lxc.
Patch originally posted by Eric Blake <eblake@redhat.com>. --- src/lxc/lxc_container.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index b51d7a2..37d2ba6 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -351,24 +351,18 @@ int lxcContainerWaitForContinue(int control) */ static int lxcContainerSetID(virDomainDefPtr def) { - gid_t *groups; - int ngroups; - /* Only call virSetUIDGID when user namespace is enabled * for this container. And user namespace is only enabled * when nuidmap&ngidmap is not zero */
VIR_DEBUG("Set UID/GID to 0/0"); if (def->idmap.nuidmap && - ((ngroups = virGetGroupList(0, 0, &groups) < 0) || - virSetUIDGID(0, 0, groups, ngroups) < 0)) { + virSetUIDGID(0, 0, groups, ngroups) < 0) {
How does this compile ? You're removing the 'groups' and 'ngroups' variables but still referencing them here. Don't you mean to use NULL, 0 as the args for virSetUIDGID } Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 17.07.2013 11:43, Daniel P. Berrange wrote:
On Wed, Jul 17, 2013 at 11:28:42AM +0200, Michal Privoznik wrote:
Commit 75c1256 states that virGetGroupList must not be called between fork and exec, then commit ee777e99 promptly violated that for lxc.
Patch originally posted by Eric Blake <eblake@redhat.com>. --- src/lxc/lxc_container.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index b51d7a2..37d2ba6 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -351,24 +351,18 @@ int lxcContainerWaitForContinue(int control) */ static int lxcContainerSetID(virDomainDefPtr def) { - gid_t *groups; - int ngroups; - /* Only call virSetUIDGID when user namespace is enabled * for this container. And user namespace is only enabled * when nuidmap&ngidmap is not zero */
VIR_DEBUG("Set UID/GID to 0/0"); if (def->idmap.nuidmap && - ((ngroups = virGetGroupList(0, 0, &groups) < 0) || - virSetUIDGID(0, 0, groups, ngroups) < 0)) { + virSetUIDGID(0, 0, groups, ngroups) < 0) {
How does this compile ? You're removing the 'groups' and 'ngroups' variables but still referencing them here. Don't you mean to use NULL, 0 as the args for virSetUIDGID }
Daniel
Ugrh, yes.
On 07/17/2013 04:30 AM, Michal Privoznik wrote:
On 17.07.2013 11:43, Daniel P. Berrange wrote:
On Wed, Jul 17, 2013 at 11:28:42AM +0200, Michal Privoznik wrote:
Commit 75c1256 states that virGetGroupList must not be called between fork and exec, then commit ee777e99 promptly violated that for lxc.
Patch originally posted by Eric Blake <eblake@redhat.com>. --- src/lxc/lxc_container.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
Thanks; I had the same changes locally, but guess I hadn't posted them yet.
VIR_DEBUG("Set UID/GID to 0/0"); if (def->idmap.nuidmap && - ((ngroups = virGetGroupList(0, 0, &groups) < 0) || - virSetUIDGID(0, 0, groups, ngroups) < 0)) { + virSetUIDGID(0, 0, groups, ngroups) < 0) {
How does this compile ? You're removing the 'groups' and 'ngroups' variables but still referencing them here. Don't you mean to use NULL, 0 as the args for virSetUIDGID }
Yes, the 'NULL, 0' change squashed in is required :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Michal Privoznik