Re: [PATCH v10 2/4] domifaddr: Implement the remote protocol

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 03/11/2015 07:09 AM, Daniel P. Berrange wrote:
> From: Nehal J Wani <nehaljw.kkd1@xxxxxxxxx>
> 
> daemon/remote.c
>    * Define remoteSerializeDomainInterface, remoteDispatchDomainInterfaceAddresses
> 
> src/remote/remote_driver.c
>    * Define remoteDomainInterfaceAddresses
> 
> src/remote/remote_protocol.x
>    * New RPC procedure: REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES
>    * Define structs remote_domain_ip_addr, remote_domain_interface,
>      remote_domain_interfaces_addresse_args, remote_domain_interface_addresses_ret
>    * Introduce upper bounds (to handle DoS attacks):
>      REMOTE_DOMAIN_INTERFACE_MAX = 2048
>      REMOTE_DOMAIN_IP_ADDR_MAX = 2048
>      Restrictions on the maximum number of aliases per interface were
>      removed after kernel v2.0, and theoretically, at present, there
>      are no upper limits on number of interfaces per virtual machine
>      and on the number of IP addresses per interface.
> 
> src/remote_protocol-structs
>    * New structs added
> 
> Signed-off-by: Nehal J Wani <nehaljw.kkd1@xxxxxxxxx>
> ---
>  daemon/remote.c              | 124 +++++++++++++++++++++++++++++++++++++++++++
>  src/remote/remote_driver.c   | 103 +++++++++++++++++++++++++++++++++++
>  src/remote/remote_protocol.x |  37 ++++++++++++-
>  src/remote_protocol-structs  |  25 +++++++++
>  4 files changed, 288 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/remote.c b/daemon/remote.c
> index ff64eeb..f2b970a 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -6510,6 +6510,130 @@ remoteDispatchDomainGetFSInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
>  }
>  
>  
> +static int
> +remoteSerializeDomainInterface(virDomainInterfacePtr *ifaces,
> +                               unsigned int ifaces_count,
> +                               remote_domain_interface_addresses_ret *ret)
> +{
> +    size_t i, j;
> +
> +    if (ifaces_count > REMOTE_DOMAIN_INTERFACE_MAX) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Number of interfaces, %d exceeds the max limit: %d"),
> +                       ifaces_count, REMOTE_DOMAIN_INTERFACE_MAX);
> +        return -1;
> +    }
> +
> +    if (VIR_ALLOC_N(ret->ifaces.ifaces_val, ifaces_count) < 0)
> +        return -1;
> +
> +    ret->ifaces.ifaces_len = ifaces_count;
> +
> +    for (i = 0; i < ifaces_count; i++) {
> +        virDomainInterfacePtr iface = ifaces[i];
> +        remote_domain_interface *iface_ret = &(ret->ifaces.ifaces_val[i]);
> +
> +        if ((VIR_STRDUP(iface_ret->name, iface->name)) < 0)
> +            goto cleanup;
> +
> +        if ((VIR_STRDUP(iface_ret->hwaddr, iface->hwaddr)) < 0)
> +            goto cleanup;
> +
> +        if (iface->naddrs > REMOTE_DOMAIN_IP_ADDR_MAX) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Number of interfaces, %d exceeds the max limit: %d"),
> +                           iface->naddrs, REMOTE_DOMAIN_IP_ADDR_MAX);
> +            goto cleanup;
> +        }
> +
> +        if (VIR_ALLOC_N(iface_ret->addrs.addrs_val,
> +                        iface->naddrs) < 0)
> +            goto cleanup;
> +
> +        iface_ret->addrs.addrs_len = iface->naddrs;
> +
> +        for (j = 0; j < iface->naddrs; j++) {
> +            virDomainIPAddressPtr ip_addr = &(iface->addrs[j]);
> +            remote_domain_ip_addr *ip_addr_ret =
> +                &(iface_ret->addrs.addrs_val[j]);
> +
> +            if (VIR_STRDUP(ip_addr_ret->addr, ip_addr->addr) < 0)
> +                goto cleanup;
> +
> +            ip_addr_ret->prefix = ip_addr->prefix;
> +            ip_addr_ret->type = ip_addr->type;
> +        }
> +    }
> +
> +    return 0;
> +
> + cleanup:
> +    if (ret->ifaces.ifaces_val) {
> +        for (i = 0; i < ifaces_count; i++) {
> +            remote_domain_interface *iface_ret = &(ret->ifaces.ifaces_val[i]);
> +            VIR_FREE(iface_ret->name);
> +            VIR_FREE(iface_ret->hwaddr);
> +            for (j = 0; j < iface_ret->addrs.addrs_len; j++) {
> +                remote_domain_ip_addr *ip_addr =
> +                    &(iface_ret->addrs.addrs_val[j]);
> +                VIR_FREE(ip_addr->addr);
> +            }
> +        }
> +        VIR_FREE(ret->ifaces.ifaces_val);
> +    }
> +
> +    return -1;
> +}
> +
> +
> +static int
> +remoteDispatchDomainInterfaceAddresses(virNetServerPtr server ATTRIBUTE_UNUSED,
> +                                       virNetServerClientPtr client,
> +                                       virNetMessagePtr msg ATTRIBUTE_UNUSED,
> +                                       virNetMessageErrorPtr rerr,
> +                                       remote_domain_interface_addresses_args *args,
> +                                       remote_domain_interface_addresses_ret *ret)
> +{
> +    size_t i;
> +    int rv = -1;
> +    virDomainPtr dom = NULL;
> +    virDomainInterfacePtr *ifaces = NULL;
> +    int ifaces_count = 0;
> +    struct daemonClientPrivate *priv =
> +        virNetServerClientGetPrivateData(client);
> +
> +    if (!priv->conn) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
> +        goto cleanup;
> +    }
> +
> +    if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
> +        goto cleanup;
> +
> +    if ((ifaces_count = virDomainInterfaceAddresses(dom, &ifaces, args->source, args->flags)) < 0)
> +        goto cleanup;
> +
> +    if (remoteSerializeDomainInterface(ifaces, ifaces_count, ret) < 0)
> +        goto cleanup;
> +
> +    rv = 0;
> +
> + cleanup:
> +    if (rv < 0)
> +        virNetMessageSaveError(rerr);
> +
> +    virObjectUnref(dom);
> +
> +    if (ifaces && ifaces_count > 0) {
> +        for (i = 0; i < ifaces_count; i++)
> +            virDomainInterfaceFree(ifaces[i]);
> +        VIR_FREE(ifaces);
> +    }

Based on something else seen....  The VIR_FREE(ifaces) should be outside
the if {}

> +
> +    return rv;
> +}
> +
> +
>  /*----- Helpers. -----*/
>  
>  /* get_nonnull_domain and get_nonnull_network turn an on-wire
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 42dab9d..b261d9c 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -7909,6 +7909,108 @@ remoteDomainGetFSInfo(virDomainPtr dom,
>  }
>  
>  
> +static int
> +remoteDomainInterfaceAddresses(virDomainPtr dom,
> +                               virDomainInterfacePtr **ifaces,
> +                               unsigned int source,
> +                               unsigned int flags)
> +{
> +    int rv = -1;
> +    size_t i, j;
> +
> +    virDomainInterfacePtr *ifaces_ret = NULL;
> +    remote_domain_interface_addresses_args args;
> +    remote_domain_interface_addresses_ret ret;
> +
> +    struct private_data *priv = dom->conn->privateData;
> +
> +    args.source = source;
> +    args.flags = flags;
> +    make_nonnull_domain(&args.dom, dom);
> +
> +    remoteDriverLock(priv);
> +
> +    memset(&ret, 0, sizeof(ret));
> +
> +    if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES,
> +             (xdrproc_t)xdr_remote_domain_interface_addresses_args,
> +             (char *)&args,
> +             (xdrproc_t)xdr_remote_domain_interface_addresses_ret,
> +             (char *)&ret) == -1) {
> +        goto done;
> +    }
> +
> +    if (ret.ifaces.ifaces_len > REMOTE_DOMAIN_INTERFACE_MAX) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Number of interfaces, %d exceeds the max limit: %d"),
> +                       ret.ifaces.ifaces_len, REMOTE_DOMAIN_INTERFACE_MAX);
> +        goto cleanup;
> +    }
> +
> +    if (ret.ifaces.ifaces_len &&
> +        VIR_ALLOC_N(ifaces_ret, ret.ifaces.ifaces_len) < 0)
> +        goto cleanup;
> +
> +    for (i = 0; i < ret.ifaces.ifaces_len; i++) {
> +        virDomainInterfacePtr iface;
> +        remote_domain_interface *iface_ret = &(ret.ifaces.ifaces_val[i]);
> +
> +        if (VIR_ALLOC(ifaces_ret[i]) < 0)
> +            goto cleanup;
> +
> +        iface = ifaces_ret[i];
> +
> +        if (VIR_STRDUP(iface->name, iface_ret->name) < 0)
> +            goto cleanup;
> +
> +        if (VIR_STRDUP(iface->hwaddr, iface_ret->hwaddr) < 0)
> +            goto cleanup;
> +
> +        if (iface_ret->addrs.addrs_len > REMOTE_DOMAIN_IP_ADDR_MAX) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Number of interfaces, %d exceeds the max limit: %d"),
> +                           iface_ret->addrs.addrs_len, REMOTE_DOMAIN_IP_ADDR_MAX);
> +            goto cleanup;
> +        }
> +
> +        iface->naddrs = iface_ret->addrs.addrs_len;
> +
> +        if (iface->naddrs) {
> +            if (VIR_ALLOC_N(iface->addrs, iface->naddrs) < 0)
> +                goto cleanup;
> +
> +           for (j = 0; j < iface->naddrs; j++) {
> +                virDomainIPAddressPtr ip_addr = &(iface->addrs[j]);
> +                remote_domain_ip_addr *ip_addr_ret =
> +                    &(iface_ret->addrs.addrs_val[j]);
> +
> +                if (VIR_STRDUP(ip_addr->addr, ip_addr_ret->addr) < 0)
> +                    goto cleanup;
> +
> +                ip_addr->prefix = ip_addr_ret->prefix;
> +                ip_addr->type = ip_addr_ret->type;
> +            }
> +        }
> +    }
> +    *ifaces = ifaces_ret;
> +    ifaces_ret = NULL;
> +
> +    rv = ret.ifaces.ifaces_len;
> +
> + cleanup:
> +    if (ifaces_ret) {
> +        for (i = 0; i < ret.ifaces.ifaces_len; i++)
> +            virDomainInterfaceFree(ifaces_ret[i]);
> +        VIR_FREE(ifaces_ret);
> +    }
> +    xdr_free((xdrproc_t)xdr_remote_domain_interface_addresses_ret,
> +             (char *) &ret);
> + done:
> +    remoteDriverUnlock(priv);
> +    return rv;
> +}
> +
> +
>  /* get_nonnull_domain and get_nonnull_network turn an on-wire
>   * (name, uuid) pair into virDomainPtr or virNetworkPtr object.
>   * These can return NULL if underlying memory allocations fail,
> @@ -8253,6 +8355,7 @@ static virHypervisorDriver hypervisor_driver = {
>      .connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* 1.2.8 */
>      .nodeAllocPages = remoteNodeAllocPages, /* 1.2.9 */
>      .domainGetFSInfo = remoteDomainGetFSInfo, /* 1.2.11 */
> +    .domainInterfaceAddresses = remoteDomainInterfaceAddresses, /* 1.2.14 */
>  };
>  
>  static virNetworkDriver network_driver = {
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index 4ea535e..b511b00 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -259,6 +259,12 @@ const REMOTE_DOMAIN_FSINFO_MAX = 256;
>  /* Upper limit on number of disks per mountpoint in fsinfo */
>  const REMOTE_DOMAIN_FSINFO_DISKS_MAX = 256;
>  
> +/* Upper limit on number of interfaces per domain */
> +const REMOTE_DOMAIN_INTERFACE_MAX = 2048;
> +
> +/* Upper limit on number of IP addresses per interface */
> +const REMOTE_DOMAIN_IP_ADDR_MAX = 2048;
> +
>  /* UUID.  VIR_UUID_BUFLEN definition comes from libvirt.h */
>  typedef opaque remote_uuid[VIR_UUID_BUFLEN];
>  
> @@ -3170,6 +3176,29 @@ struct remote_domain_get_fsinfo_ret {
>      unsigned int ret;
>  };
>  
> +struct remote_domain_ip_addr {
> +    int type;
> +    remote_nonnull_string addr;
> +    unsigned int prefix;
> +};
> +
> +struct remote_domain_interface {
> +    remote_nonnull_string name;
> +    remote_nonnull_string hwaddr;
> +    remote_domain_ip_addr addrs<REMOTE_DOMAIN_IP_ADDR_MAX>;
> +};
> +
> +struct remote_domain_interface_addresses_args {
> +    remote_nonnull_domain dom;
> +    unsigned int source;
> +    unsigned int flags;
> +};
> +
> +struct remote_domain_interface_addresses_ret {
> +    remote_domain_interface ifaces<REMOTE_DOMAIN_INTERFACE_MAX>;
> +};
> +
> +
>  /*----- Protocol. -----*/
>  
>  /* Define the program number, protocol version and procedure numbers here. */
> @@ -5593,5 +5622,11 @@ enum remote_procedure {
>       * @generate: none
>       * @acl: domain:read
>       */
> -    REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351
> +    REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351,
> +
> +    /**
> +     * @generate: none
> +     * @acl: domain:read
> +     */
> +    REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 352
>  };

Merge conflict here.

> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
> index 907bcd4..632576b 100644
> --- a/src/remote_protocol-structs
> +++ b/src/remote_protocol-structs
> @@ -2630,6 +2630,30 @@ struct remote_domain_get_fsinfo_ret {
>          } info;
>          u_int                      ret;
>  };
> +struct remote_domain_ip_addr {
> +        int                        type;
> +        remote_nonnull_string      addr;
> +        u_int                      prefix;
> +};
> +struct remote_domain_interface {
> +        remote_nonnull_string      name;
> +        remote_nonnull_string      hwaddr;
> +        struct {
> +                u_int              addrs_len;
> +                remote_domain_ip_addr * addrs_val;
> +        } addrs;
> +};
> +struct remote_domain_interface_addresses_args {
> +        remote_nonnull_domain      dom;
> +        u_int                      source;
> +        u_int                      flags;
> +};
> +struct remote_domain_interface_addresses_ret {
> +        struct {
> +                u_int              ifaces_len;
> +                remote_domain_interface * ifaces_val;
> +        } ifaces;
> +};
>  enum remote_procedure {
>          REMOTE_PROC_CONNECT_OPEN = 1,
>          REMOTE_PROC_CONNECT_CLOSE = 2,
> @@ -2982,4 +3006,5 @@ enum remote_procedure {
>          REMOTE_PROC_DOMAIN_GET_FSINFO = 349,
>          REMOTE_PROC_DOMAIN_DEFINE_XML_FLAGS = 350,
>          REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO = 351,
> +        REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 352,

Another merge conflict here.

>  };
> 


ACK - with the cleanups

John

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]