Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] [Bug #61649] zend gc should not mark persistent hashtable

Posted by Laruence 
Hi Dmitry:

zend gc was introducted in 5.3

thinking of a zval which is a Hashtable allocated by a extension in persistent,
and it also has hashtable children in it,

then , if the extension return this to php script:

array_init(return_value);
zend_hash_copy(Z_ARRVAL_P(return_value), Z_ARRVAL_P(persitent_zval_hashtable),
***)..

since zval_copy_ctor does shallow copy, so the persistent array return to the
php
script.


then if it happen to be parsed by zval_ptr_dtor, then the persistent array will
be
parsed by gc_zval_possible_root,

ZEND_API void gc_zval_possible_root(zval *zv TSRMLS_DC)
{

...................

if (GC_ZVAL_GET_COLOR(zv) != GC_PURPLE) {
GC_ZVAL_SET_PURPLE(zv);
...................

then the malloc info of the block(not sure before or after) will be polluted.

then when the extension try to free the block, a warning will be show like:

munmap_chunk(): invalid pointer 0x*******


I have make a patch for this(https://bugs.php.net/bug.php?id=61649),
if you think it's okey, I will commit it to all branches,

thanks

--
Laruence  Xinchen Hui
http://www.laruence.com/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Hi:
and, yes, extension can change their code to alloc a zend_gc_info
size(what I do now).

however, I think, it's no need for zend_gc care about persistent
memorys, right?

thanks


On Fri, Apr 6, 2012 at 8:21 PM, Laruence <[email protected]> wrote:
> Hi Dmitry:
>
>   zend gc was introducted in 5.3
>
> thinking of a zval which is a Hashtable allocated by a extension in persistent,
> and it also has hashtable children in it,
>
> then , if the extension return this to php script:
>
> array_init(return_value);
> zend_hash_copy(Z_ARRVAL_P(return_value), Z_ARRVAL_P(persitent_zval_hashtable),
> ***)..
>
> since zval_copy_ctor does shallow copy, so the persistent array return to the
> php
> script.
>
>
> then if it happen to be parsed by zval_ptr_dtor, then the persistent array will
> be
> parsed by gc_zval_possible_root,
>
> ZEND_API void gc_zval_possible_root(zval *zv TSRMLS_DC)
> {
>
> ..................
>
>    if (GC_ZVAL_GET_COLOR(zv) != GC_PURPLE) {
>        GC_ZVAL_SET_PURPLE(zv);
> ..................
>
> then the malloc info of the block(not sure before or after) will be polluted.
>
> then when the extension try to free the block,  a warning will be show like:
>
> munmap_chunk(): invalid pointer 0x*******
>
>
> I have make a patch for this(https://bugs.php.net/bug.php?id=61649),
> if you think it's okey,  I will commit it to all branches,
>
> thanks
>
> --
> Laruence  Xinchen Hui
> http://www.laruence.com/



--
Laruence  Xinchen Hui
http://www.laruence.com/

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

I think your patch is right, but I've never seen this problem before.
Is it related to your own extension? What does it do? Why we didn't see
this problem with other extensions?

Finally, I think that such persistent HashTables must be very rare, so
introducing additional check would slowdown the engine. So allocating
zend_gc_info may be a better solution.

Thanks. Dmitry.

On 04/06/2012 04:32 PM, Laruence wrote:
> Hi:
> and, yes, extension can change their code to alloc a zend_gc_info
> size(what I do now).
>
> however, I think, it's no need for zend_gc care about persistent
> memorys, right?
>
> thanks
>
>
> On Fri, Apr 6, 2012 at 8:21 PM, Laruence<[email protected]> wrote:
>> Hi Dmitry:
>>
>> zend gc was introducted in 5.3
>>
>> thinking of a zval which is a Hashtable allocated by a extension in persistent,
>> and it also has hashtable children in it,
>>
>> then , if the extension return this to php script:
>>
>> array_init(return_value);
>> zend_hash_copy(Z_ARRVAL_P(return_value), Z_ARRVAL_P(persitent_zval_hashtable),
>> ***)..
>>
>> since zval_copy_ctor does shallow copy, so the persistent array return to the
>> php
>> script.
>>
>>
>> then if it happen to be parsed by zval_ptr_dtor, then the persistent array will
>> be
>> parsed by gc_zval_possible_root,
>>
>> ZEND_API void gc_zval_possible_root(zval *zv TSRMLS_DC)
>> {
>>
>> ..................
>>
>> if (GC_ZVAL_GET_COLOR(zv) != GC_PURPLE) {
>> GC_ZVAL_SET_PURPLE(zv);
>> ..................
>>
>> then the malloc info of the block(not sure before or after) will be polluted.
>>
>> then when the extension try to free the block, a warning will be show like:
>>
>> munmap_chunk(): invalid pointer 0x*******
>>
>>
>> I have make a patch for this(https://bugs.php.net/bug.php?id=61649),
>> if you think it's okey, I will commit it to all branches,
>>
>> thanks
>>
>> --
>> Laruence Xinchen Hui
>> http://www.laruence.com/
>
>
>


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
On Mon, Apr 9, 2012 at 3:09 PM, Dmitry Stogov <[email protected]> wrote:
> Hi Laruence,
>
> I think your patch is right, but I've never seen this problem before.
> Is it related to your own extension? What does it do? Why we didn't see this
> problem with other extensions?
a persistent configure extension, it loads all ini configure file at MINIT..

>
> Finally, I think that such persistent HashTables must be very rare, so
> introducing additional check would slowdown the engine. So allocating
> zend_gc_info may be a better solution.
okey, it's really rare usages :)

thanks
>
> Thanks. Dmitry.
>
>
> On 04/06/2012 04:32 PM, Laruence wrote:
>>
>> Hi:
>>    and, yes,  extension can change their code to alloc a zend_gc_info
>> size(what I do now).
>>
>>    however, I think, it's no need for zend_gc care about persistent
>> memorys, right?
>>
>> thanks
>>
>>
>> On Fri, Apr 6, 2012 at 8:21 PM, Laruence<[email protected]>  wrote:
>>>
>>> Hi Dmitry:
>>>
>>>   zend gc was introducted in 5.3
>>>
>>> thinking of a zval which is a Hashtable allocated by a extension in
>>> persistent,
>>> and it also has hashtable children in it,
>>>
>>> then , if the extension return this to php script:
>>>
>>> array_init(return_value);
>>> zend_hash_copy(Z_ARRVAL_P(return_value),
>>> Z_ARRVAL_P(persitent_zval_hashtable),
>>> ***)..
>>>
>>> since zval_copy_ctor does shallow copy, so the persistent array return to
>>> the
>>> php
>>> script.
>>>
>>>
>>> then if it happen to be parsed by zval_ptr_dtor, then the persistent
>>> array will
>>> be
>>> parsed by gc_zval_possible_root,
>>>
>>> ZEND_API void gc_zval_possible_root(zval *zv TSRMLS_DC)
>>> {
>>>
>>> ..................
>>>
>>>    if (GC_ZVAL_GET_COLOR(zv) != GC_PURPLE) {
>>>        GC_ZVAL_SET_PURPLE(zv);
>>> ..................
>>>
>>> then the malloc info of the block(not sure before or after) will be
>>> polluted.
>>>
>>> then when the extension try to free the block,  a warning will be show
>>> like:
>>>
>>> munmap_chunk(): invalid pointer 0x*******
>>>
>>>
>>> I have make a patch for this(https://bugs.php.net/bug.php?id=61649),
>>> if you think it's okey,  I will commit it to all branches,
>>>
>>> thanks
>>>
>>> --
>>> Laruence  Xinchen Hui
>>> http://www.laruence.com/
>>
>>
>>
>>
>



--
Laruence  Xinchen Hui
http://www.laruence.com/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login