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(a)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