5:29 a.m.
On Thu, 2016-07-14 at 11:22 +0100, Daniel P. Berrange wrote:
When virConf 'l' field is a 'signed long long', so
whenever
s/When//
the 'type' field is VIR_CONF_ULONG, we should explicitly
cast
'l' to a 'unsigned long long' before doing range checks.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/virconf.c | 81 +++++++++++++++++++++++++++++++++---------------------
1 file changed, 49 insertions(+), 32 deletions(-)
This doesn't apply any longer after b7caf4fcd2ab, but rebasing
it is trivial.
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 5085768..f82d114 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1208,21 +1208,28 @@ int virConfGetValueSizeT(virConfPtr conf,
if (!cval)
return 0;
- if (cval->type != VIR_CONF_ULONG) {
+ if (cval->type == VIR_CONF_LONG) {
+ if (cval->l < 0 || cval->l > SIZE_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: value for '%s' parameter must be in range
0:%zu"),
+ conf->filename, setting, SIZE_MAX);
+ return -1;
+ }
+ } else if (cval->type == VIR_CONF_ULONG) {
+ if (((unsigned long long)cval->l) > SIZE_MAX) {
I personally prefer
(type) var
when doing casts, but I won't oppose it if you prefer to
keep it this way. Note, though, that the other style is
already used in the very same module...
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: value for '%s' parameter must be in range
0:%zu"),
+ conf->filename, setting, SIZE_MAX);
+ return -1;
+ }
+ } else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: expected an unsigned integer for '%s'
parameter"),
conf->filename, setting);
return -1;
}
- if (cval->l > SIZE_MAX || cval->l < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: value for '%s' parameter must be in range
0:%zu"),
- conf->filename, setting, SIZE_MAX);
- return -1;
- }
-
- *value = cval->l;
+ *value = (size_t)cval->l;
return 1;
}
@@ -1256,22 +1263,28 @@ int virConfGetValueSSizeT(virConfPtr conf,
if (!cval)
return 0;
- if (cval->type != VIR_CONF_LONG &&
- cval->type != VIR_CONF_ULONG) {
+ if (cval->type == VIR_CONF_ULONG) {
+ if (((unsigned long long)cval->l) > SSIZE_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: value for '%s' parameter must be in range
%zd:%zd"),
+ conf->filename, setting, -SSIZE_MAX - 1, SSIZE_MAX);
These need of course to be worked around same as b7caf4fcd2ab
not to break building on 32 bit.
+ return -1;
+ }
+ } else if (cval->type == VIR_CONF_LONG) {
+ if (cval->l < (-SSIZE_MAX - 1) || cval->l > SSIZE_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: value for '%s' parameter must be in range
%zd:%zd"),
+ conf->filename, setting, -SSIZE_MAX - 1, SSIZE_MAX);
Same here.
+ return -1;
+ }
+ } else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: expected a signed integer for '%s'
parameter"),
conf->filename, setting);
return -1;
}
- if (cval->l > SSIZE_MAX || cval->l < (-SSIZE_MAX - 1)) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: value for '%s' parameter must be in range
%zd:%zd"),
- conf->filename, setting, -SSIZE_MAX - 1, SSIZE_MAX);
- return -1;
- }
-
- *value = cval->l;
+ *value = (ssize_t)cval->l;
return 1;
}
@@ -1305,22 +1318,20 @@ int virConfGetValueLLong(virConfPtr conf,
if (!cval)
return 0;
- if (cval->type != VIR_CONF_LONG &&
- cval->type != VIR_CONF_ULONG) {
+ if (cval->type == VIR_CONF_ULONG) {
+ if (((unsigned long long)cval->l) > LLONG_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: value for '%s' parameter must be in range
0:%lld"),
+ conf->filename, setting, LLONG_MAX);
+ return -1;
+ }
+ } else if (cval->type != VIR_CONF_LONG) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: expected a signed integer for '%s'
parameter"),
conf->filename, setting);
return -1;
}
- if (cval->type == VIR_CONF_ULONG &&
- cval->l > LLONG_MAX) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: value for '%s' parameter must be in range
0:%lld"),
- conf->filename, setting, LLONG_MAX);
- return -1;
- }
-
*value = cval->l;
return 1;
@@ -1354,15 +1365,21 @@ int virConfGetValueULLong(virConfPtr conf,
if (!cval)
return 0;
- if (cval->type != VIR_CONF_LONG &&
- cval->type != VIR_CONF_ULONG) {
+ if (cval->type == VIR_CONF_LONG) {
+ if (cval->l < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: value for '%s' parameter must be in range
0:%llu"),
+ conf->filename, setting, ULLONG_MAX);
+ return -1;
+ }
+ } else if (cval->type != VIR_CONF_ULONG) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: expected an unsigned integer for '%s'
parameter"),
conf->filename, setting);
return -1;
}
- *value = cval->l;
+ *value = (unsigned long long)cval->l;
return 1;
}
ACK with the above taken care of.
--
Andrea Bolognani / Red Hat / Virtualization