talloc vs ASAN and other tools (Was: [PATCH] bssgp: Fix call to llist_entry in fc_queue_timer_cfg)
Hi,
On 30.04.2015 20:33, Jacob Erlbeck wrote:
On 30.04.2015 20:01, Holger Freyther wrote:
Did ASAN help?
No, since the data seems to be taken from then fc struct, so the memory area is valid. ASAN was enabled and didn't complain.
It wouldn't have helped here, but since ASAN is discussed in the context of memory errors, I'd like to check that it's been considered that ASAN's ability to detect and report memory errors may be being impaired by the use of talloc - particularly use after free bugs.
I took a very quick look at the talloc code, and it looks like the Valgrind memory poisoning macros are added, but not in a way that I can see as being active (missing #include of the Valgrind API headers + DEVELOPER needs to be defined). The 2.1.2 version of talloc looks to be perhaps more complete in this respect.
I'm not sure if ASAN can utilise the Valgrind macros though - perhaps something like Mozilla's approach to cover poisoning is needed: https://dxr.mozilla.org/mozilla-central/source/mfbt/MemoryChecking.h
Also I noticed that there is a --disable-talloc option to configure. I hoped this would fall back to libc malloc/free, but it actually disables use of the packaged talloc.c, and links with -ltalloc instead i.e. talloc isn't actually disabled:
if ENABLE_TALLOC libosmocore_la_SOURCES += talloc.c else libosmocore_la_LIBADD += -ltalloc endif
It may be better named --disable-builtin-talloc.
Kind Regards,
Mike
On 07 May 2015, at 10:40, Mike McTernan (wavemobile) mike.mcternan@wavemobile.com wrote:
Hi!
It wouldn't have helped here, but since ASAN is discussed in the context of memory errors, I'd like to check that it's been considered that ASAN's ability to detect and report memory errors may be being impaired by the use of talloc - particularly use after free bugs.
On double talloc_free I do get aborts from the libc memory management. We don’t use the pool feature (yet) so all calls to talloc_free/talloc_alloc will go directly to malloc/free and should be handled by ASAN.
holger
Why not just disable copy-pasted talloc once and for all? Corresponding patches been floating in ML for quite some time.
07.05.2015 10:40, Mike McTernan (wavemobile) пишет:
Hi,
On 30.04.2015 20:33, Jacob Erlbeck wrote:
On 30.04.2015 20:01, Holger Freyther wrote:
Did ASAN help?
No, since the data seems to be taken from then fc struct, so the memory area is valid. ASAN was enabled and didn't complain.
It wouldn't have helped here, but since ASAN is discussed in the context of memory errors, I'd like to check that it's been considered that ASAN's ability to detect and report memory errors may be being impaired by the use of talloc - particularly use after free bugs.
I took a very quick look at the talloc code, and it looks like the Valgrind memory poisoning macros are added, but not in a way that I can see as being active (missing #include of the Valgrind API headers + DEVELOPER needs to be defined). The 2.1.2 version of talloc looks to be perhaps more complete in this respect.
I'm not sure if ASAN can utilise the Valgrind macros though - perhaps something like Mozilla's approach to cover poisoning is needed: https://dxr.mozilla.org/mozilla-central/source/mfbt/MemoryChecking.h
Also I noticed that there is a --disable-talloc option to configure. I hoped this would fall back to libc malloc/free, but it actually disables use of the packaged talloc.c, and links with -ltalloc instead i.e. talloc isn't actually disabled:
if ENABLE_TALLOC libosmocore_la_SOURCES += talloc.c else libosmocore_la_LIBADD += -ltalloc endif
It may be better named --disable-builtin-talloc.
Kind Regards,
Mike
-
Holger Freyther -
Mike McTernan (wavemobile) -
☎