Since its introduction in commit 0977b8aa071 (released in v1.2.14)
qemuAgentGetInterfaces calls qemuAgentCommand with needReply=false,
which allows qemuAgentCommand to return 0 even when it did not get
any reply from the agent.

Set needReply to true, since we dereference it right after.

This can be hit if libvirt is waiting for an event from the agent
(e.g. shutdown) and the agent cannot reply in time (e.g. due to
the guest being shut down), as reported in:
https://bugzilla.redhat.com/show_bug.cgi?id=1663051

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_agent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 0f4dae37e3..80b789b5e0 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2043,7 +2043,7 @@ qemuAgentGetInterfaces(qemuAgentPtr mon,
     if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL)))
         goto cleanup;
 
-    if (qemuAgentCommand(mon, cmd, &reply, false,
+    if (qemuAgentCommand(mon, cmd, &reply, true,
                          VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
         goto cleanup;
 
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 1/10/19 3:01 AM, Ján Tomko wrote:
> Since its introduction in commit 0977b8aa071 (released in v1.2.14)
> qemuAgentGetInterfaces calls qemuAgentCommand with needReply=false,
> which allows qemuAgentCommand to return 0 even when it did not get
> any reply from the agent.
> 
> Set needReply to true, since we dereference it right after.
> 
> This can be hit if libvirt is waiting for an event from the agent
> (e.g. shutdown) and the agent cannot reply in time (e.g. due to
> the guest being shut down), as reported in:
> https://bugzilla.redhat.com/show_bug.cgi?id=1663051

I think a malicious guest can abuse their guest agent to crash libvirtd,
which makes this an escalation boundary because libvirt is supposed to
treat guest agent connections as untrusted. I'm asking Red Hat security
if this needs a CVE.

> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  src/qemu/qemu_agent.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 0f4dae37e3..80b789b5e0 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -2043,7 +2043,7 @@ qemuAgentGetInterfaces(qemuAgentPtr mon,
>      if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL)))
>          goto cleanup;
>  
> -    if (qemuAgentCommand(mon, cmd, &reply, false,
> +    if (qemuAgentCommand(mon, cmd, &reply, true,
>                           VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
>          goto cleanup;
>  
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Jan 10, 2019 at 10:01:19 +0100, Ján Tomko wrote:
> Since its introduction in commit 0977b8aa071 (released in v1.2.14)
> qemuAgentGetInterfaces calls qemuAgentCommand with needReply=false,
> which allows qemuAgentCommand to return 0 even when it did not get
> any reply from the agent.
> 
> Set needReply to true, since we dereference it right after.
> 
> This can be hit if libvirt is waiting for an event from the agent
> (e.g. shutdown) and the agent cannot reply in time (e.g. due to
> the guest being shut down), as reported in:
> https://bugzilla.redhat.com/show_bug.cgi?id=1663051
> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list