[libvirt] [PATCH] conf: Catch memory size overflow earlier

virDomainParseMemory parses the size and then rounds up while converting it to kibibytes. Since the number is limit-checked before the rounding it's possible to use a number that would be correctly parsed the first time, but not the second time. For numbers not limited to 32 bit systems the magic is 9223372036854775807 bytes. That number then can't be parsed back in kibibytes. To solve the issue add a second overflow check for the few values that would cause the problem. Since virDomainParseMemory is used in config parsing, this avoids vanishing VMs. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1221504 --- src/conf/domain_conf.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bfdc94e..ba4f430 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7237,6 +7237,13 @@ virDomainParseMemory(const char *xpath, /* Yes, we really do use kibibytes for our internal sizing. */ *mem = VIR_DIV_UP(bytes, 1024); + + if (*mem >= VIR_DIV_UP(max, 1024)) { + virReportError(VIR_ERR_OVERFLOW, "%s", _("size value too large")); + ret = -1; + goto cleanup; + } + ret = 0; cleanup: return ret; -- 2.3.5

On 19.05.2015 17:05, Peter Krempa wrote:
virDomainParseMemory parses the size and then rounds up while converting it to kibibytes. Since the number is limit-checked before the rounding it's possible to use a number that would be correctly parsed the first time, but not the second time. For numbers not limited to 32 bit systems the magic is 9223372036854775807 bytes. That number then can't be parsed back in kibibytes.
To solve the issue add a second overflow check for the few values that would cause the problem. Since virDomainParseMemory is used in config parsing, this avoids vanishing VMs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1221504 --- src/conf/domain_conf.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bfdc94e..ba4f430 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7237,6 +7237,13 @@ virDomainParseMemory(const char *xpath,
/* Yes, we really do use kibibytes for our internal sizing. */ *mem = VIR_DIV_UP(bytes, 1024); + + if (*mem >= VIR_DIV_UP(max, 1024)) { + virReportError(VIR_ERR_OVERFLOW, "%s", _("size value too large")); + ret = -1; + goto cleanup; + } + ret = 0; cleanup: return ret;
ACK Michal

On Wed, May 20, 2015 at 14:20:04 +0200, Michal Privoznik wrote:
On 19.05.2015 17:05, Peter Krempa wrote:
virDomainParseMemory parses the size and then rounds up while converting it to kibibytes. Since the number is limit-checked before the rounding it's possible to use a number that would be correctly parsed the first time, but not the second time. For numbers not limited to 32 bit systems the magic is 9223372036854775807 bytes. That number then can't be parsed back in kibibytes.
To solve the issue add a second overflow check for the few values that would cause the problem. Since virDomainParseMemory is used in config parsing, this avoids vanishing VMs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1221504
ACK
Pushed; Thanks. Peter
participants (2)
-
Michal Privoznik
-
Peter Krempa