[libvirt] [PATCH rebase v4 3/5] qemu: introduce qemuARPGetInterfaces to get IP from host's arp table

John Ferlan jferlan at redhat.com
Fri Mar 16 10:54:53 UTC 2018



On 03/08/2018 02:11 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao at gmail.com>
> 
> introduce VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP to get ip address
> of VM from the message of netlink RTM_GETNEIGH
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao at gmail.com>
> ---
> v4:
>   remove dummy entry
>   use VIR_APPEND_ELEMENT
> 
> v3:
>   add docs in virDomainInterfaceAddresses
>   remove error label
>   show network interface which did not match the arp table
> 
>  include/libvirt/libvirt-domain.h |  1 +
>  src/libvirt-domain.c             |  7 ++++
>  src/qemu/qemu_driver.c           | 72 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 80 insertions(+)
> 

More Coverity found issues...


[...]

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9e715e7a0..7d77e1643 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c

[...]

> +static int
> +qemuARPGetInterfaces(virDomainObjPtr vm,
> +                     virDomainInterfacePtr **ifaces)
> +{
> +    size_t i, j;
> +    size_t ifaces_count = 0;
> +    int ret = -1;
> +    char macaddr[VIR_MAC_STRING_BUFLEN];
> +    virDomainInterfacePtr *ifaces_ret = NULL;
> +    virDomainInterfacePtr iface = NULL;
> +    virArpTablePtr table;
> +
> +    table = virArpTableGet();
> +    if (!table)
> +        goto cleanup;
> +
> +    for (i = 0; i < vm->def->nnets; i++) {
> +        if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> +            continue;
> +
> +        virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
> +        virArpTableEntry entry;
> +        for (j = 0; j < table->n; j++) {
> +            entry = table->t[j];
> +            if (STREQ(entry.mac, macaddr)) {
> +                if (VIR_ALLOC(iface) < 0)
> +                    goto cleanup;

Prior to getting to the VIR_APPEND_ELEMENT, we can jump to cleanup and
we leak @iface and everything that's been allocated within @iface.

> +
> +                iface->naddrs = 1;
> +                if (VIR_STRDUP(iface->name, vm->def->nets[i]->ifname) < 0)
> +                    goto cleanup;
> +
> +                if (VIR_STRDUP(iface->hwaddr, macaddr) < 0)
> +                    goto cleanup;
> +
> +                if (VIR_ALLOC_N(iface->addrs, iface->naddrs) < 0)
> +                    goto cleanup;
> +
> +                if (VIR_STRDUP(iface->addrs->addr, entry.ipaddr) < 0)
> +                    goto cleanup;
> +
> +                if (VIR_APPEND_ELEMENT(ifaces_ret, ifaces_count, iface) < 0)
> +                    goto cleanup;
> +
> +            }
> +        }
> +    }
> +
> +    VIR_STEAL_PTR(*ifaces, ifaces_ret);
> +    ret = ifaces_count;
> +
> + cleanup:
> +    virArpTableFree(table);

As noted in patch2, we can get here if table == NULL when virArpTableGet
fails, so either we fix it here or in the API itself. The API should be
fixed rather than here...

> +
> +    if (ifaces_ret) {
> +        for (i = 0; i < ifaces_count; i++)
> +            virDomainInterfaceFree(ifaces_ret[i]);
> +    }
> +    VIR_FREE(ifaces_ret);
> +
> +    return ret;
> +}
> +
> +
>  static int
>  qemuDomainSetUserPassword(virDomainPtr dom,
>                            const char *user,
> 




More information about the libvir-list mailing list