Re: [Qemu-block] [Qemu-devel] [PULL 5/8] commit: Fix use after free in completion

[Kevin Wolf]

Am 13.06.2017 um 18:12 hat Peter Maydell geschrieben:
> On 7 June 2017 at 18:50, Kevin Wolf <address@hidden> wrote:
> > diff --git a/block/commit.c b/block/commit.c
> > index a3028b2..af6fa68 100644
> > --- a/block/commit.c
> > +++ b/block/commit.c
> > @@ -89,6 +89,10 @@ static void commit_complete(BlockJob *job, void *opaque)
> >      int ret = data->ret;
> >      bool remove_commit_top_bs = false;
> >
> > +    /* Make sure overlay_bs and top stay around until 
> > bdrv_set_backing_hd() */
> > +    bdrv_ref(top);
> > +    bdrv_ref(overlay_bs);
> > +
> >      /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before
> >       * the normal backing chain can be restored. */
> >      blk_unref(s->base);
> 
> Hi -- coverity complains about this change, because bdrv_ref()
> assumes that its argument is not NULL, but later on in commit_complete()
> we have a check
>     "if (overlay_bs && ...)"
> which assumes its argument might be NULL. (CID 1376205)
> 
> Which is correct?

I saw the Coverity report and am looking into it. It's not completely
clear to me yet which is correct, but I suspect it can be NULL.

Kevin