[libvirt] [PATCH 1/2] conf: Check if number of vCPUs fits in the storage variable
Peter Krempa
pkrempa at redhat.com
Tue Jan 22 16:47:36 UTC 2013
On 01/22/13 16:07, John Ferlan wrote:
> 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?
You mean changing the explicit typecast with checking of def->maxvcpus?
This works in gcc too, but I'm afraid I have -O0 as a default for
libvirt. I'm not sure if a compiler is allowed to optimize such a
comparison, but the explicit typecast is probably safer and it is more
noticeable to a possible future reader of that piece of code.
>
>> 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).
For now, I will probably push this as is. We still can fix this in the
future if there will be a cleaner solution.
Thanks for the review.
Peter
>
> John
>
More information about the libvir-list
mailing list