cynara icon indicating copy to clipboard operation
cynara copied to clipboard

cynara-test depends on random initialization of global instances

Open pohly opened this issue 10 years ago • 5 comments

When cross-compiled with gcc 4.9 using the meta-framework-security bitbake recipes [1], cynara-test immediately segfaults before even reaching main(). I suspect that PolicyKeyFeature::m_wildcardValue (a global instance) when k_nun (another global) gets created (_M_p = 0x0 in m_wildcardValue).

Program received signal SIGSEGV, Segmentation fault. 0x080c0f94 in operator== (__rhs=..., __lhs=...) at /work/build/iot/x86/tmp-glibc/sysroots/qemux86/usr/include/c++/4.9.2/bits/basic_string.h:2516 2516 /work/build/iot/x86/tmp-glibc/sysroots/qemux86/usr/include/c++/4.9.2/bits/basic_string.h: No such file or directory. (gdb) where #0 0x080c0f94 in operator== (__rhs=..., __lhs=...)

at /work/build/iot/x86/tmp-glibc/sysroots/qemux86/usr/include/c++/4.9.2/bits/basic_string.h:2516

#1 Cynara::PolicyKeyFeature::PolicyKeyFeature (this=0xbffffcb8, value=...)

at /usr/src/debug/cynara/0.8.0+gitAUTOINC+b4cb3d8e86-r0/git/src/common/types/PolicyKey.h:89

#2 0x08070a55 in create (value=...)

at /usr/src/debug/cynara/0.8.0+gitAUTOINC+b4cb3d8e86-r0/git/src/common/types/PolicyKey.h:48

#3 __static_initialization_and_destruction_0 (__priority=65535,

__initialize_p=1)
at /usr/src/debug/cynara/0.8.0+gitAUTOINC+b4cb3d8e86-r0/git/test/common/protocols/TestDataCollection.h:40

#4 0x081e2ae4 in __do_global_ctors_aux () #5 0x0805ebc0 in _init () #6 0x494d8b0c in ?? () from /usr/lib/libstdc++.so.6

Backtrace stopped: previous frame inner to this frame (corrupt stack?) (gdb) up #1 Cynara::PolicyKeyFeature::PolicyKeyFeature (this=0xbffffcb8, value=...)

at /usr/src/debug/cynara/0.8.0+gitAUTOINC+b4cb3d8e86-r0/git/src/common/types/PolicyKey.h:89

89 m_isWildcard(value == PolicyKeyFeature::m_wildcardValue), (gdb) p value $1 = (const Cynara::PolicyKeyFeature::ValueType &) @0xbffffca4: { static npos = , _M_dataplus = {std::allocator = {<__gnu_cxx::new_allocator> = {<No data fields>}, <No data fields>}, _M_p = 0x827ee8c ""}} (gdb) up #2 0x08070a55 in create (value=...)

at /usr/src/debug/cynara/0.8.0+gitAUTOINC+b4cb3d8e86-r0/git/src/common/types/PolicyKey.h:48

48 return PolicyKeyFeature(value); (gdb) up #3 __static_initialization_and_destruction_0 (__priority=65535,

__initialize_p=1)
at /usr/src/debug/cynara/0.8.0+gitAUTOINC+b4cb3d8e86-r0/git/test/common/protocols/TestDataCollection.h:40

40 static const Cynara::PolicyKey k_nun(PKF::create(""), PKF::create("u"), PKF::create("")); (gdb) p PolicyKeyFeature::m_wildcardValue $1 = {static npos = , _M_dataplus = {std::allocator = {<__gnu_cxx::new_allocator> = {<No data fields>}, <No data fields>}, _M_p = 0x0}}

[1] https://github.com/01org/meta-intel-iot-security (recipe with cynara-test enabled to be published there soon)

pohly avatar May 29 '15 10:05 pohly

m_wildcardValue was just the tip of the iceberg. Several other global instances had similar issues. Some of that code is also used in cynarad, so this issue might go beyond just testing.

For the fixes that were good enough for me at the moment, see: https://github.com/pohly/meta-intel-iot-security/blob/cynara-tests/meta-security-framework/recipes-security/cynara/cynara/PolicyKeyFeature-avoid-complex-global-constants.patch https://github.com/pohly/meta-intel-iot-security/blob/cynara-tests/meta-security-framework/recipes-security/cynara/cynara/globals-avoid-copying-other-globals.patch

However, you might want to do a more thorough code review and avoid this kind of issue completely. I only fixed the cases that really caused segfaults.

pohly avatar May 29 '15 12:05 pohly

Thanks for finding these issues and for the patches. Yes, we need to get rid of direct std::string -> std::string dependencies. First patch looks good to me. The second patch if I understand static variable initialization correctly makes it less likely to fail, because there are less variables that we depend on. However, we still rely on variable being referenced to be initialized first (references can point to variable that have not yet been initialized). I think that all std::string constants should depend on: a) C string literals/variables or b) wrapper functions like in the first patch

JacquesBurac avatar May 29 '15 16:05 JacquesBurac

The second patch if I understand static variable initialization correctly makes it less likely to fail, because there are less variables that we depend on. However, we still rely on variable being referenced to be initialized first (references can point to variable that have not yet been initialized).

Correct. It's just mitigating the problem without solving it properly.

I agree with the proposed solution, but won't have time to implement it, I'm afraid.

pohly avatar Jun 02 '15 09:06 pohly

Ok, I understand. Right now I'm working on some urgent task, but I should finish soon and I'll be able to take care of it.

JacquesBurac avatar Jun 02 '15 09:06 JacquesBurac

I've pushed your first patch to gerrit: https://review.tizen.org/gerrit/#/c/41353

And also made some more changes: https://review.tizen.org/gerrit/#/c/41354

JacquesBurac avatar Jun 12 '15 15:06 JacquesBurac