qemu-char: fix deadlock with "-monitor pty"
qemu_chr_be_generic_open cannot be called with the write lock taken,
because it calls client code that may call qemu_chr_fe_write. This
actually happens for the monitor:
0x00007ffff27dbf79 in __GI_raise (sig=sig@entry=6)
0x00007ffff27df388 in __GI_abort ()
0x00005555555ef489 in error_exit (err=<optimized out>, msg=msg@entry=0x5555559796d0 <__func__.5959> "qemu_mutex_lock")
0x00005555558f9080 in qemu_mutex_lock (mutex=mutex@entry=0x555556248a30)
0x0000555555713936 in qemu_chr_fe_write (s=0x555556248a30, buf=buf@entry=0x5555563d8870 "QEMU 2.0.90 monitor - type 'help' for more information\r\n", len=56)
0x00005555556217fd in monitor_flush_locked (mon=mon@entry=0x555556251fd0)
0x0000555555621a12 in monitor_flush_locked (mon=0x555556251fd0)
monitor_puts (mon=mon@entry=0x555556251fd0, str=0x55555634bfa7 "", str@entry=0x55555634bf70 "QEMU 2.0.90 monitor - type 'help' for more information\n")
0x0000555555624359 in monitor_vprintf (mon=0x555556251fd0, fmt=<optimized out>, ap=<optimized out>)
0x0000555555624414 in monitor_printf (mon=<optimized out>, fmt=fmt@entry=0x5555559105a0 "QEMU %s monitor - type 'help' for more information\n")
0x0000555555629806 in monitor_event (opaque=0x555556251fd0, event=<optimized out>)
0x000055555571343c in qemu_chr_be_generic_open (s=0x555556248a30)
To avoid this, defer the call to an idle callback, which will be
called as soon as the main loop is re-entered. In order to simplify
the cleanup and do it in one place only, change pty_chr_close to
call pty_chr_state.
To reproduce, run with "-monitor pty", then try to read from the
slave /dev/pts/FOO that it creates.
Fixes: 9005b2a758
Reported-by: Li Liang <liangx.z.li@intel.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
parent
2039511b8f
commit
7b3621f47a
23
qemu-char.c
23
qemu-char.c
@ -1089,6 +1089,7 @@ typedef struct {
|
|||||||
/* Protected by the CharDriverState chr_write_lock. */
|
/* Protected by the CharDriverState chr_write_lock. */
|
||||||
int connected;
|
int connected;
|
||||||
guint timer_tag;
|
guint timer_tag;
|
||||||
|
guint open_tag;
|
||||||
} PtyCharDriver;
|
} PtyCharDriver;
|
||||||
|
|
||||||
static void pty_chr_update_read_handler_locked(CharDriverState *chr);
|
static void pty_chr_update_read_handler_locked(CharDriverState *chr);
|
||||||
@ -1101,6 +1102,7 @@ static gboolean pty_chr_timer(gpointer opaque)
|
|||||||
|
|
||||||
qemu_mutex_lock(&chr->chr_write_lock);
|
qemu_mutex_lock(&chr->chr_write_lock);
|
||||||
s->timer_tag = 0;
|
s->timer_tag = 0;
|
||||||
|
s->open_tag = 0;
|
||||||
if (!s->connected) {
|
if (!s->connected) {
|
||||||
/* Next poll ... */
|
/* Next poll ... */
|
||||||
pty_chr_update_read_handler_locked(chr);
|
pty_chr_update_read_handler_locked(chr);
|
||||||
@ -1203,12 +1205,26 @@ static gboolean pty_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
|
|||||||
return TRUE;
|
return TRUE;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static gboolean qemu_chr_be_generic_open_func(gpointer opaque)
|
||||||
|
{
|
||||||
|
CharDriverState *chr = opaque;
|
||||||
|
PtyCharDriver *s = chr->opaque;
|
||||||
|
|
||||||
|
s->open_tag = 0;
|
||||||
|
qemu_chr_be_generic_open(chr);
|
||||||
|
return FALSE;
|
||||||
|
}
|
||||||
|
|
||||||
/* Called with chr_write_lock held. */
|
/* Called with chr_write_lock held. */
|
||||||
static void pty_chr_state(CharDriverState *chr, int connected)
|
static void pty_chr_state(CharDriverState *chr, int connected)
|
||||||
{
|
{
|
||||||
PtyCharDriver *s = chr->opaque;
|
PtyCharDriver *s = chr->opaque;
|
||||||
|
|
||||||
if (!connected) {
|
if (!connected) {
|
||||||
|
if (s->open_tag) {
|
||||||
|
g_source_remove(s->open_tag);
|
||||||
|
s->open_tag = 0;
|
||||||
|
}
|
||||||
remove_fd_in_watch(chr);
|
remove_fd_in_watch(chr);
|
||||||
s->connected = 0;
|
s->connected = 0;
|
||||||
/* (re-)connect poll interval for idle guests: once per second.
|
/* (re-)connect poll interval for idle guests: once per second.
|
||||||
@ -1221,8 +1237,9 @@ static void pty_chr_state(CharDriverState *chr, int connected)
|
|||||||
s->timer_tag = 0;
|
s->timer_tag = 0;
|
||||||
}
|
}
|
||||||
if (!s->connected) {
|
if (!s->connected) {
|
||||||
|
g_assert(s->open_tag == 0);
|
||||||
s->connected = 1;
|
s->connected = 1;
|
||||||
qemu_chr_be_generic_open(chr);
|
s->open_tag = g_idle_add(qemu_chr_be_generic_open_func, chr);
|
||||||
}
|
}
|
||||||
if (!chr->fd_in_tag) {
|
if (!chr->fd_in_tag) {
|
||||||
chr->fd_in_tag = io_add_watch_poll(s->fd, pty_chr_read_poll,
|
chr->fd_in_tag = io_add_watch_poll(s->fd, pty_chr_read_poll,
|
||||||
@ -1236,7 +1253,8 @@ static void pty_chr_close(struct CharDriverState *chr)
|
|||||||
PtyCharDriver *s = chr->opaque;
|
PtyCharDriver *s = chr->opaque;
|
||||||
int fd;
|
int fd;
|
||||||
|
|
||||||
remove_fd_in_watch(chr);
|
qemu_mutex_lock(&chr->chr_write_lock);
|
||||||
|
pty_chr_state(chr, 0);
|
||||||
fd = g_io_channel_unix_get_fd(s->fd);
|
fd = g_io_channel_unix_get_fd(s->fd);
|
||||||
g_io_channel_unref(s->fd);
|
g_io_channel_unref(s->fd);
|
||||||
close(fd);
|
close(fd);
|
||||||
@ -1244,6 +1262,7 @@ static void pty_chr_close(struct CharDriverState *chr)
|
|||||||
g_source_remove(s->timer_tag);
|
g_source_remove(s->timer_tag);
|
||||||
s->timer_tag = 0;
|
s->timer_tag = 0;
|
||||||
}
|
}
|
||||||
|
qemu_mutex_unlock(&chr->chr_write_lock);
|
||||||
g_free(s);
|
g_free(s);
|
||||||
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
|
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user