Re: [libvirt] [PATCH 1/2] conf: Check if number of vCPUs fits in the storage variable

On 01/22/2013 09:31 AM, Peter Krempa wrote: > The count of vCPUs for a domain is extracted as a usingned long variable > but is stored in a unsigned short. If the actual number was too large, > a faulty number was stored. > --- > src/conf/domain_conf.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 0b9ba13..3e95ec9 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -9085,7 +9085,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, > def->maxvcpus = 1; > } else { > def->maxvcpus = count; > - if (count == 0) { > + if (count == 0 || (unsigned short) count != count) { maxvcpus is a 'unsigned short' and count is an 'unsigned long', thus if def->maxvcpus != count after this point, then we have the overflow, right? Or would the compiler "adjust" that comparison behind our back on an if check?

> virReportError(VIR_ERR_XML_ERROR, > _("invalid maxvcpus %lu"), count); > goto error; > @@ -9101,7 +9101,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, > def->vcpus = def->maxvcpus; > } else { > def->vcpus = count; > - if (count == 0) { > + if (count == 0 || (unsigned short) count != count) { Same comment as 'maxvcpus' > virReportError(VIR_ERR_XML_ERROR, > _("invalid current vcpus %lu"), count); > goto error; > ACK - I think what you've done is right, although perhaps someone with a bit more knowledge of what the compiler does could pipe in (I'm curious too).

John