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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list