src/libvirt-connection.c | 8 +++----- src/libvirt-php.c | 6 ++---- src/libvirt-php.h | 1 + src/util.h | 16 +++++++++------- 4 files changed, 15 insertions(+), 16 deletions(-)
The PHP7 variant of the macro wasn't safe if the hash key was not a
string type. This was found when running php script with just
libvirt_connect call under xdebug session which segfaulted. This patch
makes the following changes:
* make sure that tmp_name is initialized to NULL
* set the key name only when zend_hash_get_current_key_ex did set it to
something which happens only when type is HASH_KEY_IS_STRING
* stash the key index in out php_libvirt_hash_key_info struct because it
wasn't there before and separate variable had to be used.
---
src/libvirt-connection.c | 8 +++-----
src/libvirt-php.c | 6 ++----
src/libvirt-php.h | 1 +
src/util.h | 16 +++++++++-------
4 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/src/libvirt-connection.c b/src/libvirt-connection.c
index 181b266..2d59d82 100644
--- a/src/libvirt-connection.c
+++ b/src/libvirt-connection.c
@@ -131,8 +131,6 @@ PHP_FUNCTION(libvirt_connect)
HashPosition pointer;
int array_count;
- zend_ulong index;
-
unsigned long libVer;
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|sba", &url, &url_len, &readonly, &zcreds) == FAILURE) {
@@ -176,13 +174,13 @@ PHP_FUNCTION(libvirt_connect)
VIRT_FOREACH(arr_hash, pointer, data) {
if (Z_TYPE_P(data) == IS_STRING) {
php_libvirt_hash_key_info info;
- VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, index, info);
+ VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, info);
if (info.type == HASH_KEY_IS_STRING) {
PHPWRITE(info.name, info.length);
} else {
- DPRINTF("%s: credentials index %d\n", PHPFUNC, (int)index);
- creds[j].type = index;
+ DPRINTF("%s: credentials index %d\n", PHPFUNC, info.index);
+ creds[j].type = info.index;
creds[j].result = (char *)emalloc(Z_STRLEN_P(data) + 1);
memset(creds[j].result, 0, Z_STRLEN_P(data) + 1);
creds[j].resultlen = Z_STRLEN_P(data);
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index ef057fe..efbef58 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -1921,7 +1921,6 @@ long get_next_free_numeric_value(virDomainPtr domain, char *xpath)
HashPosition pointer;
// int array_count;
zval *data;
- unsigned long index;
long max_slot = -1;
xml = virDomainGetXMLDesc(domain, VIR_DOMAIN_XML_INACTIVE);
@@ -1934,7 +1933,7 @@ long get_next_free_numeric_value(virDomainPtr domain, char *xpath)
VIRT_FOREACH(arr_hash, pointer, data) {
if (Z_TYPE_P(data) == IS_STRING) {
php_libvirt_hash_key_info info;
- VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, index, info);
+ VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, info);
if (info.type != HASH_KEY_IS_STRING) {
long num = -1;
@@ -2439,7 +2438,6 @@ void parse_array(zval *arr, tVMDisk *disk, tVMNetwork *network)
zval *data;
php_libvirt_hash_key_info key;
HashPosition pointer;
- unsigned long index;
arr_hash = Z_ARRVAL_P(arr);
//array_count = zend_hash_num_elements(arr_hash);
@@ -2451,7 +2449,7 @@ void parse_array(zval *arr, tVMDisk *disk, tVMNetwork *network)
VIRT_FOREACH(arr_hash, pointer, data) {
if ((Z_TYPE_P(data) == IS_STRING) || (Z_TYPE_P(data) == IS_LONG)) {
- VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, index, key);
+ VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, key);
if (key.type == HASH_KEY_IS_STRING) {
if (disk != NULL) {
if ((Z_TYPE_P(data) == IS_STRING) && strcmp(key.name, "path") == 0)
diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index 8d13a6b..f24a329 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -137,6 +137,7 @@ typedef struct tVMNetwork {
typedef struct _php_libvirt_hash_key_info {
char *name;
unsigned int length;
+ unsigned int index;
unsigned int type;
} php_libvirt_hash_key_info;
diff --git a/src/util.h b/src/util.h
index ecb3a1f..72cfa91 100644
--- a/src/util.h
+++ b/src/util.h
@@ -135,12 +135,14 @@
# define VIRT_FOREACH_END(_dummy)
-# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _idx, _info) \
+# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _info) \
do { \
- zend_string *tmp_key_info; \
- _info.type = zend_hash_get_current_key_ex(_ht, &tmp_key_info, &_idx, &_pos); \
- _info.name = ZSTR_VAL(tmp_key_info); \
- _info.length = ZSTR_LEN(tmp_key_info); \
+ zend_string *tmp_name = NULL; \
+ _info.type = zend_hash_get_current_key_ex(_ht, &tmp_name, (zend_ulong *) &_info.index, &_pos); \
+ if (tmp_name) { \
+ _info.name = ZSTR_VAL(tmp_name); \
+ _info.length = ZSTR_LEN(tmp_name); \
+ } \
} while(0)
# define VIRT_ARRAY_INIT(_name) do { \
@@ -213,9 +215,9 @@
# define VIRT_FOREACH_END(_dummy) \
}}
-# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _idx, _info) \
+# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _info) \
do { \
- _info.type = zend_hash_get_current_key_ex(_ht, &_info.name, &_info.length, &_idx, 0, &_pos); \
+ _info.type = zend_hash_get_current_key_ex(_ht, &_info.name, &_info.length, &_info.index, 0, &_pos); \
} while(0)
# define VIRT_ARRAY_INIT(_name) do {\
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 12/06/2017 12:17 AM, Dawid Zamirski wrote: > The PHP7 variant of the macro wasn't safe if the hash key was not a > string type. This was found when running php script with just > libvirt_connect call under xdebug session which segfaulted. This patch > makes the following changes: > > * make sure that tmp_name is initialized to NULL > * set the key name only when zend_hash_get_current_key_ex did set it to > something which happens only when type is HASH_KEY_IS_STRING > * stash the key index in out php_libvirt_hash_key_info struct because it > wasn't there before and separate variable had to be used. > --- > src/libvirt-connection.c | 8 +++----- > src/libvirt-php.c | 6 ++---- > src/libvirt-php.h | 1 + > src/util.h | 16 +++++++++------- > 4 files changed, 15 insertions(+), 16 deletions(-) > > diff --git a/src/libvirt-connection.c b/src/libvirt-connection.c > index 181b266..2d59d82 100644 > --- a/src/libvirt-connection.c > +++ b/src/libvirt-connection.c > @@ -131,8 +131,6 @@ PHP_FUNCTION(libvirt_connect) > HashPosition pointer; > int array_count; > > - zend_ulong index; > - > unsigned long libVer; > > if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|sba", &url, &url_len, &readonly, &zcreds) == FAILURE) { > @@ -176,13 +174,13 @@ PHP_FUNCTION(libvirt_connect) > VIRT_FOREACH(arr_hash, pointer, data) { > if (Z_TYPE_P(data) == IS_STRING) { > php_libvirt_hash_key_info info; > - VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, index, info); > + VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, info); > > if (info.type == HASH_KEY_IS_STRING) { > PHPWRITE(info.name, info.length); > } else { > - DPRINTF("%s: credentials index %d\n", PHPFUNC, (int)index); > - creds[j].type = index; > + DPRINTF("%s: credentials index %d\n", PHPFUNC, info.index); > + creds[j].type = info.index; > creds[j].result = (char *)emalloc(Z_STRLEN_P(data) + 1); > memset(creds[j].result, 0, Z_STRLEN_P(data) + 1); > creds[j].resultlen = Z_STRLEN_P(data); > diff --git a/src/libvirt-php.c b/src/libvirt-php.c > index ef057fe..efbef58 100644 > --- a/src/libvirt-php.c > +++ b/src/libvirt-php.c > @@ -1921,7 +1921,6 @@ long get_next_free_numeric_value(virDomainPtr domain, char *xpath) > HashPosition pointer; > // int array_count; > zval *data; > - unsigned long index; > long max_slot = -1; > > xml = virDomainGetXMLDesc(domain, VIR_DOMAIN_XML_INACTIVE); > @@ -1934,7 +1933,7 @@ long get_next_free_numeric_value(virDomainPtr domain, char *xpath) > VIRT_FOREACH(arr_hash, pointer, data) { > if (Z_TYPE_P(data) == IS_STRING) { > php_libvirt_hash_key_info info; > - VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, index, info); > + VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, info); > > if (info.type != HASH_KEY_IS_STRING) { > long num = -1; > @@ -2439,7 +2438,6 @@ void parse_array(zval *arr, tVMDisk *disk, tVMNetwork *network) > zval *data; > php_libvirt_hash_key_info key; > HashPosition pointer; > - unsigned long index; > > arr_hash = Z_ARRVAL_P(arr); > //array_count = zend_hash_num_elements(arr_hash); > @@ -2451,7 +2449,7 @@ void parse_array(zval *arr, tVMDisk *disk, tVMNetwork *network) > > VIRT_FOREACH(arr_hash, pointer, data) { > if ((Z_TYPE_P(data) == IS_STRING) || (Z_TYPE_P(data) == IS_LONG)) { > - VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, index, key); > + VIRT_HASH_CURRENT_KEY_INFO(arr_hash, pointer, key); > if (key.type == HASH_KEY_IS_STRING) { > if (disk != NULL) { > if ((Z_TYPE_P(data) == IS_STRING) && strcmp(key.name, "path") == 0) > diff --git a/src/libvirt-php.h b/src/libvirt-php.h > index 8d13a6b..f24a329 100644 > --- a/src/libvirt-php.h > +++ b/src/libvirt-php.h > @@ -137,6 +137,7 @@ typedef struct tVMNetwork { > typedef struct _php_libvirt_hash_key_info { > char *name; > unsigned int length; > + unsigned int index; This needs to be zend_ulong type. Thing is, zend_hash_get_current_key_ex() expects zend_ulong *. In both version 7 and 5. If we do that we can drop the typecast done ... > unsigned int type; > } php_libvirt_hash_key_info; > > diff --git a/src/util.h b/src/util.h > index ecb3a1f..72cfa91 100644 > --- a/src/util.h > +++ b/src/util.h > @@ -135,12 +135,14 @@ > > # define VIRT_FOREACH_END(_dummy) > > -# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _idx, _info) \ > +# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _info) \ > do { \ > - zend_string *tmp_key_info; \ > - _info.type = zend_hash_get_current_key_ex(_ht, &tmp_key_info, &_idx, &_pos); \ > - _info.name = ZSTR_VAL(tmp_key_info); \ > - _info.length = ZSTR_LEN(tmp_key_info); \ > + zend_string *tmp_name = NULL; \ > + _info.type = zend_hash_get_current_key_ex(_ht, &tmp_name, (zend_ulong *) &_info.index, &_pos); \ .. here, > + if (tmp_name) { \ > + _info.name = ZSTR_VAL(tmp_name); \ > + _info.length = ZSTR_LEN(tmp_name); \ > + } \ > } while(0) > > # define VIRT_ARRAY_INIT(_name) do { \ > @@ -213,9 +215,9 @@ > # define VIRT_FOREACH_END(_dummy) \ > }} > > -# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _idx, _info) \ > +# define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _info) \ > do { \ > - _info.type = zend_hash_get_current_key_ex(_ht, &_info.name, &_info.length, &_idx, 0, &_pos); \ > + _info.type = zend_hash_get_current_key_ex(_ht, &_info.name, &_info.length, &_info.index, 0, &_pos); \ but not here. Does that work for you? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2017-12-06 at 16:21 +0100, Michal Privoznik wrote: > On 12/06/2017 12:17 AM, Dawid Zamirski wrote: > > > > &_info.length, &_info.index, 0, &_pos); \ > > but not here. Does that work for you? > It does but with a compiler warning :-) I did not realize PHP5 had zend_ulong and I thought it was introduced only in PHP7. V2 coming in a minute. Thanks, Dawid > Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2017-12-06 at 11:40 -0500, Dawid Zamirski wrote: > On Wed, 2017-12-06 at 16:21 +0100, Michal Privoznik wrote: > > On 12/06/2017 12:17 AM, Dawid Zamirski wrote: > > > > > > &_info.length, &_info.index, 0, &_pos); \ > > > > but not here. Does that work for you? > > > > It does but with a compiler warning :-) I did not realize PHP5 had > zend_ulong and I thought it was introduced only in PHP7. V2 coming in > a > minute. Well PHP5 indeed does not have zend_ulong but we do typedef it already in src/util.h > > Thanks, > Dawid > > > Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.