[libvirt] [PATCH 2/2] vcpupin: Fix returning of arrays from virDomainVcpuPinAdd

[Daniel Veillard]

On Thu, Aug 30, 2012 at 03:51:55PM +0200, Peter Krempa wrote:
> virDomainVcpuPinAdd does a realloc on vcpupin_list if the new vcpu pin
> definition doesn't fit into the array. The list is an array of pointers
> but the function definition didn't support returning the changed pointer
> to the caller if it was realloced. This caused segfaults if realloc
> would change the base pointer.
> ---
>  src/conf/domain_conf.c   | 8 ++++----
>  src/conf/domain_conf.h   | 2 +-
>  src/libxl/libxl_driver.c | 2 +-
>  src/qemu/qemu_driver.c   | 6 +++---
>  src/xen/xend_internal.c  | 2 +-
>  5 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2dad64d..554298d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11037,7 +11037,7 @@ cleanup:
>      return bitmap;
>  }
> 
> -int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list,
> +int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list,
>                          int *nvcpupin,
>                          unsigned char *cpumap,
>                          int maplen,
> @@ -11052,7 +11052,7 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list,
>      if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL)
>          return -1;
> 
> -    vcpupin = virDomainVcpuPinFindByVcpu(vcpupin_list,
> +    vcpupin = virDomainVcpuPinFindByVcpu(*vcpupin_list,
>                                           *nvcpupin,
>                                           vcpu);
>      if (vcpupin) {
> @@ -11073,14 +11073,14 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list,
>      vcpupin->cpumask = cpumask;
> 
> 
> -    if (VIR_REALLOC_N(vcpupin_list, *nvcpupin + 1) < 0) {
> +    if (VIR_REALLOC_N(*vcpupin_list, *nvcpupin + 1) < 0) {
>          virReportOOMError();
>          VIR_FREE(cpumask);
>          VIR_FREE(vcpupin);
>          return -1;
>      }
> 
> -    vcpupin_list[(*nvcpupin)++] = vcpupin;
> +    (*vcpupin_list)[(*nvcpupin)++] = vcpupin;
> 
>      return 0;
>  }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 9ee57e1..dfdae49 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1885,7 +1885,7 @@ int virDomainCpuSetParse(const char *str,
>  char *virDomainCpuSetFormat(char *cpuset,
>                              int maxcpu);
> 
> -int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list,
> +int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list,
>                          int *nvcpupin,
>                          unsigned char *cpumap,
>                          int maplen,
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index d8ecf13..1638314 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -2461,7 +2461,7 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap,
>          }
>          vm->def->cputune.nvcpupin = 0;
>      }
> -    if (virDomainVcpuPinAdd(vm->def->cputune.vcpupin,
> +    if (virDomainVcpuPinAdd(&vm->def->cputune.vcpupin,
>                              &vm->def->cputune.nvcpupin,
>                              cpumap,
>                              maplen,
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7c0a5c3..5670ca0 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3782,7 +3782,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
>              newVcpuPinNum = 0;
>          }
> 
> -        if (virDomainVcpuPinAdd(newVcpuPin, &newVcpuPinNum, cpumap, maplen, vcpu) < 0) {
> +        if (virDomainVcpuPinAdd(&newVcpuPin, &newVcpuPinNum, cpumap, maplen, vcpu) < 0) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("failed to update vcpupin"));
>              virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum);
> @@ -3849,7 +3849,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
>                  }
>                  persistentDef->cputune.nvcpupin = 0;
>              }
> -            if (virDomainVcpuPinAdd(persistentDef->cputune.vcpupin,
> +            if (virDomainVcpuPinAdd(&persistentDef->cputune.vcpupin,
>                                      &persistentDef->cputune.nvcpupin,
>                                      cpumap,
>                                      maplen,
> @@ -4042,7 +4042,7 @@ qemudDomainPinEmulator(virDomainPtr dom,
>                  newVcpuPinNum = 0;
>              }
> 
> -            if (virDomainVcpuPinAdd(newVcpuPin, &newVcpuPinNum, cpumap, maplen, -1) < 0) {
> +            if (virDomainVcpuPinAdd(&newVcpuPin, &newVcpuPinNum, cpumap, maplen, -1) < 0) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                                 _("failed to update vcpupin"));
>                  virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum);
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 99def42..984f040 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -2303,7 +2303,7 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu,
>              }
>              def->cputune.nvcpupin = 0;
>          }
> -        if (virDomainVcpuPinAdd(def->cputune.vcpupin,
> +        if (virDomainVcpuPinAdd(&def->cputune.vcpupin,
>                                  &def->cputune.nvcpupin,
>                                  cpumap,
>                                  maplen,

  Nice catch ! ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/