API: add VIR_TYPED_PARAM_STRING

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 will be enforced at
      the RPC layer in the next patch, so that drivers need not
      worry about it in general.  The one exception is that
      virDomainGetSchedulerParameters lacks a flags argument, so
      it must not return a string; drivers that forward that
      function on to virDomainGetSchedulerParametersFlags will
      have to pay attention to the flag.
    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@redhat.com>

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index aa320b6f5e88af5ba706854add5cc71c0b9feb70..2ab89f5f355cbe45f82435481f105f17d023ae59 100644 (file)
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -200,11 +200,14 @@ 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.  */
     VIR_DOMAIN_AFFECT_LIVE    = 1 << 0, /* Affect running domain state.  */
     VIR_DOMAIN_AFFECT_CONFIG  = 1 << 1, /* Affect persistent domain state.  */
+    /* 1 << 2 is reserved for virTypedParameterFlags */
 } virDomainModificationImpact;
 
 /**
@@ -489,9 +492,35 @@ 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 {
+    /* 1 << 0 is reserved for virDomainModificationImpact */
+    /* 1 << 1 is reserved for virDomainModificationImpact */
+
+    /* 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:
  *
@@ -520,6 +549,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 b0d1e014ab54ee897582ae5aea48676516d4c190..1518ed20fef8437257c5faa2f588252da5448d40 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3583,6 +3583,50 @@ 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;
+}
+
 /**
  * virDomainSetMemoryParameters:
  * @domain: pointer to domain object
@@ -3621,6 +3665,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 +3691,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 +3742,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) {
@@ -3717,7 +3767,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 +3799,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 +3825,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 +3867,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) {
@@ -6410,7 +6466,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 +6512,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) {
@@ -6505,15 +6564,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 +6629,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 +6728,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 +6778,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) {

diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
index 0117c5b1ca15f953685a2ae41732cad123725bbd..2550d767c88ea423cd5a9a6ba075b4e24bb09a97 100644 (file)
--- 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 ebe34c9029337d5abdc59a0c1f59f278effa5816..b15a8db92188ee6c2f7deb895d0dc4fa05db3e05 100644 (file)
--- 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 959c224ebbb8fa7254b0e19a9f97259d32046b9a..9ecfa9dee8ae588e21cf0fc91a3ed957f1533d33 100644 (file)
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2607,3 +2607,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 d8176a8737115356eab62c9101f6eac04fb3438c..3295ce8b5555cf1cf0d633ee33893802a8cba5ea 100644 (file)
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -258,4 +258,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__ */