php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #62129 rfc1867 crashes php even though turned off
Submitted: 2012-05-23 18:16 UTC Modified: 2013-07-16 22:16 UTC
Votes:4
Avg. Score:5.0 ± 0.0
Reproduced:4 of 4 (100.0%)
Same Version:1 (25.0%)
Same OS:2 (50.0%)
From: truth at proposaltech dot com Assigned:
Status: Closed Package: Session related
PHP Version: 5.4.3 OS: CentOS
Private report: No CVE-ID:
 [2012-05-23 18:16 UTC] truth at proposaltech dot com
Description:
------------
php_session_rfc1867_callback() tries to call php_session_rfc1867_orig_callback() even if the rfc1867 feature is not enabled.  Switching the order of the opening "if" blocks fixes the problem.  (The second "if" block immediately returns success if the feature isn't enabled and the first "if" block tries to call the callback.  This is all around line 2388 of session.c.)

I upgraded from 5.3.3 to 5.4.3 and apache was segfaulting.  I produced a core and the debugger showed infinite recursion in php_session_rfc1867_callback().  There are probably deeper problems here, but at the least the feature shouldn't cause problems if it is turned off.

In any case, even if something about my configuration is less than ideal, seg faults are Bad Thing.

Test script:
---------------
Apparently, my code wasn't executed.  I tried using Xdebug, but it didn't report anything.  dump_bt didn't return anything in the debugger.

Expected result:
----------------
I expected my code to be executed normally.

Actual result:
--------------
apache seg faulted

Patches

php-5.4.11-session-reset-global-var-in-shutdown (last revision 2013-01-27 05:03 UTC) by gxd305 at gmail dot com)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-05-24 01:45 UTC] laruence@php.net
yes, the codes seems suspicious,  maybe iliaa can look into this :)

diff --git a/ext/session/session.c b/ext/session/session.c
index 7a8199d..851e4ea 100644
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -2384,13 +2384,14 @@ static int php_session_rfc1867_callback(unsigned int 
event, void *event_data, vo
 	php_session_rfc1867_progress *progress;
 	int retval = SUCCESS;
 
-	if (php_session_rfc1867_orig_callback) {
-		retval = php_session_rfc1867_orig_callback(event, event_data, 
extra TSRMLS_CC);
-	}
 	if (!PS(rfc1867_enabled)) {
 		return retval;
 	}
 
+	if (php_session_rfc1867_orig_callback) {
+		retval = php_session_rfc1867_orig_callback(event, event_data, 
extra TSRMLS_CC);
+	}
+
 	progress = PS(rfc1867_progress);
 
 	switch(event) {
 [2012-05-24 10:17 UTC] johannes@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: iliaa
 [2012-05-24 20:48 UTC] truth at proposaltech dot com
Here is a the relevant portion of the backtrace from the seg fault:

#104648 0x00000000006cd5b8 in php_session_rfc1867_callback (event=0, 
    event_data=0x7fffb2b8f950, extra=<value optimized out>)
    at /cns/build/php-5.4.3/ext/session/session.c:2388
#104649 0x00000000006cd5b8 in php_session_rfc1867_callback (event=0, 
    event_data=0x7fffb2b8f950, extra=<value optimized out>)
    at /cns/build/php-5.4.3/ext/session/session.c:2388
#104650 0x0000000000473841 in rfc1867_post_handler (
    content_type_dup=<value optimized out>, arg=0x11535e8)
    at /cns/build/php-5.4.3/main/rfc1867.c:773
#104651 0x0000000000471372 in sapi_handle_post (arg=<value optimized out>)
    at /cns/build/php-5.4.3/main/SAPI.c:182
#104652 0x000000000067efd8 in mbstr_treat_data (arg=0, str=0x0, 
    destArray=<value optimized out>)
    at /cns/build/php-5.4.3/ext/mbstring/mb_gpc.c:98
#104653 0x0000000000475e9e in php_auto_globals_create_post (
    name=0x12a6a60 "_POST", name_len=5)
    at /cns/build/php-5.4.3/main/php_variables.c:682
#104654 0x000000000049aa4b in zend_auto_global_init (auto_global=0x110e800)
    at /cns/build/php-5.4.3/Zend/zend_compile.c:6666
#104655 0x00000000004ca974 in zend_hash_apply (ht=0x111bf20, 
    apply_func=0x49aa30 <zend_auto_global_init>)
    at /cns/build/php-5.4.3/Zend/zend_hash.c:716
#104656 0x00000000004772bb in php_hash_environment ()
    at /cns/build/php-5.4.3/main/php_variables.c:642
#104657 0x00000000004697f5 in php_request_startup ()
    at /cns/build/php-5.4.3/main/main.c:1568
#104658 0x000000000056084f in apache_php_module_main (r=<value optimized out>, 
    display_source_mode=0) at /cns/build/php-5.4.3/sapi/apache/sapi_apache.c:33
#104659 0x0000000000461c00 in send_php ()
#104660 0x0000000000461c48 in send_parsed_php ()
#104661 0x000000000085f773 in ap_invoke_handler ()
#104662 0x0000000000878d90 in process_request_internal ()
#104663 0x0000000000878df3 in ap_process_request ()
#104664 0x000000000086e46f in child_main ()
#104665 0x000000000086e728 in make_child ()
#104666 0x000000000086e7e9 in startup_children ()
#104667 0x000000000086effb in standalone_main ()
#104668 0x000000000086f8cc in main ()

I don't know much about internals, but I'll try to translate the
above based on function names and values I saw in the debugger.

While activating auto_globals, the _POST auto_global had a
callback to be called: php_auto_globals_create_post().
That used the mbstring extension, which for the case of
PARSE_POST, relies on sapi_handle_post().
sapi_handle_post() used rfc1867_post_handler()
because the sapi_globals.request_info.post_entry had 
an rfc1867 post_handler:

print *sapi_globals.request_info.post_entry
$7 = {content_type = 0x8c4bc9 "multipart/form-data", content_type_len = 19, 
  post_reader = 0, post_handler = 0x473590 <rfc1867_post_handler>}

I don't know why that post_handler value was set to
rfc1867_post_handler given that my php.ini includes
session.upload_progress.enabled = off
Similarly, I don't know why php_rfc1867_callback was 
non-null given my php.ini setting.
Once the php_rfc1867_callback() was called, everything
died quickly.  That callback calls the "orig_callback"
(if non-null) and the "orig_callback" was the same as
the php_session_rfc1867_callback - endless recursion.

Perhaps the real killer is the following lines from
session.c (~line 2195):

        php_session_rfc1867_orig_callback = php_rfc1867_callback;
        php_rfc1867_callback = php_session_rfc1867_callback;

I don't think those lines should be called if I have
session.upload_progress.enabled = off
in my php.ini.  Setting php_rfc1867_callback seems
to be what cause rfc1867 code to be called.  Setting
php_session_rfc1867_orig_callback to php_rfc1867_callback
(which is php_session_rfc1867_callback) leads to the
endless recursion.

Sorry I don't have a fix.
Thanks very much for continuing to improve php!!!
 [2013-01-19 11:26 UTC] gxd305 at gmail dot com
the other extensions have the similar problems if not set the global var to null in mshutdown after reload. 

php_session_rfc1867_orig_callback reserve the last value  after reload, instead of the default value null。

such as ice-php has the same problem, not set the global vars to null in mshutdown.

if i set the global vars ' default value explicitly or set the global vars to null in mshutdown, the problem goes away.
 [2013-01-19 13:49 UTC] gxd305 at gmail dot com
i'm sorry for the last comment for the wrong analysis.

it should be php_rfc1867_callback reserve the old value ( php_session_rfc1867_callback) after reload, and make php_session_rfc1867_callback be called recursively.
 [2013-01-27 05:18 UTC] gxd305 at gmail dot com
we should reset php_rfc1867_callback and php_session_rfc1867_orig_callback to null in MSHUTDOWN while changing them in MINIT.

with this patch
php-5.4.11-session-reset-global-var-in-shutdown
our php works fine in the production.

of course, it's not the fundamental solution
 [2013-06-24 19:11 UTC] felipe@php.net
-Status: Assigned +Status: Feedback -Assigned To: iliaa +Assigned To:
 [2013-06-24 19:11 UTC] felipe@php.net
What's the problem assigning 'php_rfc1867_callback = php_session_rfc1867_orig_callback;' on PHP_MSHUTDOWN_FUNCTION? What happened to php_session_rfc1867_orig_callback is not pointing to the callback before MINIT?
 [2013-07-16 22:16 UTC] rasmus@php.net
-Status: Feedback +Status: Open
 [2013-07-16 22:16 UTC] rasmus@php.net
felipe, the problem is that if mod_php.so doesn't actually get unloaded (usually 
because of circular references in another extension) then the 
php_rfc1867_callback global stays pointing to the previous value on the next 
MINIT which means now php_session_rfc1867_orig_callback will point to it as well 
and we are hosed. I think gxd305's approach is correct.
 [2013-07-17 10:13 UTC] arpad@php.net
Automatic comment on behalf of arraypad@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=ba3234888dfbe14dadac7ac6c403a58bc1fdd220
Log: Fix bug #62129 - rfc1867 crashes php even though turned off
 [2013-07-17 10:13 UTC] arpad@php.net
-Status: Open +Status: Closed
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Thu Apr 24 21:01:55 2014 UTC