From 62aa1d887ff9fc76adb488d31447d126a78f4b8f Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Fri, 20 Jul 2018 11:34:51 +0800 Subject: monitor: Fix unsafe sharing of @cur_mon among threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @cur_mon is null unless the main thread is running monitor code, either HMP code within monitor_read(), or QMP code within monitor_qmp_dispatch(). Use of @cur_mon outside the main thread is therefore unsafe. Most of its uses are in monitor command handlers. These run in the main thread. However, there are also uses hiding elsewhere, such as in error_vprintf(), and thus error_report(), making these functions unsafe outside the main thread. No such unsafe uses are known at this time. Regardless, this is an unnecessary trap. It's an ancient trap, though. More recently, commit cf869d53172 "qmp: support out-of-band (oob) execution" spiced things up: the monitor I/O thread assigns to @cur_mon when executing commands out-of-band. Having two threads save, set and restore @cur_mon without synchronization is definitely unsafe. We can end up with @cur_mon null while the main thread runs monitor code, or non-null while it runs non-monitor code. We could fix this by making the I/O thread not mess with @cur_mon, but that would leave the trap armed and ready. Instead, make @cur_mon thread-local. It's now reliably null unless the thread is running monitor code. Reviewed-by: Eric Blake Reviewed-by: Marc-André Lureau Reviewed-by: Stefan Hajnoczi [peterx: update subject and commit message written by Markus] Reviewed-by: Markus Armbruster Signed-off-by: Peter Xu Message-Id: <20180720033451.32710-1-peterx@redhat.com> --- include/monitor/monitor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/monitor') diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index d6ab70cae2..2ef5e04b37 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -6,7 +6,7 @@ #include "qapi/qapi-types-misc.h" #include "qemu/readline.h" -extern Monitor *cur_mon; +extern __thread Monitor *cur_mon; /* flags for monitor_init */ /* 0x01 unused */ -- cgit v1.2.3