[libvirt] [PATCH 1/2] qemu: caps: Fix segfault on daemon startup
On my f16 box at least, this tries to free p unconditionally which isn't always okay, 'p' is freeable only under a limited scope. --- src/qemu/qemu_capabilities.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6dee9d8..5487ecd 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1336,10 +1336,9 @@ fail: qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse %s version number in '%s'"), qemu, p ? p : help); - -cleanup: VIR_FREE(p); +cleanup: return -1; } -- 1.7.7.5
On 01/27/2012 11:34 AM, Cole Robinson wrote:
On my f16 box at least, this tries to free p unconditionally which isn't always okay, 'p' is freeable only under a limited scope.
You must not have yajl-devel installed. No wonder I didn't see that I had introduced a regression.
--- src/qemu/qemu_capabilities.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6dee9d8..5487ecd 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1336,10 +1336,9 @@ fail: qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse %s version number in '%s'"), qemu, p ? p : help); - -cleanup: VIR_FREE(p);
+cleanup:
Yuck. Why are we even calling strndup in the first place, just to produce a nicer error message? I'm pushing this instead. From ab6f1c9814f88e0547a5567176282f501fb138e2 Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake@redhat.com> Date: Fri, 27 Jan 2012 13:53:11 -0700 Subject: [PATCH] qemu: avoid double free of qemu help output If yajl was not compiled in, we end up freeing an incoming parameter, which leads to a bogus free later on. Regression introduced in commit 6e769eb. * src/qemu/qemu_capabilities.c (qemuCapsParseHelpStr): Avoid alloc on failure path, which in turn fixes bogus free. Reported by Cole Robinson. --- src/qemu/qemu_capabilities.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6dee9d8..426637c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1330,16 +1330,14 @@ int qemuCapsParseHelpStr(const char *qemu, fail: p = strchr(help, '\n'); - if (p) - p = strndup(help, p - help); + if (!p) + p = strchr(help, '\0'); qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse %s version number in '%s'"), - qemu, p ? p : help); + _("cannot parse %s version number in '%.*s'"), + qemu, (int) (p - help), help); cleanup: - VIR_FREE(p); - return -1; } -- 1.7.7.6 -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/27/2012 03:57 PM, Eric Blake wrote:
On 01/27/2012 11:34 AM, Cole Robinson wrote:
On my f16 box at least, this tries to free p unconditionally which isn't always okay, 'p' is freeable only under a limited scope.
You must not have yajl-devel installed. No wonder I didn't see that I had introduced a regression.
--- src/qemu/qemu_capabilities.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6dee9d8..5487ecd 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1336,10 +1336,9 @@ fail: qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse %s version number in '%s'"), qemu, p ? p : help); - -cleanup: VIR_FREE(p);
+cleanup:
Yuck. Why are we even calling strndup in the first place, just to produce a nicer error message? I'm pushing this instead.
From ab6f1c9814f88e0547a5567176282f501fb138e2 Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake@redhat.com> Date: Fri, 27 Jan 2012 13:53:11 -0700 Subject: [PATCH] qemu: avoid double free of qemu help output
If yajl was not compiled in, we end up freeing an incoming parameter, which leads to a bogus free later on. Regression introduced in commit 6e769eb.
* src/qemu/qemu_capabilities.c (qemuCapsParseHelpStr): Avoid alloc on failure path, which in turn fixes bogus free. Reported by Cole Robinson. --- src/qemu/qemu_capabilities.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6dee9d8..426637c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1330,16 +1330,14 @@ int qemuCapsParseHelpStr(const char *qemu,
fail: p = strchr(help, '\n'); - if (p) - p = strndup(help, p - help); + if (!p) + p = strchr(help, '\0');
qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse %s version number in '%s'"), - qemu, p ? p : help); + _("cannot parse %s version number in '%.*s'"), + qemu, (int) (p - help), help);
cleanup: - VIR_FREE(p); - return -1; }
Cool, works for me. Thanks, Cole
participants (2)
-
Cole Robinson -
Eric Blake