remote: Check for limits when encoding typed params

The same way we check for limits when decoding typed parameters
(virTypedParamsDeserialize()) we should do the same check when
serializing them so that we don't put onto the wire more than our
limits allow. Surprisingly, we were doing so explicitly in some
places but not all of them.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>

src/admin/admin_remote.c

@@ -294,6 +294,7 @@ remoteAdminServerSetThreadPoolParameters(virAdmServerPtr srv,
     virObjectLock(priv);
 
     if (virTypedParamsSerialize(params, nparams,
+                                ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &args.params.params_val,
                                 &args.params.params_len,
                                 0) < 0)
@@ -405,6 +406,7 @@ remoteAdminServerSetClientLimits(virAdmServerPtr srv,
     virObjectLock(priv);
 
     if (virTypedParamsSerialize(params, nparams,
+                                ADMIN_SERVER_CLIENT_LIMITS_MAX,
                                 (virTypedParameterRemotePtr *) &args.params.params_val,
                                 &args.params.params_len,
                                 0) < 0)

src/admin/admin_server_dispatch.c

@@ -237,15 +237,8 @@ adminDispatchServerGetThreadpoolParameters(virNetServerPtr server ATTRIBUTE_UNUS
                                            args->flags) < 0)
         goto cleanup;
 
-    if (nparams > ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Number of threadpool parameters %d exceeds max "
-                         "allowed limit: %d"), nparams,
-                       ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX);
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len, 0) < 0)
         goto cleanup;
@@ -336,15 +329,8 @@ adminDispatchClientGetInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
     if (adminClientGetInfo(clnt, &params, &nparams, args->flags) < 0)
         goto cleanup;
 
-    if (nparams > ADMIN_CLIENT_INFO_PARAMETERS_MAX) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Number of client info parameters %d exceeds max "
-                         "allowed limit: %d"), nparams,
-                       ADMIN_CLIENT_INFO_PARAMETERS_MAX);
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                ADMIN_CLIENT_INFO_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0)
@@ -383,15 +369,8 @@ adminDispatchServerGetClientLimits(virNetServerPtr server ATTRIBUTE_UNUSED,
     if (adminServerGetClientLimits(srv, &params, &nparams, args->flags) < 0)
         goto cleanup;
 
-    if (nparams > ADMIN_SERVER_CLIENT_LIMITS_MAX) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Number of client processing parameters %d exceeds "
-                         "max allowed limit: %d"), nparams,
-                       ADMIN_SERVER_CLIENT_LIMITS_MAX);
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                ADMIN_SERVER_CLIENT_LIMITS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len, 0) < 0)
         goto cleanup;

src/remote/remote_daemon_dispatch.c

@@ -1152,6 +1152,7 @@ remoteRelayDomainEventTunable(virConnectPtr conn,
         goto error;
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_EVENT_TUNABLE_MAX,
                                 (virTypedParameterRemotePtr *) &data.params.params_val,
                                 &data.params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0) {
@@ -1318,6 +1319,7 @@ remoteRelayDomainEventJobCompleted(virConnectPtr conn,
         goto error;
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_JOB_STATS_MAX,
                                 (virTypedParameterRemotePtr *) &data.params.params_val,
                                 &data.params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0) {
@@ -2450,6 +2452,7 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUS
         goto cleanup;
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 0) < 0)
@@ -2498,6 +2501,7 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server ATTRIBUTE
         goto cleanup;
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -2663,6 +2667,7 @@ remoteDispatchDomainBlockStatsFlags(virNetServerPtr server ATTRIBUTE_UNUSED,
 
     /* Serialize the block stats. */
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -3292,6 +3297,7 @@ remoteDispatchDomainGetMemoryParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
     }
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -3351,6 +3357,7 @@ remoteDispatchDomainGetNumaParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
     }
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_NUMA_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 flags) < 0)
@@ -3410,6 +3417,7 @@ remoteDispatchDomainGetBlkioParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
     }
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -3587,12 +3595,8 @@ remoteDispatchDomainGetLaunchSecurityInfo(virNetServerPtr server ATTRIBUTE_UNUSE
     if (virDomainGetLaunchSecurityInfo(dom, &params, &nparams, args->flags) < 0)
         goto cleanup;
 
-    if (nparams > REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -3631,12 +3635,8 @@ remoteDispatchDomainGetPerfEvents(virNetServerPtr server ATTRIBUTE_UNUSED,
     if (virDomainGetPerfEvents(dom, &params, &nparams, args->flags) < 0)
         goto cleanup;
 
-    if (nparams > REMOTE_DOMAIN_PERF_EVENTS_MAX) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_PERF_EVENTS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 0) < 0)
@@ -3732,6 +3732,7 @@ remoteDispatchDomainGetBlockIoTune(virNetServerPtr server ATTRIBUTE_UNUSED,
 
     /* Serialize the block I/O tuning parameters. */
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -5289,6 +5290,7 @@ remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server ATTRIBUTE_UNUS
     }
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 flags) < 0)
@@ -5348,6 +5350,7 @@ remoteDispatchDomainGetCPUStats(virNetServerPtr server ATTRIBUTE_UNUSED,
         goto success;
 
     if (virTypedParamsSerialize(params, args->nparams * args->ncpus,
+                                REMOTE_DOMAIN_GET_CPU_STATS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -5450,13 +5453,8 @@ remoteDispatchNodeGetSevInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
     if (virNodeGetSEVInfo(conn, &params, &nparams, args->flags) < 0)
         goto cleanup;
 
-    if (nparams > REMOTE_NODE_SEV_INFO_MAX) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
-        goto cleanup;
-    }
-
-
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_NODE_SEV_INFO_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -5511,6 +5509,7 @@ remoteDispatchNodeGetMemoryParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
     }
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_NODE_MEMORY_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -5641,14 +5640,8 @@ remoteDispatchDomainGetJobStats(virNetServerPtr server ATTRIBUTE_UNUSED,
                              &nparams, args->flags) < 0)
         goto cleanup;
 
-    if (nparams > REMOTE_DOMAIN_JOB_STATS_MAX) {
-        virReportError(VIR_ERR_RPC,
-                       _("Too many job stats '%d' for limit '%d'"),
-                       nparams, REMOTE_DOMAIN_JOB_STATS_MAX);
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_JOB_STATS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 0) < 0)
@@ -7018,6 +7011,7 @@ remoteDispatchConnectGetAllDomainStats(virNetServerPtr server ATTRIBUTE_UNUSED,
 
             if (virTypedParamsSerialize(retStats[i]->params,
                                         retStats[i]->nparams,
+                                        REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX,
                                         (virTypedParameterRemotePtr *) &dst->params.params_val,
                                         &dst->params.params_len,
                                         VIR_TYPED_PARAM_STRING_OKAY) < 0)
@@ -7370,12 +7364,8 @@ remoteDispatchNetworkPortGetParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
     if (virNetworkPortGetParameters(port, &params, &nparams, args->flags) < 0)
         goto cleanup;
 
-    if (nparams > REMOTE_NETWORK_PORT_PARAMETERS_MAX) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_NETWORK_PORT_PARAMETERS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 args->flags) < 0)
@@ -7674,14 +7664,8 @@ remoteDispatchDomainGetGuestInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
     if (virDomainGetGuestInfo(dom, args->types, &params, &nparams, args->flags) < 0)
         goto cleanup;
 
-    if (nparams > REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX) {
-        virReportError(VIR_ERR_RPC,
-                       _("Too many params in guestinfo: %d for limit %d"),
-                       nparams, REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX);
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX,
                                 (virTypedParameterRemotePtr *) &ret->params.params_val,
                                 &ret->params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0)

src/remote/remote_driver.c

@@ -7109,14 +7109,8 @@ remoteDomainMigrateBegin3Params(virDomainPtr domain,
     make_nonnull_domain(&args.dom, domain);
     args.flags = flags;
 
-    if (nparams > REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX) {
-        virReportError(VIR_ERR_RPC,
-                       _("Too many migration parameters '%d' for limit '%d'"),
-                       nparams, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX);
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX,
                                 (virTypedParameterRemotePtr *) &args.params.params_val,
                                 &args.params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0) {
@@ -7177,14 +7171,8 @@ remoteDomainMigratePrepare3Params(virConnectPtr dconn,
     memset(&args, 0, sizeof(args));
     memset(&ret, 0, sizeof(ret));
 
-    if (nparams > REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX) {
-        virReportError(VIR_ERR_RPC,
-                       _("Too many migration parameters '%d' for limit '%d'"),
-                       nparams, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX);
-        goto cleanup;
-    }
-
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX,
                                 (virTypedParameterRemotePtr *) &args.params.params_val,
                                 &args.params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0) {
@@ -7261,18 +7249,12 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr dconn,
     memset(&args, 0, sizeof(args));
     memset(&ret, 0, sizeof(ret));
 
-    if (nparams > REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX) {
-        virReportError(VIR_ERR_RPC,
-                       _("Too many migration parameters '%d' for limit '%d'"),
-                       nparams, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX);
-        goto cleanup;
-    }
-
     args.cookie_in.cookie_in_val = (char *)cookiein;
     args.cookie_in.cookie_in_len = cookieinlen;
     args.flags = flags;
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX,
                                 (virTypedParameterRemotePtr *) &args.params.params_val,
                                 &args.params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0) {
@@ -7351,13 +7333,6 @@ remoteDomainMigratePerform3Params(virDomainPtr dom,
     memset(&args, 0, sizeof(args));
     memset(&ret, 0, sizeof(ret));
 
-    if (nparams > REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX) {
-        virReportError(VIR_ERR_RPC,
-                       _("Too many migration parameters '%d' for limit '%d'"),
-                       nparams, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX);
-        goto cleanup;
-    }
-
     make_nonnull_domain(&args.dom, dom);
     args.dconnuri = dconnuri == NULL ? NULL : (char **) &dconnuri;
     args.cookie_in.cookie_in_val = (char *)cookiein;
@@ -7365,6 +7340,7 @@ remoteDomainMigratePerform3Params(virDomainPtr dom,
     args.flags = flags;
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX,
                                 (virTypedParameterRemotePtr *) &args.params.params_val,
                                 &args.params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0) {
@@ -7425,19 +7401,13 @@ remoteDomainMigrateFinish3Params(virConnectPtr dconn,
     memset(&args, 0, sizeof(args));
     memset(&ret, 0, sizeof(ret));
 
-    if (nparams > REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX) {
-        virReportError(VIR_ERR_RPC,
-                       _("Too many migration parameters '%d' for limit '%d'"),
-                       nparams, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX);
-        goto cleanup;
-    }
-
     args.cookie_in.cookie_in_val = (char *)cookiein;
     args.cookie_in.cookie_in_len = cookieinlen;
     args.flags = flags;
     args.cancelled = cancelled;
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX,
                                 (virTypedParameterRemotePtr *) &args.params.params_val,
                                 &args.params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0) {
@@ -7499,13 +7469,6 @@ remoteDomainMigrateConfirm3Params(virDomainPtr domain,
 
     memset(&args, 0, sizeof(args));
 
-    if (nparams > REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX) {
-        virReportError(VIR_ERR_RPC,
-                       _("Too many migration parameters '%d' for limit '%d'"),
-                       nparams, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX);
-        goto cleanup;
-    }
-
     make_nonnull_domain(&args.dom, domain);
     args.cookie_in.cookie_in_len = cookieinlen;
     args.cookie_in.cookie_in_val = (char *) cookiein;
@@ -7513,6 +7476,7 @@ remoteDomainMigrateConfirm3Params(virDomainPtr domain,
     args.cancelled = cancelled;
 
     if (virTypedParamsSerialize(params, nparams,
+                                REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX,
                                 (virTypedParameterRemotePtr *) &args.params.params_val,
                                 &args.params.params_len,
                                 VIR_TYPED_PARAM_STRING_OKAY) < 0) {

src/rpc/gendispatch.pl

@@ -951,6 +951,7 @@ elsif ($mode eq "server") {
                     splice(@args_list, int($5), 0, "&$1_len");
 
                     push(@ret_list, "if (virTypedParamsSerialize($1, $1_len,\n" .
+                                    "                                $2,\n" .
                                     "                                (virTypedParameterRemotePtr *) &ret->$1.$1_val,\n" .
                                     "                                &ret->$1.$1_len,\n" .
                                     "                                VIR_TYPED_PARAM_STRING_OKAY) < 0)\n" .
@@ -1436,6 +1437,7 @@ elsif ($mode eq "client") {
                     push(@args_list, "virTypedParameterPtr $1");
                     push(@args_list, "int n$1");
                     push(@setters_list2, "if (virTypedParamsSerialize($1, n$1,\n" .
+                                         "                                $2,\n" .
                                          "                                (virTypedParameterRemotePtr *) &args.$1.$1_val,\n" .
                                          "                                &args.$1.$1_len,\n" .
                                          "                                VIR_TYPED_PARAM_STRING_OKAY) < 0) {\n" .

src/util/virtypedparam.c

@@ -1480,12 +1480,15 @@ virTypedParamsDeserialize(virTypedParameterRemotePtr remote_params,
  * virTypedParamsSerialize:
  * @params: array of parameters to be serialized and later sent to remote side
  * @nparams: number of elements in @params
+ * @limit: user specified maximum limit to @remote_params_len
  * @remote_params_val: protocol independent remote representation of @params
  * @remote_params_len: the final number of elements in @remote_params_val
  * @flags: bitwise-OR of virTypedParameterFlags
  *
  * This method serializes typed parameters provided by @params into
  * @remote_params_val which is the representation actually being sent.
+ * It also checks, if the @limit imposed by RPC on the maximum number of
+ * parameters is not exceeded.
  *
  * Server side using this method also filters out any string parameters that
  * must not be returned to older clients and handles possibly sparse arrays
@@ -1496,6 +1499,7 @@ virTypedParamsDeserialize(virTypedParameterRemotePtr remote_params,
 int
 virTypedParamsSerialize(virTypedParameterPtr params,
                         int nparams,
+                        int limit,
                         virTypedParameterRemotePtr *remote_params_val,
                         unsigned int *remote_params_len,
                         unsigned int flags)
@@ -1503,9 +1507,16 @@ virTypedParamsSerialize(virTypedParameterPtr params,
     size_t i;
     size_t j;
     int rv = -1;
-    virTypedParameterRemotePtr params_val;
+    virTypedParameterRemotePtr params_val = NULL;
     int params_len = nparams;
 
+    if (nparams > limit) {
+        virReportError(VIR_ERR_RPC,
+                       _("too many parameters '%d' for limit '%d'"),
+                       nparams, limit);
+        goto cleanup;
+    }
+
     if (VIR_ALLOC_N(params_val, nparams) < 0)
         goto cleanup;
 

src/util/virtypedparam.h

@@ -113,6 +113,7 @@ int virTypedParamsDeserialize(virTypedParameterRemotePtr remote_params,
 
 int virTypedParamsSerialize(virTypedParameterPtr params,
                             int nparams,
+                            int limit,
                             virTypedParameterRemotePtr *remote_params_val,
                             unsigned int *remote_params_len,
                             unsigned int flags);