From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 78A6CEE57DF for ; Mon, 11 Sep 2023 15:34:49 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qfiRU-0008No-HH; Mon, 11 Sep 2023 11:03:12 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qfiRN-0008M2-JH for qemu-devel@nongnu.org; Mon, 11 Sep 2023 11:03:07 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qfiRK-0003I4-OF for qemu-devel@nongnu.org; Mon, 11 Sep 2023 11:03:05 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694444580; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=JKN0CuLAoGQg5eUgX7FxXxMTvyRbvVZgzu0U4npqe/M=; b=DNB5qdoOD4bTAFckOWSP5Pupimt9MNM+WUinYMoj9LD0m4/gJOjBoV2o2p91+Od0qyDbkw 5kFTAoKMvXnUuzAgWfCRHRYax3FV0c2g0meVZ12IqA6cW88uUSonzsWCrfIpf3K6iKKzuH GHRGCWsSa69KmofAu8kpjsOp0X8WAVs= Received: from mail-pj1-f70.google.com (mail-pj1-f70.google.com [209.85.216.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-321-4rDXxqIzOvOAkKd1PPzbLg-1; Mon, 11 Sep 2023 11:02:58 -0400 X-MC-Unique: 4rDXxqIzOvOAkKd1PPzbLg-1 Received: by mail-pj1-f70.google.com with SMTP id 98e67ed59e1d1-271da61261aso6722557a91.1 for ; Mon, 11 Sep 2023 08:02:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694444577; x=1695049377; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=JKN0CuLAoGQg5eUgX7FxXxMTvyRbvVZgzu0U4npqe/M=; b=sY10kgI2pF5MI1vFkZ/pDpnUEZyJHqOLJ7cB6WkmU4wqMYXTWPjU70sjpnsddsBntB X/Br585UpGkEZ1mmchME4fTXjuOpV3jUtdh4J4fB4degpPyrXWhKcpjQDXBLP86ub8De GwbVYEMWAeAci0QaHcH/d/kDsPNC8y39lhRYq5M2jmon86XW6ZuJS5IYb8pj9RqMXusG bwUcDYY6Usj7dO9AVf4LnId/lm+DCRNIpjms8vaIATE8j1vIaX/g6kWdMuQtwKCgPqdJ l7hGr7yoBd7Z1wiI2Hj9dXJQIFXJk5t+Gx4xdvfbbpBWxDTuqTmcXNzPmLKfxOElC3zW hd0Q== X-Gm-Message-State: AOJu0YywyfLnSt8rZc+YHbvZ3pWBxorZIoCZLchIccDm99LU4IgpJhLG RpFoj6z+a7wZI5U2yW7PmYCVI4nB40ajHuZm8FNJNm4Rc5PaSKmD/clj6ZBCuspLPq91qcSimJF 8SslDXzZL0mxEdD9rAjkzZZbo9dOwam4= X-Received: by 2002:a17:90a:f3cf:b0:268:2af6:e48c with SMTP id ha15-20020a17090af3cf00b002682af6e48cmr13628690pjb.4.1694444577276; Mon, 11 Sep 2023 08:02:57 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGsjPRUqL61PiqJcnmuGjNGGZ0O6Ixy6P6DXixuD5gfuLBye8QMo6Hfd+g4dLhLr2CHkLEx0vGpuHWMeoPvvOY= X-Received: by 2002:a17:90a:f3cf:b0:268:2af6:e48c with SMTP id ha15-20020a17090af3cf00b002682af6e48cmr13628666pjb.4.1694444576963; Mon, 11 Sep 2023 08:02:56 -0700 (PDT) MIME-Version: 1.0 References: <20230911140638.1458156-1-marcandre.lureau@redhat.com> In-Reply-To: From: Albert Esteve Date: Mon, 11 Sep 2023 17:02:45 +0200 Message-ID: Subject: Re: [PATCH] ui: fix crash when there are no active_console To: marcandre.lureau@redhat.com Cc: qemu-devel@nongnu.org, Gerd Hoffmann Content-Type: multipart/alternative; boundary="0000000000001a177a0605169f92" Received-SPF: pass client-ip=170.10.129.124; envelope-from=aesteve@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org --0000000000001a177a0605169f92 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Sep 11, 2023 at 4:42=E2=80=AFPM Albert Esteve = wrote: > > > On Mon, Sep 11, 2023 at 4:08=E2=80=AFPM wro= te: > >> From: Marc-Andr=C3=A9 Lureau >> >> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. >> 0x0000555555888630 in dpy_ui_info_supported (con=3D0x0) at >> ../ui/console.c:812 >> 812 return con->hw_ops->ui_info !=3D NULL; >> (gdb) bt >> #0 0x0000555555888630 in dpy_ui_info_supported (con=3D0x0) at >> ../ui/console.c:812 >> #1 0x00005555558a44b1 in protocol_client_msg (vs=3D0x5555578c76c0, >> data=3D0x5555581e93f0 , len=3D24) at ../ui/vnc= .c:2585 >> #2 0x00005555558a19ac in vnc_client_read (vs=3D0x5555578c76c0) at >> ../ui/vnc.c:1607 >> #3 0x00005555558a1ac2 in vnc_client_io (ioc=3D0x5555581eb0e0, >> condition=3DG_IO_IN, opaque=3D0x5555578c76c0) at ../ui/vnc.c:1635 >> >> Fixes: >> https://issues.redhat.com/browse/RHEL-2600 >> >> Signed-off-by: Marc-Andr=C3=A9 Lureau >> --- >> 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 =3D=3D NULL) { >> con =3D active_console; >> } >> + if (con =3D=3D NULL) { >> + return false; >> + } >> >> return con->hw_ops->ui_info !=3D NULL; >> } >> @@ -817,6 +820,9 @@ const QemuUIInfo *dpy_get_ui_info(const QemuConsole >> *con) >> if (con =3D=3D NULL) { >> con =3D active_console; >> } >> + if (con =3D=3D NULL) { >> + return NULL; >> + } >> >> return &con->ui_info; >> } >> @@ -826,6 +832,9 @@ int dpy_set_ui_info(QemuConsole *con, QemuUIInfo >> *info, bool delay) >> if (con =3D=3D NULL) { >> con =3D active_console; >> } >> + if (con =3D=3D NULL) { >> + return -1; >> + } >> >> if (!dpy_ui_info_supported(con)) { >> return -1; >> @@ -1401,6 +1410,10 @@ QEMUCursor *qemu_console_get_cursor(QemuConsole >> *con) >> if (con =3D=3D NULL) { >> con =3D active_console; >> } >> + if (con =3D=3D 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 =3D=3D NULL) { >> con =3D active_console; >> } >> + if (con =3D=3D 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 =3D=3D NULL) { >> con =3D active_console; >> } >> + if (con =3D=3D 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 =3D=3D NULL) { >> con =3D active_console; >> } >> + if (con =3D=3D NULL) { >> + return -1; >> + } >> + >> > > Same as before, here we could simply "return con->index;" > > >> return con ? con->index : -1; > > } >> >> -- >> 2.41.0 >> >> >> --0000000000001a177a0605169f92 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


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


On Mon, Sep 11, 20= 23 at 4:08=E2=80=AFPM <marcandre.lureau@redhat.com> wrote:
From: Marc-Andr=C3=A9 Lureau <= ;marcandre= .lureau@redhat.com>

Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation = fault.
0x0000555555888630 in dpy_ui_info_supported (con=3D0x0) at ../ui/console.c:= 812
812=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return con->hw_ops->ui_info !=3D= NULL;
(gdb) bt
#0=C2=A0 0x0000555555888630 in dpy_ui_info_supported (con=3D0x0) at ../ui/c= onsole.c:812
#1=C2=A0 0x00005555558a44b1 in protocol_client_msg (vs=3D0x5555578c76c0, da= ta=3D0x5555581e93f0 <incomplete sequence \373>, len=3D24) at ../ui/vn= c.c:2585
#2=C2=A0 0x00005555558a19ac in vnc_client_read (vs=3D0x5555578c76c0) at ../= ui/vnc.c:1607
#3=C2=A0 0x00005555558a1ac2 in vnc_client_io (ioc=3D0x5555581eb0e0, conditi= on=3DG_IO_IN, opaque=3D0x5555578c76c0) at ../ui/vnc.c:1635

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

Signed-off-by: Marc-Andr=C3=A9 Lureau <marcandre.lureau@redhat.com>
---
=C2=A0ui/console.c | 25 +++++++++++++++++++++++++
=C2=A01 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)
=C2=A0 =C2=A0 =C2=A0if (con =3D=3D NULL) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0con =3D active_console;
=C2=A0 =C2=A0 =C2=A0}
+=C2=A0 =C2=A0 if (con =3D=3D NULL) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
+=C2=A0 =C2=A0 }

=C2=A0 =C2=A0 =C2=A0return con->hw_ops->ui_info !=3D NULL;
=C2=A0}
@@ -817,6 +820,9 @@ const QemuUIInfo *dpy_get_ui_info(const QemuConsole *co= n)
=C2=A0 =C2=A0 =C2=A0if (con =3D=3D NULL) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0con =3D active_console;
=C2=A0 =C2=A0 =C2=A0}
+=C2=A0 =C2=A0 if (con =3D=3D NULL) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return NULL;
+=C2=A0 =C2=A0 }

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

=C2=A0 =C2=A0 =C2=A0if (!dpy_ui_info_supported(con)) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -1;
@@ -1401,6 +1410,10 @@ QEMUCursor *qemu_console_get_cursor(QemuConsole *con= )
=C2=A0 =C2=A0 =C2=A0if (con =3D=3D NULL) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0con =3D active_console;
=C2=A0 =C2=A0 =C2=A0}
+=C2=A0 =C2=A0 if (con =3D=3D NULL) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return NULL;
+=C2=A0 =C2=A0 }
+
=C2=A0 =C2=A0 =C2=A0return QEMU_IS_GRAPHIC_CONSOLE(con) ? QEMU_GRAPHIC_CONS= OLE(con)->cursor : NULL;
=C2=A0}

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

I had miss t= his one before:
```
return QEMU_IS_GRAPHIC_CONSOLE(con)= ;
```

Regards,
Albert
=C2=A0
=C2=A0 =C2=A0 =C2=A0return con && QEMU_IS_GRAPHIC_CONSOLE(con);
=C2=A0}

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

The "con" initialization co= ndition is already checked in the line below (now unnecessarily).
If the if block is preferred for consistency with other functions, we coul= d remove the con check from
the condition below:
```
return QEMU_IS_GRAPHIC_CONSOLE(con) || QEMU_IS_FIXED_TEXT_CONSOLE(c= on);
```
=C2=A0
=C2=A0 =C2=A0 =C2=A0return con && (QEMU_IS_GRAPHIC_CONSOLE(con) || = QEMU_IS_FIXED_TEXT_CONSOLE(con));
=C2=A0}

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

Same as before, here we could simply = "return con->index;"
=C2=A0
=C2=A0 =C2=A0 =C2=A0return con ? con->index : -1;=C2=A0
=C2=A0}

--
2.41.0


--0000000000001a177a0605169f92--