php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #46222 Memory issues in ArrayObject
Submitted: 2008-10-03 03:57 UTC Modified: 2008-10-05 14:22 UTC
Votes:10
Avg. Score:4.3 ± 0.8
Reproduced:7 of 8 (87.5%)
Same Version:3 (42.9%)
Same OS:1 (14.3%)
From: rasmus@php.net Assigned: colder
Status: Closed Package: Scripting Engine problem
PHP Version: 5.2.6 OS: Any
Private report: No CVE-ID:
 [2008-10-03 03:57 UTC] rasmus@php.net
Description:
------------
ArrayObject appears to corrupt the symbol table

Checked it with Valgrind and didn't see anything, but haven't checked the code yet.  Appears to be a problem in both 5.2 and 5.3.

Reproduce code:
---------------
$test = new ArrayObject(); 
$test['d1']['d2'] = 'hello'; 
print_r($test3['mmmmm']);

Expected result:
----------------
nothing

Actual result:
--------------
Array
(
    [d2] => hello
)


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2008-10-03 04:47 UTC] gwynne@php.net
Ran it under:
PHP 5.3.0alpha3-dev (cli) (built: Oct  3 2008 00:09:28) (DEBUG)
Copyright (c) 1997-2008 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2008 Zend Technologies

Got:
Array
(
    [d2] => hello
)
[Fri Oct  3 00:19:06 2008]  Script:  '-'
/Users/gwynne/src/php-src/cvs/php-5.3/ext/spl/spl_array.c(354) :  Freeing 0x00C1419C (20 bytes), script=-
[Fri Oct  3 00:19:06 2008]  Script:  '-'
/Users/gwynne/src/php-src/cvs/php-5.3/Zend/zend_execute.c(932) :  Freeing 0x00C142B8 (44 bytes), script=-
/Users/gwynne/src/php-src/cvs/php-5.3/Zend/zend_API.c(930) : Actual location (location was relayed)
Last leak repeated 1 time
[Fri Oct  3 00:19:06 2008]  Script:  '-'
/Users/gwynne/src/php-src/cvs/php-5.3/Zend/zend_hash.c(247) :  Freeing 0x00C14364 (38 bytes), script=-
[Fri Oct  3 00:19:06 2008]  Script:  '-'
/Users/gwynne/src/php-src/cvs/php-5.3/Zend/zend_execute.c(727) :  Freeing 0x00C143BC (20 bytes), script=-
[Fri Oct  3 00:19:06 2008]  Script:  '-'
/Users/gwynne/src/php-src/cvs/php-5.3/Zend/zend_variables.h(45) :  Freeing 0x00C14400 (6 bytes), script=-
/Users/gwynne/src/php-src/cvs/php-5.3/Zend/zend_variables.c(120) : Actual location (location was relayed)
=== Total 6 memory leaks detected ===
 [2008-10-03 13:13 UTC] karsten at typo3 dot org
Ran it under:
PHP 5.3.0alpha1 (cli) (built: Sep  8 2008 13:16:52) 
Copyright (c) 1997-2008 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2008 Zend Technologies
    with Xdebug v2.0.3, Copyright (c) 2002-2007, by Derick Rethans

(installed via MacPorts on 10.5.5)

Got:
Array
(
    [d2] => hello
)
 [2008-10-04 20:29 UTC] mark at hell dot ne dot jp
I searched a bit too and found that we overwrite the memory space for:

EG(uninitialized_zval_ptr)

In spl/spl_array.c near line 375 (probably wrong as I added lots of debug to find out where this came from).

Basically this code :

    if (Z_REFCOUNT_PP(ret) > 1) {
      zval *newval;

      /* Separate */
      MAKE_STD_ZVAL(newval);
      *newval = **ret;
      zval_copy_ctor(newval);
      Z_SET_REFCOUNT_P(newval, 1);

      /* Replace */
      Z_DELREF_PP(ret);
      *ret = newval;
    }

Will be run in some cases with ret == &EG(uninitialized_zval_ptr), which means writing to *ret is the same as writing to EG(uninitialized_zval_ptr).

I checked how new (unexisting) array entries are managed in Zend and will try to replace some code here to see if it helps.
 [2008-10-04 20:57 UTC] mark at hell dot ne dot jp
Changing the previously mentionned code to add one line :

    if (Z_REFCOUNT_PP(ret) > 1) {
      zval *newval;

      /* Separate */
      MAKE_STD_ZVAL(newval);
      *newval = **ret;
      zval_copy_ctor(newval);
      Z_SET_REFCOUNT_P(newval, 1);

      /* Replace */
      Z_DELREF_PP(ret);
      ret = emalloc(sizeof(void*)); // Avoid overwritting stuff??
      *ret = newval;
    }

Seems to fix the initial problem (dumping anything else no longer breaks stuff; I didn't care about memleaks in this "fix", it's just to demonstrate that this part of code has problems), however I see something else coming: dumping $test in initial code shows that it stays empty even after assigning $test['d1']['d2'].

I guess the real problem is probably something like the fact $test['d1'] didn't get created, so d2 gets lost in "outer space".
 [2008-10-05 03:46 UTC] crrodriguez at opensuse dot org
Not sure, but this patch works for me


Index: ext/spl/spl_array.c
===================================================================
RCS file: /repository/php-src/ext/spl/spl_array.c,v
retrieving revision 1.71.2.17.2.13.2.26
diff -u -p -r1.71.2.17.2.13.2.26 spl_array.c
--- ext/spl/spl_array.c 29 Sep 2008 22:45:27 -0000      1.71.2.17.2.13.2.26
+++ ext/spl/spl_array.c 5 Oct 2008 03:44:42 -0000
@@ -346,7 +346,7 @@ static zval *spl_array_read_dimension_ex
        /* When in a write context,
         * ZE has to be fooled into thinking this is in a reference set
         * by separating (if necessary) and returning as an is_ref=1 zval (even if refcount == 1) */
-       if ((type == BP_VAR_W || type == BP_VAR_RW) && !Z_ISREF_PP(ret)) {
+       if (ret != &EG(uninitialized_zval_ptr) && (type == BP_VAR_W || type == BP_VAR_RW) && !Z_ISREF_PP(ret)) {
                if (Z_REFCOUNT_PP(ret) > 1) {
                        zval *newval;
 [2008-10-05 05:38 UTC] mark at hell dot ne dot jp
The patch by crrodriguez doesn't fix the main problem: If you print_r (or var_dump in my case) $test, it's empty.

This fixes the overwritting of EG(uninitialized_zval_ptr), but not the behaviour problem.

I suggest updating the reproduce code as following:

Reproduce code:
---------------

$test = new ArrayObject(); 
$test['d1']['d2'] = 'hello'; 
var_dump($test, $test3['mmmmm']);

Expected result (php5.3):
-------------------------
object(ArrayObject)#1 (1) {
  ["storage":"ArrayObject":private]=>
  array(1) {
    ["d1"]=>
    array(1) {
      ["d2"]=>
      string(5) "hello"
    }
  }
}
NULL

Actual result (php5.3):
-----------------------
object(ArrayObject)#1 (1) {
  ["storage":"ArrayObject":private]=>
  array(0) {
  }
}
array(1) {
  ["d2"]=>
  string(5) "hello"
}

Actual result with crrodriguez's patch:
object(ArrayObject)#1 (1) {
  ["storage":"ArrayObject":private]=>
  array(0) {
  }
}
NULL
 [2008-10-05 06:05 UTC] mark at hell dot ne dot jp
Adding the following code in spl_array.c near line 350 seems to fix the problem (however it's a bit dirty, I think).

After line :

  ret = spl_array_get_dimension_ptr_ptr(check_inherited, object, offset, type TSRMLS_CC);

Add :

  // Dirty test patch
  if ((type == BP_VAR_W || type == BP_VAR_RW) && (ret == &EG(uninitialized_zval_ptr))) {
    // write a NULL value in the missing part of the array
    spl_array_write_dimension_ex(check_inherited, object, offset, EG(uninitialized_zval_ptr) TSRMLS_CC);

    // And reload it
    ret = spl_array_get_dimension_ptr_ptr(check_inherited, object, offset, type TSRMLS_CC);
  }

This fixes this bug, but I don't think this is the best way to fix it. I just make things how they should be at a later point in code, I don't know the Zend engine internals enough (and the documentation's empty skeleton isn't helping much) to make a better fix than that.
 [2008-10-05 14:22 UTC] colder@php.net
This bug has been fixed in CVS.

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/.
 
Thank you for the report, and for helping us make PHP better.

Fixed by initializing zvals in dimension_get in case:
1) it doesn't exist yet
2) it is a write operation.

Thanks for the suggestions/patches
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Sun Apr 20 15:01:54 2014 UTC