Re: [PHP-DEV] free deadlock in timeout signal handler

2013-09-19 Thread Lazy
2013/9/18 Ángel González keis...@gmail.com:
 On 13/09/13 22:10, Lazy wrote:

 Hello internals,

 I'm trying to fix deadlock in an ancient php 5.2.17, php hangs on
 internal libc lock.
  From my understanding free is not safe to use in a signal handler, and
 this seems to be the issue here.

 No, it's not.


 http://marc.info/?l=php-internalsm=121999390109071w=2 seems to
 address this issue but it's not present in 5.3 or later releases.

 Very similar deadlock but with time() instead of free(),
 https://bugs.php.net/bug.php?id=31749

 Are you sure it's with time() ? I do see a free() in that call stack (and no
 time),
 as I would expect, as time() is required by POSIX.1-2004 to be
 Async-signal-safe.

time() refers to  https://bugs.php.net/bug.php?id=31749, You are right
this deadlock is related to free()

 Current php also uses free() in php_error_cb() and external
 destructors in list_entry_destructor() aren't protected by
 HANDLE_BLOCK_INTERRUPTIONS() (which seems to be a noop in fastcgi
 mode), so I suspect 5.5 may also contain this deadlock.

 main/main.c
 php_error_cb()
  835 if (display) {
  836 if (PG(last_error_message)) {
  837 free(PG(last_error_message));
 ...^^ deadlock if previous free was
 interrupted by a timeout signal
  845 PG(last_error_type) = type;
  846 PG(last_error_message) = strdup(buffer);

 I'm thinking about fixing this by leaking memory pointed by
 PG(last_error_message) if php called when a timeout is pending
 (timeouts are usually very rare, php processes will eventually be
 restarted so this little memory waste won't have time to make any
 impact.

 Is this issue fixed in modern php ? If so I would be grateful for some
 information about the way it was done. This would save me a lot of
 time trying to trigger
 a non existing confition.

 I will try to reproduce this in a modern version of php.

 It probably isn't. PG(last_error_message) is only modified in main.c, I
 would try to use a separate arena for PG(last_error_message) and
 PG(last_error_file). For instance it could be a static buffer reused during
 the whole execution and extended with mmap(2) in the unlikely case it turns
 out to be too small. I suspect it would also make the error handler faster,
 as it would avoid the free() + malloc()

thank You I didn't notice that it is used only there, i will try to
use a static buffer.

I managed to produce a segfault on current php version (php heap
corruption), bug report https://bugs.php.net/bug.php?id=65674
spprintf() allocating memory is also not safe. I will try to fix this
by using a static buffer.

Thanks,

Michal Grzedzicki

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] free deadlock in timeout signal handler

2013-09-18 Thread Ángel González

On 13/09/13 22:10, Lazy wrote:

Hello internals,

I'm trying to fix deadlock in an ancient php 5.2.17, php hangs on
internal libc lock.
 From my understanding free is not safe to use in a signal handler, and
this seems to be the issue here.

No, it's not.


http://marc.info/?l=php-internalsm=121999390109071w=2 seems to
address this issue but it's not present in 5.3 or later releases.

Very similar deadlock but with time() instead of free(),
https://bugs.php.net/bug.php?id=31749
Are you sure it's with time() ? I do see a free() in that call stack 
(and no time),
as I would expect, as time() is required by POSIX.1-2004 to be 
Async-signal-safe.



Current php also uses free() in php_error_cb() and external
destructors in list_entry_destructor() aren't protected by
HANDLE_BLOCK_INTERRUPTIONS() (which seems to be a noop in fastcgi
mode), so I suspect 5.5 may also contain this deadlock.

main/main.c
php_error_cb()
 835 if (display) {
 836 if (PG(last_error_message)) {
 837 free(PG(last_error_message));
...^^ deadlock if previous free was
interrupted by a timeout signal
 845 PG(last_error_type) = type;
 846 PG(last_error_message) = strdup(buffer);

I'm thinking about fixing this by leaking memory pointed by
PG(last_error_message) if php called when a timeout is pending
(timeouts are usually very rare, php processes will eventually be
restarted so this little memory waste won't have time to make any
impact.

Is this issue fixed in modern php ? If so I would be grateful for some
information about the way it was done. This would save me a lot of
time trying to trigger
a non existing confition.

I will try to reproduce this in a modern version of php.


It probably isn't. PG(last_error_message) is only modified in main.c, I 
would try to use a separate arena for PG(last_error_message) and 
PG(last_error_file). For instance it could be a static buffer reused 
during the whole execution and extended with mmap(2) in the unlikely 
case it turns out to be too small. I suspect it would also make the 
error handler faster, as it would avoid the free() + malloc()



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-DEV] free deadlock in timeout signal handler

2013-09-13 Thread Lazy
Hello internals,

I'm trying to fix deadlock in an ancient php 5.2.17, php hangs on
internal libc lock.

Backtrace follows

#0  0x030b555024cb in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x030b554986b8 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x030b55496aa1 in free () from /lib/x86_64-linux-gnu/libc.so.6
#3  0x00734989 in php_error_cb (type=1,
error_filename=0x100013bbf18 xxx, error_lineno=7, format=optimized
out,
   args=optimized out) at main/main.c:837
#4  0x00623e15 in soap_error_handler (error_num=1,
error_filename=0x100013bbf18 /class.cbase.php, error_lineno=7,
   format=0xa749c0 Maximum execution time of %d second%s exceeded,
args=0x3a2de4d3fa0) at //ext/soap/soap.c:2115
#5  0x00777066 in zend_error (type=1, format=0xa749c0 Maximum
execution time of %d second%s exceeded) at /Zend/zend.c:976
#6  signal handler called
#7  0x030b55493302 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#8  0x030b55496aac in free () from /lib/x86_64-linux-gnu/libc.so.6
#9  0x030b57f817c5 in free_root () from
/usr/lib/x86_64-linux-gnu/libmysqlclient.so.18
#10 0x030b57f65a34 in free_rows () from
/usr/lib/x86_64-linux-gnu/libmysqlclient.so.18
#11 0x030b57f6620d in mysql_free_result () from
/usr/lib/x86_64-linux-gnu/libmysqlclient.so.18
#12 0x0058cd1c in _free_mysql_result (rsrc=optimized out) at
ext/mysql/php_mysql.c:275
#13 0x0078340e in list_entry_destructor (ptr=0x5074760) at
Zend/zend_list.c:184

From my understanding free is not safe to use in a signal handler, and
this seems to be the issue here.

http://marc.info/?l=php-internalsm=121999390109071w=2 seems to
address this issue but it's not present in 5.3 or later releases.

Very similar deadlock but with time() instead of free(),
https://bugs.php.net/bug.php?id=31749

Current php also uses free() in php_error_cb() and external
destructors in list_entry_destructor() aren't protected by
HANDLE_BLOCK_INTERRUPTIONS() (which seems to be a noop in fastcgi
mode), so I suspect 5.5 may also contain this deadlock.

main/main.c
php_error_cb()
835 if (display) {
836 if (PG(last_error_message)) {
837 free(PG(last_error_message));
...^^ deadlock if previous free was
interrupted by a timeout signal
845 PG(last_error_type) = type;
846 PG(last_error_message) = strdup(buffer);

I'm thinking about fixing this by leaking memory pointed by
PG(last_error_message) if php called when a timeout is pending
(timeouts are usually very rare, php processes will eventually be
restarted so this little memory waste won't have time to make any
impact.

Is this issue fixed in modern php ? If so I would be grateful for some
information about the way it was done. This would save me a lot of
time trying to trigger
a non existing confition.

I will try to reproduce this in a modern version of php.

Thanks,

Michal Grzedzicki

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php