[libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment

Nikolay Shirokovskiy posted 2 patches 7 years, 5 months ago
[libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
Posted by Nikolay Shirokovskiy 7 years, 5 months ago
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.
---
 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
Re: [libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
Posted by Daniel P. Berrange 7 years, 5 months ago
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
Re: [libvirt] [PATCH 1/2] tests: commandtest: handle tcmalloc hacking environment
Posted by Nikolay Shirokovskiy 7 years, 5 months ago

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
Posted by Daniel P. Berrange 7 years, 5 months ago
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
Posted by Nikolay Shirokovskiy 7 years, 5 months ago

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
Posted by Daniel P. Berrange 7 years, 5 months ago
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
Posted by Nikolay Shirokovskiy 7 years, 5 months ago

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
Posted by Daniel P. Berrange 7 years, 5 months ago
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