Skip to content

Add zend.dlopen_deepbind php.ini directive #11094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cgzones
Copy link

@cgzones cgzones commented Apr 17, 2023

Add a runtime configuration option to control whether loading shared libraries via dlopen(3) uses the GNU extension flag RTLD_DEEPBIND, if available.

The flag ensures symbol lookup will prefer symbols from the shared object itself over global ones, which can resolve symbol namespace collisions in third party dependencies, see 601140c ("New versions of glibc support a RTLD_DEEPBIND flag to dlopen. The").

However using this flag symbols from preloaded libraries might be de- prioritized, e.g. free(3) from a preloaded allocator by the standard libc. This results in one allocator allocating memory and another one deallocating it, triggering double/invalid-free assertions.

Closes: #10670

Add a runtime configuration option to control whether loading shared
libraries via dlopen(3) uses the GNU extension flag RTLD_DEEPBIND, if
available.

The flag ensures symbol lookup will prefer symbols from the shared
object itself over global ones, which can resolve symbol namespace
collisions in third party dependencies, see 601140c ("New versions
of glibc support a RTLD_DEEPBIND flag to dlopen. The").

However using this flag symbols from preloaded libraries might be de-
prioritized, e.g. free(3) from a preloaded allocator by the standard
libc.  This results in one allocator allocating memory and another one
deallocating it, triggering double/invalid-free assertions.

Closes: php#10670
@dstogov
Copy link
Member

dstogov commented Apr 17, 2023

I'm not sure why we needed RTLD_DEEPBIND at the first place.
It was committed at 5.0 times by Jani Taskinen, I'm not sure who is Joe Orton.

I would try to remove it completely.

@nielsdos
Copy link
Member

The reason was explained by Joe here: #10670 (comment)
So I wonder if removing this can break things.

@dstogov
Copy link
Member

dstogov commented Apr 17, 2023

The reason was explained by Joe here: #10670 (comment) So I wonder if removing this can break things.

I see. We might keep RTLD_DEEPBIND only for Apache SAPI or reuse the same environment variable.
Anyway, this may be too risky, especially for third party extensions.
Giving the control to user is also risky :(

May be we should use RTLD_LOCAL instead of RTLD_GLOBAL.

@dstogov
Copy link
Member

dstogov commented Apr 17, 2023

I just afraid of this new option in user hands.
We already have --enable-rtld-now configure option.
It's better to use similar approach for RTLD_DEEPBIND.

@cgzones
Copy link
Author

cgzones commented Apr 20, 2023

The issue for me with a configure option is that distributions probably are not (or only very inert) going to adapt it. And the difference between rebuilding php to specify an additional configure option or apply a curom patch is rather cosmetic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Php8.1 with alternative malloc allocators
3 participants