Re: [libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
On Fri, Nov 17, 2017 at 04:53:13PM +0300, Nikolay Shirokovskiy wrote: > > > On 17.11.2017 16:47, Daniel P. Berrange wrote: > > On Fri, Nov 17, 2017 at 04:45:27PM +0300, Nikolay Shirokovskiy wrote: > > Ah I see. I think this smells like a bug in the tests/Makefile.am > > Ahh, this is fixed upstream already by eae746b2d the way you suggest )) Hahahah, i totally forgot that it was me who fixed it too :-) Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
On 17.11.2017 16:47, Daniel P. Berrange wrote: > On Fri, Nov 17, 2017 at 04:45:27PM +0300, Nikolay Shirokovskiy wrote: >> >> >> On 17.11.2017 16:40, Daniel P. Berrange wrote: >>> On Fri, Nov 17, 2017 at 04:31:13PM +0300, Nikolay Shirokovskiy wrote: On 17.11.2017 16:24, Daniel P. Berrange wrote: > On Fri, Nov 17, 2017 at 04:17:37PM +0300, Nikolay Shirokovskiy wrote: >> If one of the libraries is compiled with tcmalloc then >> the latter will add GLIBCPP_FORCE_NEW and GLIBCXX_FORCE_NEW to >> environment at startup and thus break commandtest. > > How are they getting those envs into our environment after we clean it > out ? We strongly aim to prevent any non-whitelisted env variable > leakage into children we spawn, so I would really like to kill these > env vars instead of changin the test. They inserted at process startup I guess [1]. They are cleared out by commandtest but visible in commandhelper. >>> >>> Hmm, so is comandhelper getting linked to tcmalloc by mistake then ? >>> If so, how easy is it to stop it being linked >> >> It is not liked directly. In my case the chain is >> libdevmapper.so -> libudev.so -> libtcmalloc.so. It is distro specific >> but I guess other can step across this issue and for a different chain. One >> just need to link on of the libraries libvirt uses to tcmalloc. > > Ah I see. I think this smells like a bug in the tests/Makefile.am Ahh, this is fixed upstream already by eae746b2d the way you suggest )) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
On Fri, Nov 17, 2017 at 04:45:27PM +0300, Nikolay Shirokovskiy wrote: > > > On 17.11.2017 16:40, Daniel P. Berrange wrote: > > On Fri, Nov 17, 2017 at 04:31:13PM +0300, Nikolay Shirokovskiy wrote: > >> > >> > >> On 17.11.2017 16:24, Daniel P. Berrange wrote: > >>> On Fri, Nov 17, 2017 at 04:17:37PM +0300, Nikolay Shirokovskiy wrote: > If one of the libraries is compiled with tcmalloc then > the latter will add GLIBCPP_FORCE_NEW and GLIBCXX_FORCE_NEW to > environment at startup and thus break commandtest. > >>> > >>> How are they getting those envs into our environment after we clean it > >>> out ? We strongly aim to prevent any non-whitelisted env variable > >>> leakage into children we spawn, so I would really like to kill these > >>> env vars instead of changin the test. > >> > >> They inserted at process startup I guess [1]. They are cleared out by > >> commandtest > >> but visible in commandhelper. > > > > Hmm, so is comandhelper getting linked to tcmalloc by mistake then ? > > If so, how easy is it to stop it being linked > > It is not liked directly. In my case the chain is > libdevmapper.so -> libudev.so -> libtcmalloc.so. It is distro specific > but I guess other can step across this issue and for a different chain. One > just need to link on of the libraries libvirt uses to tcmalloc. Ah I see. I think this smells like a bug in the tests/Makefile.am The commandhelper binary should not link to anything at all except for libc (and perhaps gnulib, but possibly even that is redundant) Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
On 17.11.2017 16:40, Daniel P. Berrange wrote: > On Fri, Nov 17, 2017 at 04:31:13PM +0300, Nikolay Shirokovskiy wrote: >> >> >> On 17.11.2017 16:24, Daniel P. Berrange wrote: >>> On Fri, Nov 17, 2017 at 04:17:37PM +0300, Nikolay Shirokovskiy wrote: If one of the libraries is compiled with tcmalloc then the latter will add GLIBCPP_FORCE_NEW and GLIBCXX_FORCE_NEW to environment at startup and thus break commandtest. >>> >>> How are they getting those envs into our environment after we clean it >>> out ? We strongly aim to prevent any non-whitelisted env variable >>> leakage into children we spawn, so I would really like to kill these >>> env vars instead of changin the test. >> >> They inserted at process startup I guess [1]. They are cleared out by >> commandtest >> but visible in commandhelper. > > Hmm, so is comandhelper getting linked to tcmalloc by mistake then ? > If so, how easy is it to stop it being linked It is not liked directly. In my case the chain is libdevmapper.so -> libudev.so -> libtcmalloc.so. It is distro specific but I guess other can step across this issue and for a different chain. One just need to link on of the libraries libvirt uses to tcmalloc. > >> >> [1] >> https://github.com/gperftools/gperftools/blob/6e3a702fb9c86eb450f22b326ecbceef4b0d6604/src/malloc_extension.cc#L85 >> >>> --- tests/commandhelper.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 1da2834..0f6ce07 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -94,8 +94,15 @@ int main(int argc, char **argv) { for (i = 0; i < n; i++) { /* Ignore the variables used to instruct the loader into * behaving differently, as they could throw the tests off. */ -if (!STRPREFIX(newenv[i], "LD_")) -fprintf(log, "ENV:%s\n", newenv[i]); +if (STRPREFIX(newenv[i], "LD_")) +continue; + +/* Fix tests if tcmalloc is used in libraries */ +if (STRPREFIX(newenv[i], "GLIBCPP_FORCE_NEW=") || +STRPREFIX(newenv[i], "GLIBCXX_FORCE_NEW=")) +continue; + +fprintf(log, "ENV:%s\n", newenv[i]); } open_max = sysconf(_SC_OPEN_MAX); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list >>> >>> Regards, >>> Daniel >>> > > Regards, > Daniel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
On Fri, Nov 17, 2017 at 04:31:13PM +0300, Nikolay Shirokovskiy wrote: > > > On 17.11.2017 16:24, Daniel P. Berrange wrote: > > On Fri, Nov 17, 2017 at 04:17:37PM +0300, Nikolay Shirokovskiy wrote: > >> If one of the libraries is compiled with tcmalloc then > >> the latter will add GLIBCPP_FORCE_NEW and GLIBCXX_FORCE_NEW to > >> environment at startup and thus break commandtest. > > > > How are they getting those envs into our environment after we clean it > > out ? We strongly aim to prevent any non-whitelisted env variable > > leakage into children we spawn, so I would really like to kill these > > env vars instead of changin the test. > > They inserted at process startup I guess [1]. They are cleared out by > commandtest > but visible in commandhelper. Hmm, so is comandhelper getting linked to tcmalloc by mistake then ? If so, how easy is it to stop it being linked > > [1] > https://github.com/gperftools/gperftools/blob/6e3a702fb9c86eb450f22b326ecbceef4b0d6604/src/malloc_extension.cc#L85 > > > > >> --- > >> tests/commandhelper.c | 11 +-- > >> 1 file changed, 9 insertions(+), 2 deletions(-) > >> > >> diff --git a/tests/commandhelper.c b/tests/commandhelper.c > >> index 1da2834..0f6ce07 100644 > >> --- a/tests/commandhelper.c > >> +++ b/tests/commandhelper.c > >> @@ -94,8 +94,15 @@ int main(int argc, char **argv) { > >> for (i = 0; i < n; i++) { > >> /* Ignore the variables used to instruct the loader into > >> * behaving differently, as they could throw the tests off. */ > >> -if (!STRPREFIX(newenv[i], "LD_")) > >> -fprintf(log, "ENV:%s\n", newenv[i]); > >> +if (STRPREFIX(newenv[i], "LD_")) > >> +continue; > >> + > >> +/* Fix tests if tcmalloc is used in libraries */ > >> +if (STRPREFIX(newenv[i], "GLIBCPP_FORCE_NEW=") || > >> +STRPREFIX(newenv[i], "GLIBCXX_FORCE_NEW=")) > >> +continue; > >> + > >> +fprintf(log, "ENV:%s\n", newenv[i]); > >> } > >> > >> open_max = sysconf(_SC_OPEN_MAX); > >> -- > >> 1.8.3.1 > >> > >> -- > >> libvir-list mailing list > >> libvir-list@redhat.com > >> https://www.redhat.com/mailman/listinfo/libvir-list > > > > Regards, > > Daniel > > Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
On 17.11.2017 16:24, Daniel P. Berrange wrote: > On Fri, Nov 17, 2017 at 04:17:37PM +0300, Nikolay Shirokovskiy wrote: >> If one of the libraries is compiled with tcmalloc then >> the latter will add GLIBCPP_FORCE_NEW and GLIBCXX_FORCE_NEW to >> environment at startup and thus break commandtest. > > How are they getting those envs into our environment after we clean it > out ? We strongly aim to prevent any non-whitelisted env variable > leakage into children we spawn, so I would really like to kill these > env vars instead of changin the test. They inserted at process startup I guess [1]. They are cleared out by commandtest but visible in commandhelper. [1] https://github.com/gperftools/gperftools/blob/6e3a702fb9c86eb450f22b326ecbceef4b0d6604/src/malloc_extension.cc#L85 > >> --- >> tests/commandhelper.c | 11 +-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/tests/commandhelper.c b/tests/commandhelper.c >> index 1da2834..0f6ce07 100644 >> --- a/tests/commandhelper.c >> +++ b/tests/commandhelper.c >> @@ -94,8 +94,15 @@ int main(int argc, char **argv) { >> for (i = 0; i < n; i++) { >> /* Ignore the variables used to instruct the loader into >> * behaving differently, as they could throw the tests off. */ >> -if (!STRPREFIX(newenv[i], "LD_")) >> -fprintf(log, "ENV:%s\n", newenv[i]); >> +if (STRPREFIX(newenv[i], "LD_")) >> +continue; >> + >> +/* Fix tests if tcmalloc is used in libraries */ >> +if (STRPREFIX(newenv[i], "GLIBCPP_FORCE_NEW=") || >> +STRPREFIX(newenv[i], "GLIBCXX_FORCE_NEW=")) >> +continue; >> + >> +fprintf(log, "ENV:%s\n", newenv[i]); >> } >> >> open_max = sysconf(_SC_OPEN_MAX); >> -- >> 1.8.3.1 >> >> -- >> libvir-list mailing list >> libvir-list@redhat.com >> https://www.redhat.com/mailman/listinfo/libvir-list > > Regards, > Daniel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
On Fri, Nov 17, 2017 at 04:17:37PM +0300, Nikolay Shirokovskiy wrote: > If one of the libraries is compiled with tcmalloc then > the latter will add GLIBCPP_FORCE_NEW and GLIBCXX_FORCE_NEW to > environment at startup and thus break commandtest. How are they getting those envs into our environment after we clean it out ? We strongly aim to prevent any non-whitelisted env variable leakage into children we spawn, so I would really like to kill these env vars instead of changin the test. > --- > tests/commandhelper.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tests/commandhelper.c b/tests/commandhelper.c > index 1da2834..0f6ce07 100644 > --- a/tests/commandhelper.c > +++ b/tests/commandhelper.c > @@ -94,8 +94,15 @@ int main(int argc, char **argv) { > for (i = 0; i < n; i++) { > /* Ignore the variables used to instruct the loader into > * behaving differently, as they could throw the tests off. */ > -if (!STRPREFIX(newenv[i], "LD_")) > -fprintf(log, "ENV:%s\n", newenv[i]); > +if (STRPREFIX(newenv[i], "LD_")) > +continue; > + > +/* Fix tests if tcmalloc is used in libraries */ > +if (STRPREFIX(newenv[i], "GLIBCPP_FORCE_NEW=") || > +STRPREFIX(newenv[i], "GLIBCXX_FORCE_NEW=")) > +continue; > + > +fprintf(log, "ENV:%s\n", newenv[i]); > } > > open_max = sysconf(_SC_OPEN_MAX); > -- > 1.8.3.1 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list