[Qemu-devel] [PATCH v2] qemu-char: fix terminal crash when using "-monitor stdio -nographic"

* [Qemu-devel] [PATCH v2] qemu-char: fix terminal crash when using "-monitor stdio -nographic"
@ 2014-09-09 11:19 john.liuli
  2014-09-15 12:50 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
  0 siblings, 1 reply; 7+ messages in thread
From: john.liuli @ 2014-09-09 11:19 UTC (permalink / raw)
  To: armbru; +Cc: qemu-trivial, Li Liu, peter.huangpeng, kraxel, qemu-devel

From: Li Liu <john.liuli@huawei.com>

Eeay to reproduce, just try "qemu -monitor stdio -nographic"
and type "quit", then the terminal will be crashed.

There are two pathes try to call tcgetattr of stdio in vl.c:

1) Monitor_parse(optarg, "readline");
   .....
   qemu_opts_foreach(qemu_find_opts("chardev"),
                     chardev_init_func, NULL, 1) != 0)

2) if (default_serial)
   add_device_config(DEV_SERIAL, "stdio");
   ....
   if (foreach_device_config(DEV_SERIAL, serial_parse) < 0)

Both of them will trigger qemu_chr_open_stdio which will disable
ECHO attributes. First one has updated the attributes of stdio
by calling qemu_chr_fe_set_echo(chr, false). And the tty
attributes has been saved in oldtty. Then the second path will
redo such actions, and the oldtty is overlapped. So till "quit",
term_exit can't recove the correct attributes.

Signed-off-by: Li Liu <john.liuli@huawei.com>
---
changes v1 -> v2:
1) as Markus Armbruster and Gerd Hoffmann suggested :
   Multiple character devices can't use the same terminal.
   So catch and reject the attempt if stdio is opened already.

---
 qemu-char.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/qemu-char.c b/qemu-char.c
index d4f327a..f8f0c48 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1017,6 +1017,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
 /* init terminal so that we can grab keys */
 static struct termios oldtty;
 static int old_fd0_flags;
+static bool stdio_is_ready;
 static bool stdio_allow_signal;
 
 static void term_exit(void)
@@ -1060,8 +1061,15 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
         error_report("cannot use stdio with -daemonize");
         return NULL;
     }
+
+    if (stdio_is_ready) {
+        error_report("cannot use stdio by multiple character devices");
+        exit(1);
+    }
+
+    stdio_is_ready = true;
     old_fd0_flags = fcntl(0, F_GETFL);
-    tcgetattr (0, &oldtty);
+    tcgetattr(0, &oldtty);
     qemu_set_nonblock(0);
     atexit(term_exit);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 7+ messages in thread