Re: [PATCH] ui/gtk: fix NULL pointer dereference

[Peter Maydell]

On Mon, 8 Mar 2021 at 13:32, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Montag, 8. März 2021 12:31:33 CET Akihiko Odaki wrote:
> > 2021年3月8日(月) 19:39 Christian Schoenebeck <qemu_oss@crudebyte.com>:
> > > This was just about silencing the mentioned automated Coverity defects
> > > report. If you have a better solution, then just ignore this patch.
> > >
> > > Best regards,
> > > Christian Schoenebeck
> >
> > I do not have an access to Coverity defects report. I'd appreciate the
> > details if you provide one. I suspect I made a mistake somewhere else
> > ui/gtk.c in c821a58ee7 ("ui/console: Pass placeholder surface to
> > display").
>
> Unfortunately Coverity's defects reports are not very verbose.

The online defect viewer is a bit better for showing why it thought
something was an issue. In this case we have at the top of the function:

    trace_gd_switch(vc->label,
                    surface ? surface_width(surface)  : 0,
                    surface ? surface_height(surface) : 0);

which tests whether surface is NULL, implying that sometimes it is.

Then later we have:
    if (vc->gfx.ds && surface &&

also checking surface for NULL-ness.

Finally we have:
    if (surface->format == PIXMAN_x8r8g8b8) {

which dereferences surface without checking if it's NULL.

So there is definitely a bug here:
(1) either surface can never be NULL, and all the places where
the function is testing for NULL-ness are wrong and need to be removed
(2) or surface can be NULL, and we should check here too

Coverity can't tell us which of the two possibilities is right, of course.

thanks
-- PMM