[libvirt] [PATCH] qemu: Check the unsigned integer overflow
As perhaps other hypervisor drivers use different capacity units, do the checking in qemu driver instead of in conf/domain_conf.c. --- src/qemu/qemu_command.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 198a4e2..42be6ee 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1933,6 +1933,13 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video, virBufferVSprintf(&buf, ",id=%s", video->info.alias); if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram > (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + goto error; + } + /* QEMU accepts bytes for vram_size. */ virBufferVSprintf(&buf, ",vram_size=%u", video->vram * 1024); } -- 1.7.4
As perhaps other hypervisor drivers use different capacity units, do the checking in qemu driver instead of in conf/domain_conf.c. --- src/qemu/qemu_command.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 198a4e2..59fd2ac 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1933,6 +1933,13 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video, virBufferVSprintf(&buf, ",id=%s", video->info.alias); if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram > (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + goto error; + } + /* QEMU accepts bytes for vram_size. */ virBufferVSprintf(&buf, ",vram_size=%u", video->vram * 1024); } @@ -4043,6 +4050,13 @@ qemuBuildCommandLine(virConnectPtr conn, if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { if (def->videos[0]->vram && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (def->videos[0]->vram > (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + goto error; + } + if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL_VGA)) virCommandAddArgFormat(cmd, "-global qxl-vga.vram_size=%u", def->videos[0]->vram * 1024); -- 1.7.4
On 03/09/2011 01:34 AM, Osier Yang wrote:
As perhaps other hypervisor drivers use different capacity units, do the checking in qemu driver instead of in conf/domain_conf.c. --- src/qemu/qemu_command.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 198a4e2..59fd2ac 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1933,6 +1933,13 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video, virBufferVSprintf(&buf, ",id=%s", video->info.alias);
if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram > (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR,
Wrong error. That's a user-supplied value, so it's not an internal error. Better would be VIR_ERR_INVALID_ARG.
if (def->videos[0]->vram && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (def->videos[0]->vram > (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR,
Again. ACK with those nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2011年03月15日 11:27, Eric Blake 写道:
On 03/09/2011 01:34 AM, Osier Yang wrote:
As perhaps other hypervisor drivers use different capacity units, do the checking in qemu driver instead of in conf/domain_conf.c. --- src/qemu/qemu_command.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 198a4e2..59fd2ac 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1933,6 +1933,13 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video, virBufferVSprintf(&buf, ",id=%s", video->info.alias);
if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram> (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR,
Wrong error. That's a user-supplied value, so it's not an internal error. Better would be VIR_ERR_INVALID_ARG.
if (def->videos[0]->vram&& qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (def->videos[0]->vram> (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR,
Again.
ACK with those nits fixed.
Thanks, pushed with those nits fixed. Regards Osier
On Mon, Mar 14, 2011 at 09:27:40PM -0600, Eric Blake wrote:
On 03/09/2011 01:34 AM, Osier Yang wrote:
As perhaps other hypervisor drivers use different capacity units, do the checking in qemu driver instead of in conf/domain_conf.c. --- src/qemu/qemu_command.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 198a4e2..59fd2ac 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1933,6 +1933,13 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video, virBufferVSprintf(&buf, ",id=%s", video->info.alias);
if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram > (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR,
Wrong error. That's a user-supplied value, so it's not an internal error. Better would be VIR_ERR_INVALID_ARG.
No, INVALID_ARG should rarely be used - it indicates that a parameter to a method was incorrect. Using it for this will give horrible error messages. Better to use CONFIG_UNSUPPORTED for any semantic XML error. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 03/15/2011 04:51 AM, Daniel P. Berrange wrote:
@@ -1933,6 +1933,13 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video, virBufferVSprintf(&buf, ",id=%s", video->info.alias);
if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram > (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR,
Wrong error. That's a user-supplied value, so it's not an internal error. Better would be VIR_ERR_INVALID_ARG.
No, INVALID_ARG should rarely be used - it indicates that a parameter to a method was incorrect. Using it for this will give horrible error messages. Better to use CONFIG_UNSUPPORTED for any semantic XML error.
Oh, sorry about that. As penance, I'm pushing the following under the trivial rule: From 30a50fc3b03cafc063df6ee0adb07487db12ba0b Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake@redhat.com> Date: Tue, 15 Mar 2011 08:49:04 -0600 Subject: [PATCH] qemu: use more appropriate error Fixes bug in commit acacced * src/qemu/qemu_command.c (qemuBuildCommandLine): s/INVALID_ARG/CONFIG_UNSUPPORTED/. Reported by Daniel P. Berrange. --- src/qemu/qemu_command.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c63de09..c9b9850 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1935,7 +1935,7 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video, if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { if (video->vram > (UINT_MAX / 1024)) { - qemuReportError(VIR_ERR_INVALID_ARG, + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("value for 'vram' must be less than '%u'"), UINT_MAX / 1024); goto error; @@ -4050,7 +4050,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (def->videos[0]->vram && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { if (def->videos[0]->vram > (UINT_MAX / 1024)) { - qemuReportError(VIR_ERR_INVALID_ARG, + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("value for 'vram' must be less than '%u'"), UINT_MAX / 1024); goto error; -- 1.7.4 -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Osier Yang