[PATCH] qmp: Fix race causing events to be sent during negotiation

Ross Lagerwall posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260107135824.1681685-1-ross.lagerwall@citrix.com
Maintainers: Markus Armbruster <armbru@redhat.com>
monitor/qmp.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
[PATCH] qmp: Fix race causing events to be sent during negotiation
Posted by Ross Lagerwall 1 month ago
As per the QMP spec, asynchronous messages should not be sent during
negotiation.

The event sending code checks if the monitor is in the negotiation phase
by checking for mon->commands != qmp_cap_negotiation_commands. However,
events may be incorrectly sent from the point the connection is opened
to when monitor_qmp_event() sets the negotiation phase.

Ensure it is always in the negotiation phase when a connection is opened
by initializing it during monitor init and close.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 monitor/qmp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/monitor/qmp.c b/monitor/qmp.c
index e1419a9efa39..187a5d7477c9 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -462,15 +462,15 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
 
     switch (event) {
     case CHR_EVENT_OPENED:
-        WITH_QEMU_LOCK_GUARD(&mon->common.mon_lock) {
-            mon->commands = &qmp_cap_negotiation_commands;
-            monitor_qmp_caps_reset(mon);
-        }
         data = qmp_greeting(mon);
         qmp_send_response(mon, data);
         qobject_unref(data);
         break;
     case CHR_EVENT_CLOSED:
+        WITH_QEMU_LOCK_GUARD(&mon->common.mon_lock) {
+            mon->commands = &qmp_cap_negotiation_commands;
+            monitor_qmp_caps_reset(mon);
+        }
         /*
          * Note: this is only useful when the output of the chardev
          * backend is still open.  For example, when the backend is
@@ -527,6 +527,7 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
     monitor_data_init(&mon->common, true, false,
                       qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
 
+    mon->commands = &qmp_cap_negotiation_commands;
     mon->pretty = pretty;
 
     qemu_mutex_init(&mon->qmp_queue_lock);
-- 
2.52.0
Re: [PATCH] qmp: Fix race causing events to be sent during negotiation
Posted by Daniel P. Berrangé 1 month ago
On Wed, Jan 07, 2026 at 01:58:24PM +0000, Ross Lagerwall wrote:
> As per the QMP spec, asynchronous messages should not be sent during
> negotiation.
> 
> The event sending code checks if the monitor is in the negotiation phase
> by checking for mon->commands != qmp_cap_negotiation_commands. However,
> events may be incorrectly sent from the point the connection is opened
> to when monitor_qmp_event() sets the negotiation phase.
> 
> Ensure it is always in the negotiation phase when a connection is opened
> by initializing it during monitor init and close.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  monitor/qmp.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|