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

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: Jim Meyering <meyering(a)redhat.com&gt; 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