[libvirt] [PATCH 3/3] cpuGetModels: Create a NULL-terminated list

[Pavel Hrdina]

On Fri, May 13, 2016 at 11:23:30PM +0200, Jiri Denemark wrote:
> The list of CPU models is freed using virStringFreeList, which expects
> the list to by NULL-terminated. This bug could theoretically crash
> libvirtd in remoteDispatchConnectGetCPUModelNames, but luckily enough we
> never return more than REMOTE_CONNECT_CPU_MODELS_MAX models in the list.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/cpu/cpu.c       | 2 +-
>  src/cpu/cpu_ppc64.c | 7 +++++++
>  src/cpu/cpu_x86.c   | 7 +++++++
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
> index 69055e2..2f2b658 100644
> --- a/src/cpu/cpu.c
> +++ b/src/cpu/cpu.c
> @@ -716,7 +716,7 @@ cpuModelIsAllowed(const char *model,
>   * cpuGetModels:
>   *
>   * @arch: CPU architecture
> - * @models: where to store the list of supported models
> + * @models: where to store the NULL-terminated list of supported models
>   *
>   * Fetches all CPU models supported by libvirt on @archName.
>   *
> diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
> index 1aed984..c784a55 100644
> --- a/src/cpu/cpu_ppc64.c
> +++ b/src/cpu/cpu_ppc64.c
> @@ -897,6 +897,13 @@ ppc64DriverGetModels(char ***models)
>          model = model->next;
>      }
>  
> +    if (models) {
> +        /* Make sure models is NULL-terminated */
> +        if (VIR_EXPAND_N(*models, nmodels, 1) < 0)
> +            goto error;
> +        nmodels--;
> +    }
> +

This fixes the function only in case that there is no failure.  If the
VIR_EXPAND_N() or VIR_APPEND_ELEMENT() fails we will hit segfault in
virStringFreeList().  We need to preallocate the whole array with nmodels + 1.

Pavel

>   cleanup:
>      ppc64MapFree(map);
>  
> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index c6aacc1..6ee7ff9 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -2230,6 +2230,13 @@ x86GetModels(char ***models)
>          model = model->next;
>      }
>  
> +    if (models) {
> +        /* Make sure models is NULL-terminated */
> +        if (VIR_EXPAND_N(*models, nmodels, 1) < 0)
> +            goto error;
> +        nmodels--;
> +    }
> +
>      return nmodels;
>  
>   error:
> -- 
> 2.8.2
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list