-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi,
See https://bugs.php.net/63595
Short, changing gmp memory allocator can cause segfaults in various
case where gnutls is used and initialized "before" gmp.
-
- php + gmp + curl => segfaults
-
- apache + mod_php + mod_gnutls + gmp => segfaults
-
- php + gmp + odbc + freetds => se'gfaults
The simple proposal will be to drop the mp_set_memory_functions call.
Any other (better) idea ?
Remi
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlQ3m0AACgkQYUppBSnxahjmJgCbBXum9FXacENC+Gwf/DDgDNvF
vxAAoOyywFiAqFK4ykqAB4gz8OZFFHQi
=uKGJ
-----END PGP SIGNATURE
On Fri, Oct 10, 2014 at 10:39 AM, Remi Collet remi@fedoraproject.org
wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1Hi,
See https://bugs.php.net/63595
Short, changing gmp memory allocator can cause segfaults in various
case where gnutls is used and initialized "before" gmp.
- php + gmp + curl => segfaults
- apache + mod_php + mod_gnutls + gmp => segfaults
- php + gmp + odbc + freetds => se'gfaults
The simple proposal will be to drop the mp_set_memory_functions call.
Any other (better) idea ?
The crashes seem to occur during shutdown. Maybe it would be sufficient to
do a mp_set_memory_functions(NULL, NULL, NULL) during gmp MSHUTDOWN?
Nikita
Short, changing gmp memory allocator can cause segfaults in various
case where gnutls is used and initialized "before" gmp.
- php + gmp + curl => segfaults
- apache + mod_php + mod_gnutls + gmp => segfaults
- php + gmp + odbc + freetds => se'gfaults
The simple proposal will be to drop the mp_set_memory_functions call.
Any other (better) idea ?
Can't we just make gnutls use its own (statically-linked?) gmp instance?
I'd rather not make gmp just use malloc. Would cause memory leaks.
Andrea Faulds
http://ajf.me/
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Le 10/10/2014 12:57, Andrea Faulds a écrit :
On 10 Oct 2014, at 09:39, Remi Collet remi@fedoraproject.org
wrote:Short, changing gmp memory allocator can cause segfaults in
various case where gnutls is used and initialized "before" gmp.
- php + gmp + curl => segfaults - - apache + mod_php +
mod_gnutls + gmp => segfaults - - php + gmp + odbc + freetds =>
se'gfaultsThe simple proposal will be to drop the mp_set_memory_functions
call.Any other (better) idea ?
Can't we just make gnutls use its own (statically-linked?) gmp
instance?
Not an option.
As gnutls change will imply downtream change,
and downstream doesn't allowed bundled library.
I'd rather not make gmp just use malloc. Would cause memory leaks.
Can you please elaborate a little ?
Doesn't such leak have to be fixed in ext/gmp ?
Remi.
-- Andrea Faulds http://ajf.me/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlQ3zEgACgkQYUppBSnxahhLOwCfaZ2OlRuYaDAyfyERnUceXO5z
gWIAoM9XmaZFMtKdKe56aQ2aPOOOq9/G
=XbLF
-----END PGP SIGNATURE
Hi!
Can you please elaborate a little ?
Doesn't such leak have to be fixed in ext/gmp ?
We clean up the memory at the end of the request. But if the request
ended abnormally and we had some active gmp objects, with gmp data
allocated outside emalloc we could not clean that and it ends up being
persistent memory leak. Also, of course, that also excepts gmp objects
from PHP memory limit, which makes it less useful.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
Hi!
Can you please elaborate a little ?
Doesn't such leak have to be fixed in ext/gmp ?We clean up the memory at the end of the request. But if the request
ended abnormally and we had some active gmp objects, with gmp data
allocated outside emalloc we could not clean that and it ends up being
persistent memory leak. Also, of course, that also excepts gmp objects
from PHP memory limit, which makes it less useful.
This causes a particular problem for me with my bigint patch, were it to ever make it into master. It sets gmp to use emalloc as its allocators (much like ext/gmp). This means it obeys memory limits. Being able to circumvent the memory limit and DoS the server by doing a stupidly large exponentiation would be quite scary.
--
Andrea Faulds
http://ajf.me/
Hi!
Can you please elaborate a little ?
Doesn't such leak have to be fixed in ext/gmp ?We clean up the memory at the end of the request. But if the request
ended abnormally and we had some active gmp objects, with gmp data
allocated outside emalloc we could not clean that and it ends up being
persistent memory leak. Also, of course, that also excepts gmp objects
from PHP memory limit, which makes it less useful.
At the same time we can't just switch libgmp to our allocator in MINIT
since it gets called by other libraries outside of a PHP request as Remi
pointed out via Apache's mod_gnutls. And also from within a PHP request
but indirectly via freetds+gnutls where freetds has no idea we have
switched the allocator on them.
We need to make it more granular. gmp_init() should set it and set a
flag and then in RSHUTDOWN we can check that flag and restore the
default allocator to take care of the outside-request access, and for
inside-request access like the freetds extension, we probably need to
explicitly set it back to the default handler in that extension in each
call that might call it. Huge PITA to catch all those cases though.
-Rasmus
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Le 11/10/2014 02:24, Rasmus Lerdorf a écrit :
Hi!
Can you please elaborate a little ? Doesn't such leak have to
be fixed in ext/gmp ?We clean up the memory at the end of the request. But if the
request ended abnormally and we had some active gmp objects, with
gmp data allocated outside emalloc we could not clean that and it
ends up being persistent memory leak. Also, of course, that also
excepts gmp objects from PHP memory limit, which makes it less
useful.At the same time we can't just switch libgmp to our allocator in
MINIT since it gets called by other libraries outside of a PHP
request as Remi pointed out via Apache's mod_gnutls. And also from
within a PHP request but indirectly via freetds+gnutls where
freetds has no idea we have switched the allocator on them.We need to make it more granular. gmp_init() should set it and set
a flag and then in RSHUTDOWN we can check that flag and restore
the default allocator to take care of the outside-request access,
and for inside-request access like the freetds extension, we
probably need to explicitly set it back to the default handler in
that extension in each call that might call it. Huge PITA to catch
all those cases though.
We cannot, as we are not always aware of it (in case of odbc =>
freetds => gnutls)
I think the only place we are aware of the need of our allocator is in
our gmp extension, which means
-
- beginning of each gmp* function
- save current allocators => mp_get_memory_functions
- set ours => mp_set_memory_functions
-
- end of end function
- restore allocators => mp_set_memory_functions
Yes, and probably lower perfs (but perhaps not significantly)
I will try to create a patch, for test, as we really need to fix this
problem, especially it gmp going to go into core (bigint patch).
Remi.
-Rasmus
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlQ4whQACgkQYUppBSnxahiOPQCgwqFgYYTFp1sTFHyd0zaZuYwg
yqQAoMY3fAghfHjk+OFMabTIyuv1oiGF
=vH1h
-----END PGP SIGNATURE
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Le 11/10/2014 07:37, Remi Collet a écrit :
I will try to create a patch, for test, as we really need to fix
this problem, especially it gmp going to go into core (bigint
patch).
See
https://bugs.php.net/patch-display.php?bug=63595&patch=gmp-memory.patch&revision=1413008196
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlQ4y4EACgkQYUppBSnxahiudgCfe9kOa/K54kfRORT/I6JNnTma
sNYAoJwAL1SkdKELrLijFaOYeY67+RGC
=4Cec
-----END PGP SIGNATURE
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Le 11/10/2014 08:17, Remi Collet a écrit :
Le 11/10/2014 07:37, Remi Collet a écrit :
I will try to create a patch, for test, as we really need to fix
this problem, especially it gmp going to go into core (bigint
patch).See
https://bugs.php.net/patch-display.php?bug=63595&patch=gmp-memory.patch&revision=1413008196
Sorry,
use
https://bugs.php.net/patch-display.php?bug_id=63595&patch=gmp-memory.patch&revision=latest
(against 5.6.1)
Test suite result is unchanged
(I have tests/gmp_remroot.phpt failed [1] with or without the patch)
Execution time of the test suite seems not significantly changed.
Remi.
P.S. $ cat tests/gmp_remroot.diff
036+ string(3) "-36"
036- string(2) "36"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlQ412cACgkQYUppBSnxahhhBgCfV0Vw7LiwizkuwFDBVivc0meX
hVEAoNL4dVP4ZjDs56qpOPIlehlbJ7ZL
=ZIQy
-----END PGP SIGNATURE
Le 10/10/2014 12:57, Andrea Faulds a écrit :
Can't we just make gnutls use its own (statically-linked?) gmp
instance?Not an option.
As gnutls change will imply downtream change,
and downstream doesn't allowed bundled library.
Hmm. Is there a way to force the dynamically-linked library not to be shared then?
Andrea Faulds
http://ajf.me/
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Le 10/10/2014 10:39, Remi Collet a écrit :
Hi,
See https://bugs.php.net/63595
Short, changing gmp memory allocator can cause segfaults in
various case where gnutls is used and initialized "before" gmp.
- php + gmp + curl => segfaults - apache + mod_php + mod_gnutls +
gmp => segfaults - php + gmp + odbc + freetds => segfaultsThe simple proposal will be to drop the mp_set_memory_functions
call.
As this seems to be the only sane solution,
I plan to fix 5.5+ in the next days (before next RC).
Remi.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlRF9rEACgkQYUppBSnxahi89QCg0qIhYjcgL8JeIuLap52zeuHl
COgAoO/8tWvamKVqktZWYh5h0Dhwd5Wr
=W5g2
-----END PGP SIGNATURE