Everything

php

ZendEngine 2 - Double Free BUG

php.internals

Stefan Esser

22 years ago

Hi,

I ported Hardened-PHP to PHP5 yesterday night and got into the problem
that there were some crashes. While one of the crashes is a problem in
the port (at least I guess so), the other one was a detected double
efree() in __set__get_001.phpt

I tracked this double free down to

static void zend_post_incdec_property(...)

where z is passed to

Z_OBJ_HT_P(object)->write_property(object, property, z TSRMLS_CC);

within this function z is freed and

if (z->refcount == 0) {
zval_dtor(z);
FREE_ZVAL(z);
}

will free it again.

I fixed this by adding a z->refcount++ after
*retval = *z; but I do not know if this is the correct place,
because I am not really into ZE2 internals.

Stefan Esser

Stanislav Malyshev

22 years ago

SE>>Z_OBJ_HT_P(object)->write_property(object, property, z TSRMLS_CC);
SE>>
SE>>within this function z is freed and

Why should write_property free z?

--
Stanislav Malyshev, Zend Products Engineer
stas@zend.com  http://www.zend.com/ +972-3-6139665 ext.115

Stefan Esser

22 years ago

Stanislav Malyshev wrote:

> SE>>Z_OBJ_HT_P(object)->write_property(object, property, z TSRMLS_CC);
> SE>>
> SE>>within this function z is freed and
>
> Why should write_property free z?

static int zend_std_call_setter(zval *object, zval *member, zval *value
TSRMLS_DC)
{
zval **call_args[2];
zval *retval = NULL;
zval __set_name;
int call_result;
int ret;

/* __set handler is called with two arguments:
property name
value to be set

it should return whether the call was successfull or not
*/
INIT_PZVAL(&__set_name);
ZVAL_STRINGL(&__set_name, ZEND_SET_FUNC_NAME,
sizeof(ZEND_SET_FUNC_NAME)-1, 0);

call_args[0] = &member;
value->refcount++;
call_args[1] = &value;

/* go call the __set handler */
call_result = call_user_function_ex(NULL,

&object,

&__set_name,

&retval,

2, call_args,

0, NULL TSRMLS_CC);

/*
call_result is if call_user_function gone OK.
retval shows if __get method went OK.
*/


if (call_result == FAILURE) {
zend_error(E_ERROR, "Could not call __set handler for
class %s", Z_OBJCE_P(object)->name);
return FAILURE;
}

zval_ptr_dtor(&value);


Maybe you should ask the person who wrote it "why". Well the problem is
not that it frees it. The problem is that the refcount is zero when the
std_setter is called.

Stefan

Stanislav Malyshev

22 years ago

SE>>Maybe you should ask the person who wrote it "why". Well the problem
SE>>is not that it frees it. The problem is that the refcount is zero when
SE>>the std_setter is called.

I think you are right, it should be fixed in zend_post_incdec_property. Do
you have reproducing code example so it can be tested?

--
Stanislav Malyshev, Zend Products Engineer
stas@zend.com  http://www.zend.com/ +972-3-6139665 ext.115

Stefan Esser

22 years ago

Stanislav Malyshev wrote:

> I think you are right, it should be fixed in zend_post_incdec_property. Do
> you have reproducing code example so it can be tested?

No it cannot be tested. In the default configuration Zend_MM is
activated. This will catch double frees. No violation will happen when
it is activated. This is why valgrind etc... do not catch it.

And I think there is another bug with simple classes on termination
of a request.

class xy
{
function a()
{
}
}

$y = new xy();

crashes over here with Hardened-PHP applied AND maintainer-zts
activated. It crashs in a llist destruction from within
zend_deactivate. The reason for the crash seems that the memory
pointed to by TRMS_ls is already freed.

Stefan

Marcus Börger

22 years ago

Hello Stefan,

i made a fix for TSRM which is comitted but disabled by
#if MBO_0
could you add
#define MBO_0
at the to of TSRM/TSRM.c and check if that helps a bit?

marcus

Wednesday, June 2, 2004, 2:59:49 PM, you wrote:

> Stanislav Malyshev wrote:

>> I think you are right, it should be fixed in zend_post_incdec_property. Do
>> you have reproducing code example so it can be tested?

> No it cannot be tested. In the default configuration Zend_MM is
> activated. This will catch double frees. No violation will happen when
> it is activated. This is why valgrind etc... do not catch it.

> And I think there is another bug with simple classes on termination
> of a request.

> class xy
> {
> 	function a()
> 	{
> 	}
> }

> $y = new xy();

> crashes over here with Hardened-PHP applied AND maintainer-zts
> activated. It crashs in a llist destruction from within
> zend_deactivate. The reason for the crash seems that the memory
> pointed to by TRMS_ls is already freed.

> Stefan

--
Best regards,
Marcus                            mailto:helly@php.net