On 07/08/2013 02:08 PM, jmiao@redhat.com wrote:

From: Jincheng Miao <jmiao@redhat.com>

The index of the controller should not be less than zero. So use virStrToLong_ui() to check the controller index in virDomainControllerDefParseXML(). --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 2 +- src/vmx/vmx.c | 3 +-- 4 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 402e6e9..2831edf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5593,7 +5593,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,

idx = virXMLPropString(node, "index"); if (idx) { - if (virStrToLong_i(idx, NULL, 10, &def->idx) < 0) { + if (virStrToLong_ui(idx, NULL, 10, &def->idx) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot parse controller index %s"), idx); goto error;

I wonder if we should forbid values larger than INT_MAX as well, since virDomainDefRejectDuplicateControllers uses -1 as 'no controllers' and qemuDomainAssignPCIAddresses uses it for 'no PCI buses'. If it overflowed, this could result in some weird errors (but that many PCI bridges wouldn't be usable anyway). Jan

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index da83eb6..7897b4b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -790,7 +790,7 @@ struct _virDomainVirtioSerialOpts { /* Stores the virtual disk controller configuration */ struct _virDomainControllerDef { int type; - int idx; + unsigned int idx; int model; /* -1 == undef */ unsigned int queues; union { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 46db28a..7fd1cbf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1585,7 +1585,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,

for (i = 0; i < def->ncontrollers; i++) { if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { - if (def->controllers[i]->idx > max_idx) + if ((int)def->controllers[i]->idx > max_idx) max_idx = def->controllers[i]->idx; } } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 5464d13..deddfaa 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1664,8 +1664,7 @@ virVMXParseConfig(virVMXContext *ctx,

for (controller = 0; controller < def->ncontrollers; ++controller) { if (def->controllers[controller]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - if (def->controllers[controller]->idx < 0 || - def->controllers[controller]->idx > 3) { + if (def->controllers[controller]->idx > 3) { virReportError(VIR_ERR_INTERNAL_ERROR, _("SCSI controller index %d out of [0..3] range"), def->controllers[controller]->idx);