From 4218e89f8df4ca3897e3aad595e0c2c9cf4c3aca Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 29 Oct 2013 20:58:30 +0100 Subject: [PATCH] Fix bug #65997 by switching to Serializable for GMP Rather than using get_properties and __wakeup for serialization the code now uses Serializable::serialize() and Serializable::unserialize(). The get_properties handler is switched to a get_debug_info handler. Thus get_gc will now return only the normal properties and not do any modifications, thus fixing the leak. This also avoids a $num property from being publicly visible after the object was dumped or serialized, so that's an extra plus. --- ext/gmp/gmp.c | 107 ++++++++++++++++++++++++++++------- ext/gmp/tests/bug659967.phpt | 15 +++++ ext/gmp/tests/serialize.phpt | 22 ++++++- 3 files changed, 124 insertions(+), 20 deletions(-) create mode 100644 ext/gmp/tests/bug659967.phpt diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c index 6b3dadf405..af9c73afa2 100644 --- a/ext/gmp/gmp.c +++ b/ext/gmp/gmp.c @@ -24,7 +24,10 @@ #include "php_ini.h" #include "php_gmp.h" #include "ext/standard/info.h" +#include "ext/standard/php_var.h" +#include "ext/standard/php_smart_str_public.h" #include "zend_exceptions.h" +#include "zend_interfaces.h" #if HAVE_GMP @@ -564,12 +567,17 @@ static int gmp_cast_object(zval *readobj, zval *writeobj, int type TSRMLS_DC) /* } /* }}} */ -static HashTable *gmp_get_properties(zval *obj TSRMLS_DC) /* {{{ */ +static HashTable *gmp_get_debug_info(zval *obj, int *is_temp TSRMLS_DC) /* {{{ */ { - HashTable *ht = zend_std_get_properties(obj TSRMLS_CC); + HashTable *ht, *props = zend_std_get_properties(obj TSRMLS_CC); mpz_ptr gmpnum = GET_GMP_FROM_ZVAL(obj); zval *zv; + *is_temp = 1; + ALLOC_HASHTABLE(ht); + ZEND_INIT_SYMTABLE_EX(ht, zend_hash_num_elements(props) + 1, 0); + zend_hash_copy(ht, props, (copy_ctor_func_t) zval_add_ref, NULL, sizeof(zval *)); + MAKE_STD_ZVAL(zv); gmp_strval(zv, gmpnum, 10); zend_hash_update(ht, "num", sizeof("num"), &zv, sizeof(zval *), NULL); @@ -678,36 +686,96 @@ static int gmp_compare(zval *result, zval *op1, zval *op2 TSRMLS_DC) /* {{{ */ } /* }}} */ -PHP_METHOD(GMP, __wakeup) /* {{{ */ +PHP_METHOD(GMP, serialize) /* {{{ */ { - HashTable *props; - zval **num_zv; + mpz_ptr gmpnum = GET_GMP_FROM_ZVAL(getThis()); + smart_str buf = {0}; + php_serialize_data_t var_hash; + zval zv, *zv_ptr = &zv; if (zend_parse_parameters_none() == FAILURE) { return; } - props = zend_std_get_properties(getThis() TSRMLS_CC); - if (zend_hash_find(props, "num", sizeof("num"), (void **) &num_zv) == SUCCESS - && Z_TYPE_PP(num_zv) == IS_STRING && Z_STRLEN_PP(num_zv) > 0 + PHP_VAR_SERIALIZE_INIT(var_hash); + + INIT_PZVAL(zv_ptr); + + gmp_strval(zv_ptr, gmpnum, 10); + php_var_serialize(&buf, &zv_ptr, &var_hash TSRMLS_CC); + zval_dtor(zv_ptr); + + Z_ARRVAL_P(zv_ptr) = zend_std_get_properties(getThis() TSRMLS_CC); + Z_TYPE_P(zv_ptr) = IS_ARRAY; + php_var_serialize(&buf, &zv_ptr, &var_hash TSRMLS_CC); + + PHP_VAR_SERIALIZE_DESTROY(var_hash); + + if (buf.c) { + RETURN_STRINGL(buf.c, buf.len, 0); + } +} +/* }}} */ + +PHP_METHOD(GMP, unserialize) /* {{{ */ +{ + mpz_ptr gmpnum = GET_GMP_FROM_ZVAL(getThis()); + char *str; + int str_len; + php_unserialize_data_t var_hash; + const unsigned char *p, *max; + zval zv, *zv_ptr = &zv; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &str, &str_len) == FAILURE) { + return; + } + + PHP_VAR_UNSERIALIZE_INIT(var_hash); + + p = (unsigned char *) str; + max = (unsigned char *) str + str_len; + + INIT_ZVAL(zv); + if (!php_var_unserialize(&zv_ptr, &p, max, &var_hash TSRMLS_CC) + || Z_TYPE_P(zv_ptr) != IS_STRING + || convert_to_gmp(gmpnum, zv_ptr, 10 TSRMLS_CC) == FAILURE ) { - mpz_ptr gmpnumber = GET_GMP_FROM_ZVAL(getThis()); - if (convert_to_gmp(gmpnumber, *num_zv, 10 TSRMLS_CC) == SUCCESS) { - return; - } + zend_throw_exception(NULL, "Could not unserialize number", 0 TSRMLS_CC); + goto exit; } + zval_dtor(&zv); - zend_throw_exception( - NULL, "Invalid serialization data", 0 TSRMLS_CC - ); + INIT_ZVAL(zv); + if (!php_var_unserialize(&zv_ptr, &p, max, &var_hash TSRMLS_CC) + || Z_TYPE_P(zv_ptr) != IS_ARRAY + ) { + zend_throw_exception(NULL, "Could not unserialize properties", 0 TSRMLS_CC); + goto exit; + } + + if (zend_hash_num_elements(Z_ARRVAL_P(zv_ptr)) != 0) { + zend_hash_copy( + zend_std_get_properties(getThis() TSRMLS_CC), Z_ARRVAL_P(zv_ptr), + (copy_ctor_func_t) zval_add_ref, NULL, sizeof(zval *) + ); + } + +exit: + zval_dtor(&zv); + PHP_VAR_UNSERIALIZE_DESTROY(var_hash); } /* }}} */ -ZEND_BEGIN_ARG_INFO_EX(arginfo_wakeup, 0, 0, 0) +ZEND_BEGIN_ARG_INFO_EX(arginfo_serialize, 0, 0, 0) +ZEND_END_ARG_INFO() + +ZEND_BEGIN_ARG_INFO_EX(arginfo_unserialize, 0, 0, 1) +ZEND_ARG_INFO(0, serialized) ZEND_END_ARG_INFO() const zend_function_entry gmp_methods[] = { - PHP_ME(GMP, __wakeup, arginfo_wakeup, ZEND_ACC_PUBLIC) + PHP_ME(GMP, serialize, arginfo_serialize, ZEND_ACC_PUBLIC) + PHP_ME(GMP, unserialize, arginfo_unserialize, ZEND_ACC_PUBLIC) PHP_FE_END }; @@ -724,13 +792,14 @@ static ZEND_GINIT_FUNCTION(gmp) ZEND_MINIT_FUNCTION(gmp) { zend_class_entry tmp_ce; - INIT_CLASS_ENTRY(tmp_ce, "GMP", gmp_methods); /* No methods on the class for now */ + INIT_CLASS_ENTRY(tmp_ce, "GMP", gmp_methods); gmp_ce = zend_register_internal_class(&tmp_ce TSRMLS_CC); + zend_class_implements(gmp_ce TSRMLS_CC, 1, zend_ce_serializable); gmp_ce->create_object = gmp_create_object; memcpy(&gmp_object_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers)); gmp_object_handlers.cast_object = gmp_cast_object; - gmp_object_handlers.get_properties = gmp_get_properties; + gmp_object_handlers.get_debug_info = gmp_get_debug_info; gmp_object_handlers.clone_obj = gmp_clone_obj; gmp_object_handlers.do_operation = gmp_do_operation; gmp_object_handlers.compare = gmp_compare; diff --git a/ext/gmp/tests/bug659967.phpt b/ext/gmp/tests/bug659967.phpt new file mode 100644 index 0000000000..6ba220274c --- /dev/null +++ b/ext/gmp/tests/bug659967.phpt @@ -0,0 +1,15 @@ +--TEST-- +Bug #65997: Leak when using gc_collect_cycles with new GMP implementation +--SKIPIF-- + +--FILE-- + +===DONE=== +--EXPECT-- +===DONE=== diff --git a/ext/gmp/tests/serialize.phpt b/ext/gmp/tests/serialize.phpt index 26716b659c..208e0e9800 100644 --- a/ext/gmp/tests/serialize.phpt +++ b/ext/gmp/tests/serialize.phpt @@ -9,14 +9,34 @@ var_dump($n = gmp_init(42)); var_dump($s = serialize($n)); var_dump(unserialize($s)); +$n = gmp_init(13); +$n->foo = "bar"; +var_dump(unserialize(serialize($n))); + +try { + unserialize('C:3:"GMP":0:{}'); +} catch (Exception $e) { var_dump($e->getMessage()); } + +try { + unserialize('C:3:"GMP":8:{s:2:"42"}'); +} catch (Exception $e) { var_dump($e->getMessage()); } + ?> --EXPECTF-- object(GMP)#%d (1) { ["num"]=> string(2) "42" } -string(33) "O:3:"GMP":1:{s:3:"num";s:2:"42";}" +string(30) "C:3:"GMP":15:{s:2:"42";a:0:{}}" object(GMP)#%d (1) { ["num"]=> string(2) "42" } +object(GMP)#%d (2) { + ["foo"]=> + string(3) "bar" + ["num"]=> + string(2) "13" +} +string(28) "Could not unserialize number" +string(32) "Could not unserialize properties" -- 2.40.0