Re: [PATCH] ui: fix crash when there are no active_console

[Albert Esteve]

On Mon, Sep 11, 2023 at 4:42 PM Albert Esteve <aesteve@redhat.com> wrote:

On Mon, Sep 11, 2023 at 4:08 PM <marcandre.lureau@redhat.com> wrote:

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x0000555555888630 in dpy_ui_info_supported (con=0x0) at ../ui/console.c:812
812         return con->hw_ops->ui_info != NULL;
(gdb) bt
#0  0x0000555555888630 in dpy_ui_info_supported (con=0x0) at ../ui/console.c:812
#1  0x00005555558a44b1 in protocol_client_msg (vs=0x5555578c76c0, data="" <incomplete sequence \373>, len=24) at ../ui/vnc.c:2585
#2  0x00005555558a19ac in vnc_client_read (vs=0x5555578c76c0) at ../ui/vnc.c:1607
#3  0x00005555558a1ac2 in vnc_client_io (ioc=0x5555581eb0e0, condition=G_IO_IN, opaque=0x5555578c76c0) at ../ui/vnc.c:1635

Fixes:
https://issues.redhat.com/browse/RHEL-2600

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/ui/console.c b/ui/console.c
index 90ae4be602..0f31ecece6 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -808,6 +808,9 @@ bool dpy_ui_info_supported(QemuConsole *con)
     if (con == NULL) {
         con = active_console;
     }
+    if (con == NULL) {
+        return false;
+    }

     return con->hw_ops->ui_info != NULL;
 }
@@ -817,6 +820,9 @@ const QemuUIInfo *dpy_get_ui_info(const QemuConsole *con)
     if (con == NULL) {
         con = active_console;
     }
+    if (con == NULL) {
+        return NULL;
+    }

     return &con->ui_info;
 }
@@ -826,6 +832,9 @@ int dpy_set_ui_info(QemuConsole *con, QemuUIInfo *info, bool delay)
     if (con == NULL) {
         con = active_console;
     }
+    if (con == NULL) {
+        return -1;
+    }

     if (!dpy_ui_info_supported(con)) {
         return -1;
@@ -1401,6 +1410,10 @@ QEMUCursor *qemu_console_get_cursor(QemuConsole *con)
     if (con == NULL) {
         con = active_console;
     }
+    if (con == NULL) {
+        return NULL;
+    }
+
     return QEMU_IS_GRAPHIC_CONSOLE(con) ? QEMU_GRAPHIC_CONSOLE(con)->cursor : NULL;
 }

@@ -1414,6 +1427,10 @@ bool qemu_console_is_graphic(QemuConsole *con)
     if (con == NULL) {
         con = active_console;
     }
+    if (con == NULL) {
+        return false;
+    }
+

I had miss this one before:

```

return QEMU_IS_GRAPHIC_CONSOLE(con);

```

Regards,

Albert

 

     return con && QEMU_IS_GRAPHIC_CONSOLE(con);
 }

@@ -1422,6 +1439,10 @@ bool qemu_console_is_fixedsize(QemuConsole *con)
     if (con == NULL) {
         con = active_console;
     }
+    if (con == NULL) {
+        return false;
+    }
+

The "con" initialization condition is already checked in the line below (now unnecessarily).

If the if block is preferred for consistency with other functions, we could remove the con check from

the condition below:

```

return QEMU_IS_GRAPHIC_CONSOLE(con) || QEMU_IS_FIXED_TEXT_CONSOLE(con);

```

 

     return con && (QEMU_IS_GRAPHIC_CONSOLE(con) || QEMU_IS_FIXED_TEXT_CONSOLE(con));
 }

@@ -1493,6 +1514,10 @@ int qemu_console_get_index(QemuConsole *con)
     if (con == NULL) {
         con = active_console;
     }
+    if (con == NULL) {
+        return -1;
+    }
+

Same as before, here we could simply "return con->index;"

 

     return con ? con->index : -1; 

 }

--
2.41.0