avoid use strdup() in libevent

[thefallentree]

strdup() allocates memory through internal malloc, which can't be masked by allocator like jemalloc, and will crash the program if being static linked.

libevent right now has a wrapper that it only uses if mm_malloc_fn is set, but it should always use it instead.

[azat]

and will crash the program if being static linked.

When what is statically linked, libevent/glibc/jemalloc?

The only problem here is that if glibc will be statically linked, but I doubt that you have these, plus jemalloc will not be able to replace event malloc in this case.

Can you provide an example of what fails for you?

[thefallentree]

The problem happens when jemalloc is staticly linked, not glibc is statically linked . Because strdup uses Malloc internally, there is no way for jemalloc to hijack that.

…

On Sat, Sep 18, 2021 at 17:37 Azat Khuzhin ***@***.***> wrote: and will crash the program if being static linked. When what is statically linked, libevent/glibc/jemalloc? The only problem here is that if glibc will be statically linked, but I doubt that you have these, plus jemalloc will not be able to replace event malloc in this case. Can you provide an example of what fails for you? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1202 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJSYEHP3X3J2OCIKWRUHDDUCUBANANCNFSM5EFOPXQA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[ploxiln]

If you dynamically link both jemalloc and glibc, or statically link both jemalloc and glibc, in the correct order, it should work correctly. If you statically link one and dynamically link the other ... I don't know the correct way to do that, I think that is a fragile use-case. There are other libc functions that may call malloc().

[thefallentree]

There is no safe way to static link glibc as far as I know, the problem is simple and constraint: we need to use a custom strdup , let’s not get hang up on pedantic universal solutions.

…

On Sun, Sep 19, 2021 at 01:00 Pierce Lopez ***@***.***> wrote: If you dynamically link both jemalloc and glibc, or statically link both jemalloc and glibc, in the correct order, it should work correctly. If you statically link one and dynamically link the other ... I don't know the correct way to do that, I think that is a fragile use-case. There are other libc functions that may call malloc(). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1202 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJSYEGVAJHIUOWTK4N4FDLUCVU6FANCNFSM5EFOPXQA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[azat]

If you statically link one and dynamically link the other ... I don't know the correct way to do that

@ploxiln this should easily work.

Because strdup uses Malloc internally, there is no way for jemalloc to hijack that.

This does not changes anything, strdup internally uses the same malloc (via PLT) that jemalloc intercepts.
Here is a minimal example

/* Minimal exapmle of static jemalloc.
 *
 * $ gcc -g3 -o main main.cpp -Wl,-Bstatic -ljemalloc_pic -Wl,-Bdynamic -nodefaultlibs -lc -lpthread -ldl -lgcc
 * (or if you have only libjemalloc.a then -ljemalloc)
 * NOTE: order is important
 *
 * $ ldd main
 *         linux-vdso.so.1 (0x00007ffff7fcb000)
 *         libc.so.6 => /usr/lib/libc.so.6 (0x00007ffff7b22000)
 *         libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007ffff7b01000)
 *         libdl.so.2 => /usr/lib/libdl.so.2 (0x00007ffff7afa000)
 *         /lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007ffff7fcd000)
 *
 * $ gdb -q -ex start -ex 'b malloc' -ex c -ex bt ./main
 *
 * Breakpoint 2, malloc (size=9) at tsd.h:341
 * 341     tsd.h: No such file or directory.
 * #0  malloc (size=9) at tsd.h:341
 * #1  0x00007ffff7e6534f in strdup () from /usr/lib/libc.so.6
 * #2  0x000055555555a200 in main () at main.cpp:16
 *
 * NOTE: if you configured jemalloc with prefix,
 * than take a look at malloc hooks example [1].
 *
 *   [1]: http://jemalloc.net/mailman/jemalloc-discuss/2013-July/000617.html
 */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main()
{
	char *body = strdup("It works");
	puts(body);
	free(body);

	return 0;
}

Also glibc provide hooks for malloc/free

[thefallentree]

The reason I reported this is that my applicaiton when linking with both jemalloc and libevmt will cause libevent crash on strdup, unless I added an strdup replacement. Yeah some strdup implementation maybe able to be intercepted by jemalloc, but at least not on OS X and mingw. Since there are no performance loss on using an local strdup (the implementation is so small) I don’t understand the reluctance.

…

On Sun, Sep 19, 2021 at 08:38 Azat Khuzhin ***@***.***> wrote: Closed #1202 <#1202>. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1202 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJSYECTHJPAH4MY2HLNRT3UCXKUJANCNFSM5EFOPXQA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[azat]

Yeah some strdup implementation maybe able to be intercepted by jemalloc,
but at least not on OS X and mingw.

On OSX you need register jemalloc allocator, and also look at jemalloc/jemalloc#708 (for the possible issues when linking jemalloc static)

Since there are no performance loss on using an local strdup (the
implementation is so small) I don’t understand the reluctance.

Some people have "optimized" version of strdup and overwriting it just because it is simple it not the correct choice.

You should use event_set_mem_functions explicitly if you want to override it.

[ploxiln]

@ploxiln this should easily work

Indeed it does! I assumed that the proper way was probably more esoteric/complicated, since some quick googling showed that ruby and rust developers have been struggling with this in years past ...

There is no safe way to static link glibc as far as I know

It's only problematic if you use name resolution functions, or pthreads it seems, these really want to load plugins dynamically ... but it seems that even libevent_core references getaddrinfo() stuff, and jemalloc references pthread stuff (makes sense), so that's true in this case: one would need musl-libc or uclibc to link with libevent, jemalloc, and libc all statically. But I've written some simple tools, and even tiny tcp servers, which could be linked statically with glibc. fwiw.

[ploxiln]

interesting side-note:

https://github.com/ClickHouse/ClickHouse/blob/55cf857aba10044eb5d6e00892869e1c1e543f7f/CMakeLists.txt#L610-L624

        # In case of static jemalloc, because zone_register() is located in zone.c and
        # is never used outside (it is declared as constructor) it is omitted
        # by the linker, and so jemalloc will not be registered as system
        # allocator under osx [1], and clickhouse will SIGSEGV.
        #
        #   [1]: https://github.com/jemalloc/jemalloc/issues/708
        #
        # About symbol name:
        # - _zone_register not zone_register due to Mach-O binary format,
        # - _je_zone_register due to JEMALLOC_PRIVATE_NAMESPACE=je_ under OS X.
        # - but jemalloc-cmake does not run private_namespace.sh
        #   so symbol name should be _zone_register
        if (ENABLE_JEMALLOC AND MAKE_STATIC_LIBRARIES AND OS_DARWIN)
            set_property(TARGET ${target} APPEND PROPERTY LINK_OPTIONS -u_zone_register)
        endif()

(ah nvm that's what azat already linked to)

[thefallentree]

My experience is different than above, the only thing that static link jemalloc & libevent is strdup(). Once I added a simple replacement strdup in my own application code, everything works fine.

I don't think the zone_register issue is still a issue