[libvirt] [PATCH 2/4] remote: Check for limits when encoding typed params
Michal Privoznik
mprivozn at redhat.com
Tue Aug 27 12:05:07 UTC 2019
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 at redhat.com>
---
src/admin/admin_remote.c | 2 ++
src/admin/admin_server_dispatch.c | 27 ++-------------
src/remote/remote_daemon_dispatch.c | 54 ++++++++++-------------------
src/remote/remote_driver.c | 48 ++++---------------------
src/rpc/gendispatch.pl | 2 ++
src/util/virtypedparam.c | 13 ++++++-
src/util/virtypedparam.h | 1 +
7 files changed, 45 insertions(+), 102 deletions(-)
diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c
index e112c1f115..0846e72f6d 100644
--- a/src/admin/admin_remote.c
+++ b/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)
diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c
index 1973664488..3c4d72dedb 100644
--- a/src/admin/admin_server_dispatch.c
+++ b/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, ¶ms, &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, ¶ms, &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;
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 1a002957ef..e205c743c3 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/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, ¶ms, &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, ¶ms, &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, ¶ms, &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, ¶ms, &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, ¶ms, &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)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 8cc7ab3663..2b86f55035 100644
--- a/src/remote/remote_driver.c
+++ b/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) {
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index ff21834091..9d3cc5b145 100755
--- a/src/rpc/gendispatch.pl
+++ b/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" .
diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index 8f23348d97..779714d146 100644
--- a/src/util/virtypedparam.c
+++ b/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;
diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h
index ce4e75b389..f9d22b24fb 100644
--- a/src/util/virtypedparam.h
+++ b/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);
--
2.21.0
More information about the libvir-list
mailing list