[libvirt] [libvirt-php][PATCH] libvirt_list_active_domains: Don't free domain before strduping its name

[Matthias Bolte]

2016-09-07 18:01 GMT+02:00 Michal Privoznik <mprivozn at redhat.com>:
> There's a bug in the implementation of
> libvirt_list_active_domains(). We try to return an array of names
> of active domains. So we iterate over array of domains as
> returned by libvirt and get each domain's name. But we don't
> obtain a duplicate of that name rather just a pointer into
> virDomain object. The very next thing we do is we free the
> virDomain object and then try to copy what's now invalid pointer.
> Reverse the order of those two actions.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/libvirt-php.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/src/libvirt-php.c b/src/libvirt-php.c
> index 2045c59..8edcb10 100644
> --- a/src/libvirt-php.c
> +++ b/src/libvirt-php.c
> @@ -9128,18 +9128,15 @@ PHP_FUNCTION(libvirt_list_active_domains)
>          domain=virDomainLookupByID(conn->conn,ids[i]);
>          if (domain!=NULL)
>          {
> -            name=virDomainGetName(domain);
> -
> -            if (virDomainFree (domain))
> -                resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, domain, 0 TSRMLS_CC);
> -
> -            if (name==NULL)
> -            {
> -                efree (ids);
> +            if (!(name = virDomainGetName(domain))) {
> +                efree(ids);

You need to call virDomainFree here as well. Otherwise the domain
objects leaks if virDomainGetName fails.

>                  RETURN_FALSE;
>              }
>
>              VIRT_ADD_NEXT_INDEX_STRING(return_value, name);
> +
> +            if (virDomainFree(domain))
> +                resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, domain, 0 TSRMLS_CC);
>          }
>      }
>      efree(ids);


-- 
Matthias Bolte
http://photron.blogspot.com