mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

monitor: Fix unsafe sharing of @cur_mon among threads

Browse Source 
@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 cf869d5317 "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 <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[peterx: update subject and commit message written by Markus]
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180720033451.32710-1-peterx@redhat.com>
This commit is contained in:
Peter Xu 2018-07-20 11:34:51 +08:00 committed by Markus Armbruster
parent 25b1ef31db
commit 62aa1d887f
4 changed files with 4 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/monitor/monitor.h
View File

@ -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 */

2
monitor.c
View File

@ -290,7 +290,7 @@ static mon_cmd_t info_cmds[];
QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
Monitor *cur_mon;
__thread Monitor *cur_mon;
static void monitor_command_cb(void *opaque, const char *cmdline,
void *readline_opaque);

2
stubs/monitor.c
View File

@ -3,7 +3,7 @@
#include "qemu-common.h"
#include "monitor/monitor.h"
Monitor *cur_mon = NULL;
__thread Monitor *cur_mon;
int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
{

2
tests/test-util-sockets.c
View File

@ -69,7 +69,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
* stubs/monitor.c is defined, to make sure monitor.o is discarded
* otherwise we get duplicate syms at link time.
*/
Monitor *cur_mon;
__thread Monitor *cur_mon;
void monitor_init(Chardev *chr, int flags) {}