php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #60339 valgrind reports LEAK --with-mm
Submitted: 2011-11-19 17:30 UTC Modified: 2011-11-19 18:51 UTC
From: yohgaki at ohgaki dot net Assigned: felipe (profile)
Status: Closed Package: Session related
PHP Version: 5.4.0RC1 OS: Linux
Private report: No CVE-ID: None
View Developer Edit
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: yohgaki at ohgaki dot net
New email:
PHP Version: OS:

 

 [2011-11-19 17:30 UTC] yohgaki at ohgaki dot net 
Description:
------------
Valgrind reports LEAKs --with-mm
(php-src HEAD is affected, too. I noticed this while I'm making strict session 
patch.)


=====================================================================
LEAKED TEST SUMMARY
---------------------------------------------------------------------
session rfc1867 [ext/session/tests/rfc1867.phpt]
session rfc1867 [ext/session/tests/rfc1867_cleanup.phpt]
session rfc1867 disabled [ext/session/tests/rfc1867_disabled.phpt]
session rfc1867 disabled 2 [ext/session/tests/rfc1867_disabled_2.phpt]
session rfc1867 [ext/session/tests/rfc1867_inter.phpt]
session rfc1867 no name [ext/session/tests/rfc1867_no_name.phpt]
session rfc1867 sid cookie [ext/session/tests/rfc1867_sid_cookie.phpt]
session rfc1867 sid get [ext/session/tests/rfc1867_sid_get.phpt]
session rfc1867 sid get 2 [ext/session/tests/rfc1867_sid_get_2.phpt]
session rfc1867 sid cookie [ext/session/tests/rfc1867_sid_invalid.phpt]
session rfc1867 sid only cookie [ext/session/tests/rfc1867_sid_only_cookie.phpt]
session rfc1867 sid only cookie 2 
[ext/session/tests/rfc1867_sid_only_cookie_2.phpt]
session rfc1867 sid post [ext/session/tests/rfc1867_sid_post.phpt]
Test session_module_name() function : variation 
[ext/session/tests/session_module_name_variation3.phpt]
Test session_name() function : error functionality 
[ext/session/tests/session_name_basic.phpt]
Test session_name() function : error functionality 
[ext/session/tests/session_name_error.phpt]
Test session_name() function : variation 
[ext/session/tests/session_name_variation1.phpt]
Test session_save_path() function : basic functionality 
[ext/session/tests/session_save_path_basic.phpt]
Test session_save_path() function : error functionality 
[ext/session/tests/session_save_path_error.phpt]
Test session_save_path() function : variation 
[ext/session/tests/session_save_path_variation1.phpt]
Test session_save_path() function : variation 
[ext/session/tests/session_save_path_variation4.phpt]
Test session_save_path() function : variation 
[ext/session/tests/session_save_path_variation5.phpt]
Test session_set_save_handler() function : basic functionality 
[ext/session/tests/session_set_save_handler_basic.phpt]
Test session_set_save_handler() function : using closures as callbacks 
[ext/session/tests/session_set_save_handler_closures.phpt]
Test session_set_save_handler() function : error functionality 
[ext/session/tests/session_set_save_handler_error3.phpt]
Test session_set_save_handler() function : variation 
[ext/session/tests/session_set_save_handler_variation4.phpt]
=====================================================================


All LEAK reports are the same.

******* ext/session/tests/rfc1867.mem ************
==29422== Invalid read of size 1
==29422==    at 0x57D8D3: zm_startup_ps_mm (mod_mm.c:281)
==29422==    by 0x578A94: zm_startup_session (session.c:2178)
==29422==    by 0x6784D4: zend_startup_module_ex (zend_API.c:1653)
==29422==    by 0x681304: zend_hash_apply (zend_hash.c:716)
==29422==    by 0x67BB15: zend_startup_modules (zend_API.c:1780)
==29422==    by 0x61C27B: php_module_startup (main.c:2132)
==29422==    by 0x718F02: php_cgi_startup (cgi_main.c:931)
==29422==    by 0x7195D2: main (cgi_main.c:1873)
==29422==  Address 0x4fa31af is 1 bytes before a block of size 1 alloc'd
==29422==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==29422==    by 0x6516AD: zend_strndup (zend_alloc.c:2617)
==29422==    by 0x62361B: php_ini_parser_cb (php_ini.c:274)
==29422==    by 0x64B84E: ini_parse (zend_ini_parser.c:1669)
==29422==    by 0x64BAED: zend_parse_ini_string (zend_ini_parser.c:348)
==29422==    by 0x622B44: php_init_config (php_ini.c:722)
==29422==    by 0x61C1C9: php_module_startup (main.c:2078)
==29422==    by 0x718F02: php_cgi_startup (cgi_main.c:931)
==29422==    by 0x7195D2: main (cgi_main.c:1873)
==29422==


Test script:
---------------
N/A

Expected result:
----------------
No LEAKs

Actual result:
--------------
LEAKs are reported

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2011-11-19 17:53 UTC] yohgaki at ohgaki dot net 
Adding extra byte to OnUpdateSaveDir() in session.c removes LEAKs

@@ -607,7 +613,7 @@
 		}
 	}
 
-	OnUpdateString(entry, new_value, new_value_length, mh_arg1, mh_arg2, 
mh_arg3, stage TSRMLS_CC);
+	OnUpdateString(entry, new_value, new_value_length+1, mh_arg1, mh_arg2, 
mh_arg3, stage TSRMLS_CC);
 	return SUCCESS;
 }
 /* }}} */

but it does not seem to be a correct fix for this.
 [2011-11-19 17:59 UTC] yohgaki at ohgaki dot net 
I figured out the cause. This is simple underflow. The correct patch is this.

--- mod_mm.c	(リビジョン 319529)
+++ mod_mm.c	(作業コピー)
@@ -278,7 +278,7 @@
 	ps_mm_path = emalloc(save_path_len + 1 + (sizeof(PS_MM_FILE) - 1) + 
mod_name_len + euid_len + 1);
 
 	memcpy(ps_mm_path, PS(save_path), save_path_len);
-	if (PS(save_path)[save_path_len - 1] != DEFAULT_SLASH) {
+	if (save_path_len && PS(save_path)[save_path_len - 1] != DEFAULT_SLASH) 
{
 		ps_mm_path[save_path_len] = DEFAULT_SLASH;
 		save_path_len++;
 	}
 [2011-11-19 18:10 UTC] felipe@php.net 
I can't reproduce it, are you using any .INI?

=====================================================================
PHP         : sapi/cli/php 
PHP_SAPI    : cli
PHP_VERSION : 5.4.0RC2-dev
ZEND_VERSION: 2.4.0
PHP_OS      : Linux - Linux sig11 2.6.32-5-amd64 #1 SMP Mon Oct 3 03:59:20 UTC 2011 x86_64
INI actual  : /home/felipe/dev/php5_4
More .INIs  :  
CWD         : /home/felipe/dev/php5_4
Extra dirs  : 
VALGRIND    : valgrind-3.6.0.SVN-Debian
=====================================================================
 [2011-11-19 18:20 UTC] yohgaki at ohgaki dot net 
Since my tree is full of changes for strict session patch.
Could anyone commit patch for this bug?

Patch should be applied to php-src, php-src-5.4 and php-src-5.3.

I'm not sure if this bug is exploitable with current memory manager. Since 
DEFAULT_SLASH would be ascii 47 or 97, it would be difficult.
 [2011-11-19 18:30 UTC] yohgaki at ohgaki dot net 
I've tested as follows.

(from my bash history)
 1004  tar zxvf ../Download/php-5.4.0RC1.tar.bz2 
 1005  cd php-5.4.0RC1/
 1006  ./configure --with-mm && make -j 8 
 1007  TEST_PHP_EXECUTABLE="./sapi/cli/php" ./run-tests.php -m ext/session/

Felipe, if you could commit the patch, I appreciated it. 

If you take a look at PHP_MINIT_FUNCTION(ps_mm) in ext/session/mod_mm.c, you'll 
see it will underflow by 1 byte when strlen(PS(save_path)) equals 0.
 [2011-11-19 18:50 UTC] felipe@php.net 
Automatic comment from SVN on behalf of felipe
Revision: http://svn.php.net/viewvc/?view=revision&revision=319553
Log: - Fixed bug #60339 (valgrind reports LEAK --with-mm)
  patch by: yohgaki at ohgaki dot net
 [2011-11-19 18:51 UTC] felipe@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: felipe
 [2011-11-19 18:51 UTC] felipe@php.net 
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.

I can reproduce it. :P Patch committed, thanks!
 [2012-04-18 09:47 UTC] laruence@php.net 
Automatic comment on behalf of felipe
Revision: http://git.php.net/?p=php-src.git;a=commit;h=276518ff174a8d8a416cef8ada75dd803ae6591c
Log: - Fixed bug #60339 (valgrind reports LEAK --with-mm)   patch by: yohgaki at ohgaki dot net
 [2012-07-24 23:38 UTC] rasmus@php.net 
Automatic comment on behalf of felipe
Revision: http://git.php.net/?p=php-src.git;a=commit;h=276518ff174a8d8a416cef8ada75dd803ae6591c
Log: - Fixed bug #60339 (valgrind reports LEAK --with-mm)   patch by: yohgaki at ohgaki dot net
 [2013-11-17 09:35 UTC] laruence@php.net 
Automatic comment on behalf of felipe
Revision: http://git.php.net/?p=php-src.git;a=commit;h=276518ff174a8d8a416cef8ada75dd803ae6591c
Log: - Fixed bug #60339 (valgrind reports LEAK --with-mm)   patch by: yohgaki at ohgaki dot net
 
PHP Copyright © 2001-2025 The PHP Group
All rights reserved. 		Last updated: Thu Jul 03 15:01:34 2025 UTC