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
 [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 14:01:34 2025 UTC