[PATCH 2/3] qemu_conf: Avoid dereferencing NULL in virQEMUDriverGetHost{NUMACaps, CPU}

Peter Krempa pkrempa at redhat.com
Fri Jan 24 11:05:28 UTC 2020


On Fri, Jan 24, 2020 at 11:27:17 +0100, Michal Privoznik wrote:
> When fixing [1] I've ran attached reproducer and had it spawn
> 1024 threads and query capabilities XML in each one of them. This
> lead libvirtd to hit the RLIMIT_NOFILE limit which was kind of
> expected. What wasn't expected was a subsequent segfault. It
> happened because virCPUProbeHost failed and returned NULL. We've
> taken the NULL and passed it to virCapabilitiesHostNUMARef()
> which dereferenced it. Code inspection showed the same flas in
> virQEMUDriverGetHostNUMACaps(), so I'm fixing both places.
> 
> 1: https://bugzilla.redhat.com/show_bug.cgi?id=1791790
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_conf.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)

Reviewed-by: Peter Krempa <pkrempa at redhat.com>

Consider fixing the whitespace discrepancy though:

> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index b62dd1df52..8040f8ab3a 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1201,32 +1201,41 @@ virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver,
>  virCapsHostNUMAPtr
>  virQEMUDriverGetHostNUMACaps(virQEMUDriverPtr driver)
>  {
> +    virCapsHostNUMAPtr hostnuma;
> +
>      qemuDriverLock(driver);
>  
>      if (!driver->hostnuma)
>          driver->hostnuma = virCapabilitiesHostNUMANewHost();
>  
> +    hostnuma = driver->hostnuma;
>      qemuDriverUnlock(driver);

No newline before unlock.

>  
> -    virCapabilitiesHostNUMARef(driver->hostnuma);
> +    if (hostnuma)
> +        virCapabilitiesHostNUMARef(hostnuma);
>  
> -    return driver->hostnuma;
> +    return hostnuma;
>  }
>  
>  
>  virCPUDefPtr
>  virQEMUDriverGetHostCPU(virQEMUDriverPtr driver)
>  {
> +    virCPUDefPtr hostcpu;
> +
>      qemuDriverLock(driver);
>  
>      if (!driver->hostcpu)
>          driver->hostcpu = virCPUProbeHost(virArchFromHost());
>  
> +    hostcpu = driver->hostcpu;
> +
>      qemuDriverUnlock(driver);

Newline before unlock.

>  
> -    virCPUDefRef(driver->hostcpu);
> +    if (hostcpu)
> +        virCPUDefRef(hostcpu);
>  
> -    return driver->hostcpu;
> +    return hostcpu;
>  }
>  
>  
> -- 
> 2.24.1
> 




More information about the libvir-list mailing list