[libvirt] [PATCH] cpu: fix possible crash in getModels

[Peter Krempa]

On 12/03/14 19:02, Pavel Hrdina wrote:
> Commit 86a15a25 introduced a new cpu driver API 'getModels'. Public API
> allow you to pass NULL for models to get only number of existing models.
> However the new code will crash with segfault so we have to count with
> the possibility that the user wants only the number.
> 
> There is also difference in order of the models gathered by this new API
> as the old approach was inserting the elements to the end of the array
> so we should use 'VIR_APPEND_ELEMENT'.
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  src/cpu/cpu_powerpc.c | 15 ++++++++++-----
>  src/cpu/cpu_x86.c     | 15 ++++++++++-----
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
> index 871401b..86f80b3 100644
> --- a/src/cpu/cpu_powerpc.c
> +++ b/src/cpu/cpu_powerpc.c
> @@ -666,11 +666,15 @@ ppcGetModels(char ***models)
>  
>      model = map->models;
>      while (model != NULL) {
> -        if (VIR_STRDUP(name, model->name) < 0)
> -            goto error;
> +        if (models) {
> +            if (VIR_STRDUP(name, model->name) < 0)
> +                goto error;
>  
> -        if (VIR_INSERT_ELEMENT(*models, 0, nmodels, name) < 0)
> -            goto error;
> +            if (VIR_APPEND_ELEMENT(*models, nmodels, name) < 0)
> +                goto error;
> +        } else {
> +            nmodels++;
> +        }
>  
>          model = model->next;
>      }
> @@ -681,7 +685,8 @@ ppcGetModels(char ***models)
>      return nmodels;
>  
>   error:
> -    virStringFreeList(*models);
> +    if (models)
> +        virStringFreeList(*models);

if (models) {
	virStringFreeList(*models);
	*models = NULL;
}

To remove the possibly invalid pointer.

>      nmodels = -1;
>      goto cleanup;
>  }
> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index f6dcba4..dfbc16c 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -2176,11 +2176,15 @@ x86GetModels(char ***models)
>  
>      model = map->models;
>      while (model != NULL) {
> -        if (VIR_STRDUP(name, model->name) < 0)
> -            goto error;
> +        if (models) {
> +            if (VIR_STRDUP(name, model->name) < 0)
> +                goto error;
>  
> -        if (VIR_INSERT_ELEMENT(*models, 0, nmodels, name) < 0)
> -            goto error;
> +            if (VIR_APPEND_ELEMENT(*models, nmodels, name) < 0)
> +                goto error;
> +        } else {
> +            nmodels++;
> +        }
>  
>          model = model->next;
>      }
> @@ -2188,7 +2192,8 @@ x86GetModels(char ***models)
>      return nmodels;
>  
>   error:
> -    virStringFreeList(*models);
> +    if (models)
> +        virStringFreeList(*models);

Same as above.

>      return -1;
>  }
>  
> 

ACK,

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141203/c41deff2/attachment-0001.sig>