[PATCH] console: avoid passing con=NULL to graphic_hw_update_done() In graphic_hw_update(), first select an existing console, a specific-console or active_console(if not specified), then updating the console.

lichun posted 1 patch 3 years, 4 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1604682219-114389-1-git-send-email-lichun@ruijie.com.cn
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
ui/console.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] console: avoid passing con=NULL to graphic_hw_update_done() In graphic_hw_update(), first select an existing console, a specific-console or active_console(if not specified), then updating the console.
Posted by lichun 3 years, 4 months ago
Signed-off-by: lichun <lichun@ruijie.com.cn>
---
 ui/console.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index e8e5970..e07d2c3 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -270,10 +270,11 @@ void graphic_hw_update_done(QemuConsole *con)
 void graphic_hw_update(QemuConsole *con)
 {
     bool async = false;
+    con = con ? con : active_console;
     if (!con) {
-        con = active_console;
+        return;
     }
-    if (con && con->hw_ops->gfx_update) {
+    if (con->hw_ops->gfx_update) {
         con->hw_ops->gfx_update(con->hw);
         async = con->hw_ops->gfx_update_async;
     }
-- 
1.8.3.1


Re: [PATCH] console: avoid passing con=NULL to graphic_hw_update_done() In graphic_hw_update(), first select an existing console, a specific-console or active_console(if not specified), then updating the console.
Posted by Gerd Hoffmann 3 years, 4 months ago
  Hi,

If you have an long commit message put it into the body not the subject
please.

On Sat, Nov 07, 2020 at 01:03:39AM +0800, lichun wrote:
> Signed-off-by: lichun <lichun@ruijie.com.cn>
> ---
>  ui/console.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/ui/console.c b/ui/console.c
> index e8e5970..e07d2c3 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -270,10 +270,11 @@ void graphic_hw_update_done(QemuConsole *con)
>  void graphic_hw_update(QemuConsole *con)
>  {
>      bool async = false;
> +    con = con ? con : active_console;

con should not be NULL at this point.

Can you trigger a NULL pointer dereference here somehow?

thanks,
  Gerd


Re: [PATCH] console: avoid passing con=NULL to graphic_hw_update_done() In graphic_hw_update(), first select an existing console, a specific-console or active_console(if not specified), then updating the console.
Posted by Peter Maydell 3 years, 4 months ago
On Fri, 6 Nov 2020 at 09:28, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> If you have an long commit message put it into the body not the subject
> please.
>
> On Sat, Nov 07, 2020 at 01:03:39AM +0800, lichun wrote:
> > Signed-off-by: lichun <lichun@ruijie.com.cn>
> > ---
> >  ui/console.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/ui/console.c b/ui/console.c
> > index e8e5970..e07d2c3 100644
> > --- a/ui/console.c
> > +++ b/ui/console.c
> > @@ -270,10 +270,11 @@ void graphic_hw_update_done(QemuConsole *con)
> >  void graphic_hw_update(QemuConsole *con)
> >  {
> >      bool async = false;
> > +    con = con ? con : active_console;
>
> con should not be NULL at this point.

There is definitely a bug in the code currently in master, though.
Coverity points out (CID 1436158) that it checks for con being
NULL in the "if (con && con->hw_ops->gfx_update) {" line, but
then proceeds to call "graphic_hw_update_done(con);" which
assumes con is non-NULL. If con can't be NULL then the check
in the if() is unnecessary.

thnask
-- PMM