Php8.1 with alternative malloc allocators

[ryancinsight]

Description

The following code:

LD_PRELOAD=libsnmallocshim.so php cron.php

Resulted in this output:

FREE():  invalid pointer
Aborted

But I expected this output instead:

111111111

more info is here:
microsoft/snmalloc#595

microsoft/mimalloc#377

I’m not an allocator expert but seems valgrind is showing memory leaks with alternative allocators and free is being called by malloc when replaced with LD_PRELOAD

PHP Version

PHP 8.1

Operating System

Debian Bullseye

[ryancinsight]

To clarify further I am using docker container and cron.php is for Nextcloud though same error occurs just writing “php” alone.

[nielsdos]

Executing php without extensions does not trigger the bug.
I can reproduce it if I enable the right extensions, so the problem is in the extensions and not in the core.
I found that enabling any of the following 3 extensions breaks php for me using snmalloc:

• curl
• mysqli
• zip

I'll now spend some time creating a debug build of PHP with these extensions to track down the bug.
EDIT: but only reproducible when the extensions are built as shared objects 🤔
EDIT2: it's probably all the extensions, it's just that the above three do some cleanup that calls to free upon the initialization, so that's why the error is triggered immediately.

Backtrace for curl:

#0  0x00007ffff7a068ec in ?? () from /usr/lib/libc.so.6
#1  0x00007ffff79b7ea8 in raise () from /usr/lib/libc.so.6
#2  0x00007ffff79a153d in abort () from /usr/lib/libc.so.6
#3  0x00007ffff79a229e in ?? () from /usr/lib/libc.so.6
#4  0x00007ffff7a10657 in ?? () from /usr/lib/libc.so.6
#5  0x00007ffff7a124bc in ?? () from /usr/lib/libc.so.6
#6  0x00007ffff7a14e63 in free () from /usr/lib/libc.so.6
#7  0x00007ffff7f7a7f2 in zend_string_release (s=0x7fbff4601b00) at /home/niels/php-src/Zend/zend_string.h:322
#8  0x00007ffff7f7ab42 in register_class_CURLFile () at /home/niels/php-src/ext/curl/curl_file_arginfo.h:69
#9  0x00007ffff7f7bd47 in curlfile_register_class () at /home/niels/php-src/ext/curl/curl_file.c:148
#10 0x00007ffff7f6b338 in zm_startup_curl (type=1, module_number=27) at /home/niels/php-src/ext/curl/interface.c:1212
#11 0x0000555555abcfc8 in zend_startup_module_ex (module=0x7fbff446a940) at /home/niels/php-src/Zend/zend_API.c:2199
#12 0x0000555555abd026 in zend_startup_module_zval (zv=0x7fbff4464440) at /home/niels/php-src/Zend/zend_API.c:2214
#13 0x0000555555ace385 in zend_hash_apply (ht=0x555556a21220 <module_registry>, 
    apply_func=0x555555abd003 <zend_startup_module_zval>) at /home/niels/php-src/Zend/zend_hash.c:1872
#14 0x0000555555abd669 in zend_startup_modules () at /home/niels/php-src/Zend/zend_API.c:2325
#15 0x0000555555a0fb3f in php_module_startup (sf=0x555556a07160 <cli_sapi_module>, additional_modules=0x0, num_additional_modules=0)
    at /home/niels/php-src/main/main.c:2260
#16 0x0000555555c295ff in php_cli_startup (sapi_module=0x555556a07160 <cli_sapi_module>)
    at /home/niels/php-src/sapi/cli/php_cli.c:409
#17 0x0000555555c2b7dd in main (argc=4, argv=0x7fbff44501b0) at /home/niels/php-src/sapi/cli/php_cli.c:1334

We can see that the freeing happens with the libc allocator indeed instead of snmalloc. So when the extension gets loaded the LD_PRELOAD isn't respected.

[nielsdos]

I identified the problem.
The problem is that extensions are loaded through dlopen, and dlopen does not respect LD_PRELOAD with the DEEPBIND option.

[nielsdos]

Removing RTLD_DEEPBIND from the DL_LOAD macro in Zend/zend_portability.h fixes this issue for me.
I don't know why we're using RTLD_DEEPBIND, but there's probably a good reason for it.

[nielsdos]

The flag was introduced by commit 601140cbe9a so I don't think we can easily get rid of it.

[NathanFreeman]

Maybe you can try the follow command to test it.

export USE_ZEND_ALLOC=0 && LD_PRELOAD=/your-path/libsnmallocshim.so

php -v

[nielsdos]

Maybe you can try the follow command to test it.

export USE_ZEND_ALLOC=0 && LD_PRELOAD=/your-path/libsnmallocshim.so

php -v

That won't work AFAIK, because it does not influence the dlopen flag. So that means the LD_PRELOAD has no effect on the extension and hence the libc allocator will be used instead of snmalloc.

[mjp41]

I believe I have fixed snmalloc to interact correctly with RTLD_DEEPBIND, based on the approach taken in jemalloc:

microsoft/snmalloc#598

@nielsdos if you still have the set up you were using to debug this issues, would you be able to check the latest main on snmalloc fixes it for you?

[nielsdos]

@mjp41 thanks for looking into this. I checked but it does not seem to fix the issue. I still get the invalid free message, and looking at the gdb backtrace it still calls free in libc.so.6

[nielsdos]

It actually looks like those hooks were deprecated and maybe it does not work because they are already removed in the glibc version I use (didnt check this)

[devnexen]

It actually looks like those hooks were deprecated and maybe it does not work because they are already removed in the glibc version I use (didnt check this)

You re right they are, the Mesh folks have been removed those.

[mjp41]

It actually looks like those hooks were deprecated and maybe it does not work because they are already removed in the glibc version I use (didnt check this)

This explains why some people observed jemalloc working and some didn't on the mimalloc issue.

@devnexen I don't think jemalloc is seeing the deprecation warning that Mesh had due to providing its own prototypes rather than include malloc.h:

https://github.com/jemalloc/jemalloc/blob/09e4b38fb1f9a9b505e35ac13b8f99282990bc2c/src/jemalloc.c#L3122-L3140

@interwq, @davidtgoldblatt, FYI: As glibc is updated the current approach you use to RTLD_DEEPBIND is going to stop working.

[cgzones]

Rebuilding php without the RTLD_DEEPBIND flag seems to work with hardened_malloc at least.

[nielsdos]

Rebuilding php without the RTLD_DEEPBIND flag seems to work with hardened_malloc at least.

Thanks for confirming this.

I don't really know what we should do with this issue. It's not really a bug in PHP, but rather a limitation in one of the ways alternative allocators inject themselves into applications.
And the current workaround (i.e. the glibc hook) doesn't work on the most recent glibc versions unfortunately.

[cgzones]

The introducing commit 601140cbe9a from 2005 mentions API constraints, but not whether they origin from PHP itself or third party libraries. Maybe @notroj remembers?

I would suggest adding a new php.ini directive, which controls what dlopen(3) flag to use, with a default of use RTLD_DEEPBIND if available.
Then users could toggle that at runtime (instead of rebuilding php) to run and test custom allocators.
Also developers could test if it is actually still necessary for some symbol resolution.
Then if no regressions are found in php 8.4 one could try to change the default to dont't use RTLD_DEEPBIND.

[notroj]

The problem was extensions loading third-party libraries which used common names, IIRC there were a couple of common libraries using global symbols like "hash_create" or something like that? I can't remember exactly which ones caused me to submit the PHP change, and my mail archive doesn't go back that far either - sorry.

We still see this kind of issue occasionally on still-supported systems (not just 2005-era distributions!), but switching to using php-fpm has helped since it isolates PHP-loaded libraries from httpd-loaded libraries.

I would suggest adding a new php.ini directive, which controls what dlopen(3) flag to use, with a default of use RTLD_DEEPBIND if available.

I'd agree with this, we patch httpd on RHEL to allow doing loading modules using RTLD_DEEPBIND if an environment variable is set.

[cgzones]

Initial draft:

(I am unsure whether the flag belongs in _zend_compiler_globals or _zend_executor_globals.)

From: =?utf-8?q?Christian_G=C3=B6ttsche?= <[email protected]>
Date: Thu, 13 Apr 2023 12:28:34 +0200
Subject: Add zend.dlopen_deepbind php.ini directive

---
 Zend/zend.c             | 1 +
 Zend/zend_globals.h     | 2 ++
 Zend/zend_portability.h | 2 +-
 php.ini-development     | 6 ++++++
 php.ini-production      | 6 ++++++
 5 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Zend/zend.c b/Zend/zend.c
index 33e1c4d..68fdfb2 100644
--- a/Zend/zend.c
+++ b/Zend/zend.c
@@ -194,6 +194,7 @@ static ZEND_INI_MH(OnUpdateFiberStackSize) /* {{{ */
 ZEND_INI_BEGIN()
 	ZEND_INI_ENTRY("error_reporting",				NULL,		ZEND_INI_ALL,		OnUpdateErrorReporting)
 	STD_ZEND_INI_ENTRY("zend.assertions",				"1",    ZEND_INI_ALL,       OnUpdateAssertions,           assertions,   zend_executor_globals,  executor_globals)
+	STD_ZEND_INI_BOOLEAN("zend.dlopen_deepbind",		"1",	ZEND_INI_SYSTEM,	OnUpdateBool, dlopen_deepbind, zend_compiler_globals, compiler_globals)
 	ZEND_INI_ENTRY3_EX("zend.enable_gc",				"1",	ZEND_INI_ALL,		OnUpdateGCEnabled, NULL, NULL, NULL, zend_gc_enabled_displayer_cb)
 	STD_ZEND_INI_BOOLEAN("zend.multibyte", "0", ZEND_INI_PERDIR, OnUpdateBool, multibyte,      zend_compiler_globals, compiler_globals)
 	ZEND_INI_ENTRY("zend.script_encoding",			NULL,		ZEND_INI_ALL,		OnUpdateScriptEncoding)
diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h
index 368127b..f1a9e02 100644
--- a/Zend/zend_globals.h
+++ b/Zend/zend_globals.h
@@ -92,6 +92,8 @@ struct _zend_compiler_globals {
 
 	bool ini_parser_unbuffered_errors;
 
+	bool dlopen_deepbind;
+
 	zend_llist open_files;
 
 	struct _zend_ini_parser_param *ini_parser_param;
diff --git a/Zend/zend_portability.h b/Zend/zend_portability.h
index d59fc6d..3bea2d7 100644
--- a/Zend/zend_portability.h
+++ b/Zend/zend_portability.h
@@ -153,7 +153,7 @@
 # if defined(RTLD_GROUP) && defined(RTLD_WORLD) && defined(RTLD_PARENT)
 #  define DL_LOAD(libname)			dlopen(libname, PHP_RTLD_MODE | RTLD_GLOBAL | RTLD_GROUP | RTLD_WORLD | RTLD_PARENT)
 # elif defined(RTLD_DEEPBIND) && !defined(__SANITIZE_ADDRESS__) && !__has_feature(memory_sanitizer)
-#  define DL_LOAD(libname)			dlopen(libname, PHP_RTLD_MODE | RTLD_GLOBAL | RTLD_DEEPBIND)
+#  define DL_LOAD(libname)			dlopen(libname, PHP_RTLD_MODE | RTLD_GLOBAL | (CG(dlopen_deepbind) ? RTLD_DEEPBIND : 0))
 # else
 #  define DL_LOAD(libname)			dlopen(libname, PHP_RTLD_MODE | RTLD_GLOBAL)
 # endif
diff --git a/php.ini-development b/php.ini-development
index 1565f6b..2239d92 100644
--- a/php.ini-development
+++ b/php.ini-development
@@ -392,6 +392,12 @@ zend.exception_ignore_args = Off
 ; Production Value: 0
 zend.exception_string_param_max_len = 15
 
+; Decides whether to use the dlopen(3) flag RTLD_DEEPBIND, if availalable, when
+; loading shared libraries.  This ensures symbol lookup will prefer symbols from
+; the shared object itself over global ones.  Conflicts with the use of custom
+; memory allocators.
+zend.dlopen_deepbind = On
+
 ;;;;;;;;;;;;;;;;;
 ; Miscellaneous ;
 ;;;;;;;;;;;;;;;;;
diff --git a/php.ini-production b/php.ini-production
index 6cb3f1a..ada04d6 100644
--- a/php.ini-production
+++ b/php.ini-production
@@ -388,6 +388,12 @@ zend.exception_ignore_args = On
 ; of sensitive information in stack traces.
 zend.exception_string_param_max_len = 0
 
+; Decides whether to use the dlopen(3) flag RTLD_DEEPBIND, if availalable, when
+; loading shared libraries.  This ensures symbol lookup will prefer symbols from
+; the shared object itself over global ones.  Conflicts with the use of custom
+; memory allocators.
+zend.dlopen_deepbind = On
+
 ;;;;;;;;;;;;;;;;;
 ; Miscellaneous ;
 ;;;;;;;;;;;;;;;;;