[libvirt] [PATCH] qemu restore: don't let corrupt input provoke unwarranted OOM

[Jim Meyering]

As the log says, we must not use a just-read (and unverified)
header.xml_len as the size of buffer to allocate.

While looking at this, I noticed a small problem with virFileReadLimFD.
Passing it a limit < -1 would cause int->size_t trouble with saferead_lim.
Separate patch coming up.

>From a4906ba24b576f3219234c519c88d102df2173b2 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 3 Mar 2010 11:27:16 +0100
Subject: [PATCH] qemu restore: don't let corrupt input provoke unwarranted OOM

* src/qemu/qemu_driver.c (qemudDomainRestore): A corrupt save file
(in particular, a too-large header.xml_len value) would cause an
unwarranted out-of-memory error.  Do not trust the just-read
header.xml_len.  Instead, merely use that as a hint, and
read/allocate up to that number of bytes from the file.
Also verify that header.xml_len is positive; if it were negative,
passing it to virFileReadLimFD could cause trouble.
---
 src/qemu/qemu_driver.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7f7c459..5c9d003 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4725,44 +4725,45 @@ static int qemudDomainRestore(virConnectPtr conn,
     if (saferead(fd, &header, sizeof(header)) != sizeof(header)) {
         qemuReportError(VIR_ERR_OPERATION_FAILED,
                         "%s", _("failed to read qemu header"));
         goto cleanup;
     }

     if (memcmp(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)) != 0) {
         qemuReportError(VIR_ERR_OPERATION_FAILED,
                         "%s", _("image magic is incorrect"));
         goto cleanup;
     }

     if (header.version > QEMUD_SAVE_VERSION) {
         qemuReportError(VIR_ERR_OPERATION_FAILED,
                         _("image version is not supported (%d > %d)"),
                         header.version, QEMUD_SAVE_VERSION);
         goto cleanup;
     }

-    if (VIR_ALLOC_N(xml, header.xml_len) < 0) {
-        virReportOOMError();
+    if (header.xml_len <= 0) {
+        qemuReportError(VIR_ERR_OPERATION_FAILED,
+                        _("invalid XML length: %d"), header.xml_len);
         goto cleanup;
     }

-    if (saferead(fd, xml, header.xml_len) != header.xml_len) {
+    if (virFileReadLimFD(fd, header.xml_len, &xml) != header.xml_len) {
         qemuReportError(VIR_ERR_OPERATION_FAILED,
                         "%s", _("failed to read XML"));
         goto cleanup;
     }

     /* Create a domain from this XML */
     if (!(def = virDomainDefParseString(driver->caps, xml,
                                         VIR_DOMAIN_XML_INACTIVE))) {
         qemuReportError(VIR_ERR_OPERATION_FAILED,
                         "%s", _("failed to parse XML"));
         goto cleanup;
     }

     if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
         goto cleanup;

     if (!(vm = virDomainAssignDef(driver->caps,
                                   &driver->domains,
                                   def))) {
--
1.7.0.1.464.g0adc7