[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/6] conf: domain: Improve vcpus validation reporting



On 11/22/2010 02:35 PM, Cole Robinson wrote:
> 
> Signed-off-by: Cole Robinson <crobinso redhat com>
> ---
>  src/conf/domain_conf.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 11a6280..045934d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4569,7 +4569,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>          def->maxvcpus = 1;
>      } else {
>          def->maxvcpus = count;
> -        if (def->maxvcpus != count || count == 0) {
> +        if (count == 0) {

At first glance, I was about to complain: Since def->maxvcpus is an
unsigned short but count is an int, someone calling setvcpus 0x10001
will silently overflow and end up setting def->maxvcpus == 1.  In other
words, you just deleted the 'def->maxvcpus != count' overflow check...

>              virDomainReportError(VIR_ERR_XML_ERROR,
>                                   _("invalid maxvcpus %lu"), count);
>              goto error;
> @@ -4585,11 +4585,18 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>          def->vcpus = def->maxvcpus;
>      } else {
>          def->vcpus = count;
> -        if (def->vcpus != count || count == 0 || def->maxvcpus < count) {
> +        if (count == 0) {
>              virDomainReportError(VIR_ERR_XML_ERROR,
>                                   _("invalid current vcpus %lu"), count);
>              goto error;
>          }
> +
> +        if (def->maxvcpus < count) {

...but this new code is an equally effective overflow check.  No
complaint after all; def is local, so it doesn't matter if we changed
def->maxvcpus to an invalid value before detecting overflow.  Thanks for
cleaning this up for me.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]