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

[Albert Esteve <aesteve@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3870 bytes --]

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=0x5555581e93f0 <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
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 6089 bytes --]