On Thu, 25 Jun 2020 10:30:24 +0100 Peter Maydell <peter.mayd...@linaro.org> wrote:
> On Thu, 18 Jun 2020 at 14:23, Jason Wang <jasow...@redhat.com> wrote:
> >
> > From: Lukas Straub <lukasstra...@web.de>
> >
> > In colo_compare_complete, insert CompareState into net_compares
> > only after everything has been initialized.
> > In colo_compare_finalize, remove CompareState from net_compares
> > before anything is deinitialized.
>
> Hi; this code-motion seems to have prompted Coverity to
> discover a possible deref-of-NULL-pointer (cID 1429969):
>
>
> > @@ -1409,6 +1397,19 @@ static void colo_compare_finalize(Object *obj)
> > }
> > qemu_mutex_unlock(&colo_compare_mutex);
> >
> > + qemu_chr_fe_deinit(&s->chr_pri_in, false);
> > + qemu_chr_fe_deinit(&s->chr_sec_in, false);
> > + qemu_chr_fe_deinit(&s->chr_out, false);
> > + if (s->notify_dev) {
> > + qemu_chr_fe_deinit(&s->chr_notify_dev, false);
> > + }
> > +
> > + if (s->iothread) {
>
> Here we check s->iothread, which implies that it could be NULL...
>
> > + colo_compare_timer_del(s);
> > + }
> > +
> > + qemu_bh_delete(s->event_bh);
> > +
> > AioContext *ctx = iothread_get_aio_context(s->iothread);
>
> ...but here we pass it to iothread_get_aio_context(), which
> unconditionally dereferences it, so will crash if it is NULL.
>
> Either we need to avoid calling this if s->iothread is NULL,
> or if it can't ever be NULL then the earlier NULL check was
> pointless and can be removed.
I'll look into it.
Regards,
Lukas Straub
>
> > aio_context_acquire(ctx);
> > AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
> > --
> > 2.5.0
>
> thanks
> -- PMM
pgpxyopQap2tH.pgp
Description: OpenPGP digital signature
