[PATCH 2/2] qemuDomainGetGuestInfo: Don't try to free a negative number of entries

[Ján Tomko]

On a Thursday in 2020, Peter Krempa wrote:
>'nfs' variable was set to -1 or -2 on agent failure. Cleanup then tried
>to free 'nfs' elements of the array which resulted into a crash.
>
>Make 'nfs' size_t and assign it only on successful agent call.
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1812965
>
>Broken by commit 599ae372d8cf092
>
>Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>---
> src/qemu/qemu_agent.c  |  2 +-
> src/qemu/qemu_driver.c | 12 ++++++++----
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
>diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
>index 9f3fb9732f..dff327e8d5 100644
>--- a/src/qemu/qemu_agent.c
>+++ b/src/qemu/qemu_agent.c
>@@ -1914,7 +1914,7 @@ qemuAgentGetFSInfoFillDisks(virJSONValuePtr jsondisks,
>     return 0;
> }
>
>-/* Returns: 0 on success
>+/* Returns: number of entries in '@info' on success
>  *          -2 when agent command is not supported by the agent
>  *          -1 otherwise
>  */
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index 02ea582767..e285e9373c 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -22814,7 +22814,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom,
>     g_autofree char *hostname = NULL;
>     unsigned int supportedTypes = types;
>     int rc;
>-    int nfs = 0;
>+    size_t nfs = 0;
>     qemuAgentFSInfoPtr *agentfsinfo = NULL;
>     size_t i;
>
>@@ -22867,9 +22867,13 @@ qemuDomainGetGuestInfo(virDomainPtr dom,

Some separate issues:

The hostname call above also shares the same code path on unsupported
command and success, assigning NULL to the TypedParameter

Also, I'm confused about the 'types' semantics - info types unsupported
by libvirt (none so far, unless the caller passed in nonsensical values)
are quietly filtered out. But if a type was requested and the agent does
not support it, we error out without actually setting an error.

>         }
>     }
>     if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
>-        rc = nfs = qemuAgentGetFSInfo(agent, &agentfsinfo);
>-        if (rc < 0 && !(rc == -2 && types == 0))
>-            goto exitagent;
>+        rc = qemuAgentGetFSInfo(agent, &agentfsinfo);
>+        if (rc < 0) {
>+            if (!(rc == -2 && types == 0))
>+                goto exitagent;


Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200313/5c592f08/attachment-0001.sig>