[libvirt] [PATCHv5 1/3] API: add VIR_TYPED_PARAM_STRING

Eric Blake eblake at redhat.com
Tue Nov 1 23:47:40 UTC 2011


This allows strings to be transported between client and server
in the context of name-type-value virTypedParameter functions.
For compatibility,

    o new clients will not send strings to old servers, based on
      a feature check
    o new servers will not send strings to old clients without the
      flag VIR_TYPED_PARAM_STRING_OKAY; this is enforced by
      libvirt.c, so that drivers need not worry about it
    o the flag VIR_TYPED_PARAM_STRING_OKAY is set automatically,
      based on a feature check (so far, no driver implements it),
      so clients do not have to worry about it

Future patches can then enable the feature on a per-driver basis.

This patch also ensures that drivers can blindly strdup() field
names (previously, a malicious client could stuff 80 non-NUL bytes
into field and cause a read overrun).

* src/libvirt_internal.h (VIR_DRV_FEATURE_TYPED_PARAM_STRING): New
driver feature.
* src/libvirt.c (virTypedParameterValidateSet)
(virTypedParameterSanitizeGet): New helper functions.
(virDomainSetMemoryParameters, virDomainSetBlkioParameters)
(virDomainSetSchedulerParameters)
(virDomainSetSchedulerParametersFlags)
(virDomainGetMemoryParameters, virDomainGetBlkioParameters)
(virDomainGetSchedulerParameters)
(virDomainGetSchedulerParametersFlags, virDomainBlockStatsFlags):
Use them.
* src/util/util.h (virTypedParameterArrayClear): New helper
function.
* src/util/util.c (virTypedParameterArrayClear): Implement it.
* src/libvirt_private.syms (util.h): Export it.
Based on an initial patch by Hu Tao, with feedback from
Daniel P. Berrange.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 include/libvirt/libvirt.h.in |   28 ++++++++++-
 src/libvirt.c                |  119 +++++++++++++++++++++++++++++++++++++-----
 src/libvirt_internal.h       |    9 +++-
 src/libvirt_private.syms     |    1 +
 src/util/util.c              |   14 +++++
 src/util/util.h              |    2 +
 6 files changed, 156 insertions(+), 17 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 0840d46..c737e72 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -200,6 +200,8 @@ typedef virDomainControlInfo *virDomainControlInfoPtr;
  * current domain state. VIR_DOMAIN_AFFECT_LIVE requires a running
  * domain, and VIR_DOMAIN_AFFECT_CONFIG requires a persistent domain
  * (whether or not it is running).
+ *
+ * These enums should not conflict with those of virTypedParameterFlags.
  */
 typedef enum {
     VIR_DOMAIN_AFFECT_CURRENT = 0,      /* Affect current domain state.  */
@@ -489,10 +491,33 @@ typedef enum {
     VIR_TYPED_PARAM_LLONG   = 3, /* long long case */
     VIR_TYPED_PARAM_ULLONG  = 4, /* unsigned long long case */
     VIR_TYPED_PARAM_DOUBLE  = 5, /* double case */
-    VIR_TYPED_PARAM_BOOLEAN = 6  /* boolean(character) case */
+    VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */
+    VIR_TYPED_PARAM_STRING  = 7, /* string case */
 } virTypedParameterType;

 /**
+ * virTypedParameterFlags:
+ *
+ * Flags related to libvirt APIs that use virTypedParameter.
+ *
+ * These enums should not conflict with those of virDomainModificationImpact.
+ */
+typedef enum {
+    /* Older servers lacked the ability to handle string typed
+     * parameters.  Attempts to set a string parameter with an older
+     * server will fail at the client, but attempts to retrieve
+     * parameters must not return strings from a new server to an
+     * older client, so this flag exists to identify newer clients to
+     * newer servers.  This flag is automatically set when needed, so
+     * the user does not have to worry about it; however, manually
+     * setting the flag can be used to reject servers that cannot
+     * return typed strings, even if no strings would be returned.
+     */
+    VIR_TYPED_PARAM_STRING_OKAY = 1 << 2,
+
+} virTypedParameterFlags;
+
+/**
  * VIR_TYPED_PARAM_FIELD_LENGTH:
  *
  * Macro providing the field length of virTypedParameter name
@@ -520,6 +545,7 @@ struct _virTypedParameter {
         unsigned long long int ul;  /* type is ULLONG */
         double d;                   /* type is DOUBLE */
         char b;                     /* type is BOOLEAN */
+        char *s;                    /* type is STRING, may not be NULL */
     } value; /* parameter value */
 };

diff --git a/src/libvirt.c b/src/libvirt.c
index b0d1e01..5041d88 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3583,6 +3583,71 @@ error:
     return -1;
 }

+/* Helper function called to validate incoming client array on any
+ * interface that sets typed parameters in the hypervisor.  */
+static int
+virTypedParameterValidateSet(virDomainPtr domain,
+                             virTypedParameterPtr params,
+                             int nparams)
+{
+    bool string_okay;
+    int i;
+
+    string_okay = VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver,
+                                           domain->conn,
+                                           VIR_DRV_FEATURE_TYPED_PARAM_STRING);
+    for (i = 0; i < nparams; i++) {
+        if (strnlen(params[i].field, VIR_TYPED_PARAM_FIELD_LENGTH) ==
+            VIR_TYPED_PARAM_FIELD_LENGTH) {
+            virLibDomainError(VIR_ERR_INVALID_ARG,
+                              _("string parameter name '%.*s' too long"),
+                              VIR_TYPED_PARAM_FIELD_LENGTH,
+                              params[i].field);
+            virDispatchError(NULL);
+            return -1;
+        }
+        if (params[i].type == VIR_TYPED_PARAM_STRING) {
+            if (string_okay) {
+                if (!params[i].value.s) {
+                    virLibDomainError(VIR_ERR_INVALID_ARG,
+                                      _("NULL string parameter '%s'"),
+                                      params[i].field);
+                    virDispatchError(NULL);
+                    return -1;
+                }
+            } else {
+                virLibDomainError(VIR_ERR_INVALID_ARG,
+                                  _("string parameter '%s' unsupported"),
+                                  params[i].field);
+                virDispatchError(NULL);
+                return -1;
+            }
+        }
+    }
+    return 0;
+}
+
+/* Helper function called to sanitize outgoing hypervisor array on any
+ * interface that gets typed parameters for the client.  */
+static void
+virTypedParameterSanitizeGet(virTypedParameterPtr params,
+                             int *nparams,
+                             unsigned int flags)
+{
+    if (params && !(flags & VIR_TYPED_PARAM_STRING_OKAY)) {
+        int i;
+        for (i = 0; i < *nparams; i++) {
+            if (params[i].type == VIR_TYPED_PARAM_STRING) {
+                VIR_FREE(params[i].value.s);
+                memmove(&params[i], &params[i + 1], *nparams - i + 1);
+                --i;
+                --*nparams;
+                memset(&params[*nparams], 0, sizeof(*params));
+            }
+        }
+    }
+}
+
 /**
  * virDomainSetMemoryParameters:
  * @domain: pointer to domain object
@@ -3621,6 +3686,9 @@ virDomainSetMemoryParameters(virDomainPtr domain,
         virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
+    if (virTypedParameterValidateSet(domain, params, nparams) < 0)
+        return -1;
+
     conn = domain->conn;

     if (conn->driver->domainSetMemoryParameters) {
@@ -3644,7 +3712,7 @@ error:
  * @params: pointer to memory parameter object
  *          (return value, allocated by the caller)
  * @nparams: pointer to number of memory parameters; input and output
- * @flags: one of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags
  *
  * Get all memory parameters.  On input, @nparams gives the size of the
  * @params array; on output, @nparams gives how many slots were filled
@@ -3695,6 +3763,9 @@ virDomainGetMemoryParameters(virDomainPtr domain,
         virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
+    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
+        flags |= VIR_TYPED_PARAM_STRING_OKAY;
     conn = domain->conn;

     if (conn->driver->domainGetMemoryParameters) {
@@ -3702,6 +3773,7 @@ virDomainGetMemoryParameters(virDomainPtr domain,
         ret = conn->driver->domainGetMemoryParameters (domain, params, nparams, flags);
         if (ret < 0)
             goto error;
+        virTypedParameterSanitizeGet(params, nparams, flags);
         return ret;
     }
     virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
@@ -3717,7 +3789,7 @@ error:
  * @params: pointer to blkio parameter objects
  * @nparams: number of blkio parameters (this value can be the same or
  *          less than the number of parameters supported)
- * @flags: an OR'ed set of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact
  *
  * Change all or a subset of the blkio tunables.
  * This function may require privileged access to the hypervisor.
@@ -3749,6 +3821,9 @@ virDomainSetBlkioParameters(virDomainPtr domain,
         virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
+    if (virTypedParameterValidateSet(domain, params, nparams) < 0)
+        return -1;
+
     conn = domain->conn;

     if (conn->driver->domainSetBlkioParameters) {
@@ -3772,7 +3847,7 @@ error:
  * @params: pointer to blkio parameter object
  *          (return value, allocated by the caller)
  * @nparams: pointer to number of blkio parameters; input and output
- * @flags: an OR'ed set of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags
  *
  * Get all blkio parameters.  On input, @nparams gives the size of the
  * @params array; on output, @nparams gives how many slots were filled
@@ -3814,6 +3889,9 @@ virDomainGetBlkioParameters(virDomainPtr domain,
         virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
+    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
+        flags |= VIR_TYPED_PARAM_STRING_OKAY;
     conn = domain->conn;

     if (conn->driver->domainGetBlkioParameters) {
@@ -3821,6 +3899,7 @@ virDomainGetBlkioParameters(virDomainPtr domain,
         ret = conn->driver->domainGetBlkioParameters (domain, params, nparams, flags);
         if (ret < 0)
             goto error;
+        virTypedParameterSanitizeGet(params, nparams, flags);
         return ret;
     }
     virLibConnError (VIR_ERR_NO_SUPPORT, __FUNCTION__);
@@ -6384,7 +6463,6 @@ virDomainGetSchedulerParameters(virDomainPtr domain,
         virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
-
     conn = domain->conn;

     if (conn->driver->domainGetSchedulerParameters) {
@@ -6392,6 +6470,7 @@ virDomainGetSchedulerParameters(virDomainPtr domain,
         ret = conn->driver->domainGetSchedulerParameters (domain, params, nparams);
         if (ret < 0)
             goto error;
+        virTypedParameterSanitizeGet(params, nparams, 0);
         return ret;
     }

@@ -6410,7 +6489,7 @@ error:
  * @nparams: pointer to number of scheduler parameter
  *          (this value should be same than the returned value
  *           nparams of virDomainGetSchedulerType()); input and output
- * @flags: one of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags
  *
  * Get all scheduler parameters.  On input, @nparams gives the size of the
  * @params array; on output, @nparams gives how many slots were filled
@@ -6456,6 +6535,9 @@ virDomainGetSchedulerParametersFlags(virDomainPtr domain,
         goto error;
     }

+    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
+        flags |= VIR_TYPED_PARAM_STRING_OKAY;
     conn = domain->conn;

     if (conn->driver->domainGetSchedulerParametersFlags) {
@@ -6464,6 +6546,7 @@ virDomainGetSchedulerParametersFlags(virDomainPtr domain,
                                                                nparams, flags);
         if (ret < 0)
             goto error;
+        virTypedParameterSanitizeGet(params, nparams, flags);
         return ret;
     }

@@ -6505,15 +6588,17 @@ virDomainSetSchedulerParameters(virDomainPtr domain,
         return -1;
     }

+    if (domain->conn->flags & VIR_CONNECT_RO) {
+        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+        goto error;
+    }
     if (params == NULL || nparams < 0) {
         virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
+    if (virTypedParameterValidateSet(domain, params, nparams) < 0)
+        return -1;

-    if (domain->conn->flags & VIR_CONNECT_RO) {
-        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
-        goto error;
-    }
     conn = domain->conn;

     if (conn->driver->domainSetSchedulerParameters) {
@@ -6568,15 +6653,17 @@ virDomainSetSchedulerParametersFlags(virDomainPtr domain,
         return -1;
     }

+    if (domain->conn->flags & VIR_CONNECT_RO) {
+        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+        goto error;
+    }
     if (params == NULL || nparams < 0) {
         virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
+    if (virTypedParameterValidateSet(domain, params, nparams) < 0)
+        return -1;

-    if (domain->conn->flags & VIR_CONNECT_RO) {
-        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
-        goto error;
-    }
     conn = domain->conn;

     if (conn->driver->domainSetSchedulerParametersFlags) {
@@ -6665,7 +6752,7 @@ error:
  * @params: pointer to block stats parameter object
  *          (return value)
  * @nparams: pointer to number of block stats; input and output
- * @flags: unused, always pass 0
+ * @flags: bitwise-OR of virTypedParameterFlags
  *
  * This function is to get block stats parameters for block
  * devices attached to the domain.
@@ -6715,6 +6802,9 @@ int virDomainBlockStatsFlags(virDomainPtr dom,
         virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
+    if (VIR_DRV_SUPPORTS_FEATURE(dom->conn->driver, dom->conn,
+                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
+        flags |= VIR_TYPED_PARAM_STRING_OKAY;
     conn = dom->conn;

     if (conn->driver->domainBlockStatsFlags) {
@@ -6722,6 +6812,7 @@ int virDomainBlockStatsFlags(virDomainPtr dom,
         ret = conn->driver->domainBlockStatsFlags(dom, path, params, nparams, flags);
         if (ret < 0)
             goto error;
+        virTypedParameterSanitizeGet(params, nparams, flags);
         return ret;
     }
     virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
index 0117c5b..2550d76 100644
--- a/src/libvirt_internal.h
+++ b/src/libvirt_internal.h
@@ -1,7 +1,7 @@
 /*
  * libvirt.h: publically exported APIs, not for public use
  *
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2008, 2011 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -83,7 +83,12 @@ enum {
     /*
      * Support for file descriptor passing
      */
-    VIR_DRV_FEATURE_FD_PASSING = 8
+    VIR_DRV_FEATURE_FD_PASSING = 8,
+
+    /*
+     * Support for VIR_TYPED_PARAM_STRING
+     */
+    VIR_DRV_FEATURE_TYPED_PARAM_STRING = 9,
 };


diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6a1562e..2185294 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1162,6 +1162,7 @@ virStrncpy;
 virTimeMs;
 virTimestamp;
 virTrimSpaces;
+virTypedParameterArrayClear;
 virVasprintf;


diff --git a/src/util/util.c b/src/util/util.c
index bd52b21..b25f8db 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2572,3 +2572,17 @@ or other application using the libvirt API.\n\

     return 0;
 }
+
+void
+virTypedParameterArrayClear(virTypedParameterPtr params, int nparams)
+{
+    int i;
+
+    if (!params)
+        return;
+
+    for (i = 0; i < nparams; i++) {
+        if (params[i].type == VIR_TYPED_PARAM_STRING)
+            VIR_FREE(params[i].value.s);
+    }
+}
diff --git a/src/util/util.h b/src/util/util.h
index e869f1b..e5594cb 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -263,4 +263,6 @@ bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1);
 int virEmitXMLWarning(int fd,
                       const char *name,
                       const char *cmd) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
+void virTypedParameterArrayClear(virTypedParameterPtr params, int nparams);
 #endif /* __VIR_UTIL_H__ */
-- 
1.7.4.4




More information about the libvir-list mailing list