On 15.04.2016 08:05, Remi Collet wrote:
Le 14/04/2016 18:30, Michal Privoznik a écrit :
On 13.04.2016 18:13, Neal Gompa wrote:
From: Remi Collet <fedora@famillecollet.com>
--- src/libvirt-php.c | 715 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 553 insertions(+), 162 deletions(-)
diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 1af6077..3f06edc 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c
@@ -5984,7 +6371,7 @@ PHP_FUNCTION(libvirt_domain_memory_peek) zval *zdomain; int retval; long flags=0; - long long start; + long start;
This is spurious for two reasons: 1) this patch aims at something different. So this change does not belong here. 2) The @start variable is passed to virDomainMemoryPeek(). It expects unsigned long long. So this change does not belong here.
I confirm this patch is correct and needed.
GET_DOMAIN_FROM_ARGS("rlll",&zdomain,&start,&size,&flags);
So this is broken too. But there's more similar bugs to this in our code.
zend_parse_parameters expect a (long *) for "l" option
Casting a (long long *) to a (long *) is terrible, mash the stack, can raise segfault, and very bad thing on big endian
How can it mash the stack? I'd expect sizeof(long long *) == sizeof (long *) == sizeof(void *). It's a pointer after all. True about endiandness though. But then again - bug is in GET_DOMAIN_FROM_ARGS() arg string. Then again, this commit focuses on adding missing arginfo. This should not had been here rather than a separate patch.
retval=virDomainMemoryPeek(domain->domain,start,size,buff,flags);
Casting a (long) to a (long long) is perfectly OK.
So current version is broken
- zend_ulong64 start; + zend_long start;
And if you want to manage the negative value, just add a test
if (start<0) RETURN_FALSE;
I don't think we need to come up with an extension to libvirt API. The @start argument tells an address from which the API should fetch data. Unsigned integer value is just fine for that. Michal