[libvirt] [PATCH] domain controller index check

[Ján Tomko]

On 07/08/2013 02:08 PM, jmiao at redhat.com wrote:
> From: Jincheng Miao <jmiao at 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);
>