diff mbox

[v3] gdbstub.c: fix GDB connection segfault caused by empty machines

Message ID 1484702885-11437-1-git-send-email-skiver.cloud.yzy@gmail.com
State New
Headers show

Commit Message

skiver.cloud.yzy@gmail.com Jan. 18, 2017, 1:28 a.m. UTC 
From: Ziyue Yang <yzylivezh@hotmail.com>

This patch is to fix the segmentation fault caused by attaching
GDB to a QEMU instance initialized with "-M none" option.

The bug can be reproduced by

> ./qemu-system-x86_64 -M none -nographic -S -s

and attach a GDB to it by

> gdb -ex 'target remote :1234

The segmentation fault was originally caused by trying to read
the information about CPU when communicating with GDB. However,
it's impossible for any control flow to exist on an empty machine,
nor can CPU's be hot plugged to an empty machine later by QOM
commands. So I think simply disabling GDB connections on empty
machines makes sense.

Also some updates from fprintf(stderr, ...) to error_report.

Signed-off-by: Ziyue Yang <skiver.cloud.yzy@gmail.com>
---
 gdbstub.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

--
2.7.4

Comments

Fam Zheng Jan. 18, 2017, 2:15 a.m. UTC | #1 
On Wed, 01/18 09:28, Ziyue Yang wrote:
> Also some updates from fprintf(stderr, ...) to error_report.

Looks like they belong to a separate patch.

Fam
skiver.cloud.yzy@gmail.com Jan. 18, 2017, 3:25 a.m. UTC | #2 
Hi Fam,

I've split the patch into 2 smaller ones and submitted them tagged with "v4".
Thanks!

Ziyue Yang

2017-01-18 10:15 GMT+08:00 Fam Zheng <famz@redhat.com>:
> On Wed, 01/18 09:28, Ziyue Yang wrote:
>> Also some updates from fprintf(stderr, ...) to error_report.
>
> Looks like they belong to a separate patch.
>
> Fam
diff mbox

Patch

diff --git a/gdbstub.c b/gdbstub.c
index de62d26..fe1d0f8 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -18,6 +18,7 @@ 
  */
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "qemu/cutils.h"
 #include "cpu.h"
 #ifdef CONFIG_USER_ONLY
@@ -636,8 +637,8 @@  void gdb_register_coprocessor(CPUState *cpu,
     *p = s;
     if (g_pos) {
         if (g_pos != s->base_reg) {
-            fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n"
-                    "Expected %d got %d\n", xml, g_pos, s->base_reg);
+            error_report("Error: Bad gdb register numbering for '%s', "
+                         "Expected %d got %d", xml, g_pos, s->base_reg);
         } else {
             cpu->gdb_num_g_regs = cpu->gdb_num_regs;
         }
@@ -889,7 +890,7 @@  static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
     case 'k':
         /* Kill the target */
-        fprintf(stderr, "\nQEMU: Terminated via GDBstub\n");
+        error_report("QEMU: Terminated via GDBstub");
         exit(0);
     case 'D':
         /* Detach packet */
@@ -1357,8 +1358,8 @@  void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
                 break;
             default:
             bad_format:
-                fprintf(stderr, "gdbstub: Bad syscall format string '%s'\n",
-                        fmt - 1);
+                error_report("gdbstub: Bad syscall format string '%s'",
+                             fmt - 1);
                 break;
             }
         } else {
@@ -1731,6 +1732,12 @@  int gdbserver_start(const char *device)
     CharDriverState *mon_chr;
     ChardevCommon common = { 0 };

+    if (!first_cpu) {
+        error_report("gdbstub: meaningless to attach gdb to a "
+                     "machine without any CPU.");
+        return -1;
+    }
+
     if (!device)
         return -1;
     if (strcmp(device, "none") != 0) {