[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] QEMU xen coverity issues
From: |
Paul Durrant |
Subject: |
Re: [Qemu-devel] QEMU xen coverity issues |
Date: |
Mon, 18 Feb 2019 10:28:20 +0000 |
> -----Original Message-----
> From: Kevin Wolf [mailto:address@hidden
> Sent: 18 February 2019 10:09
> To: Paul Durrant <address@hidden>
> Cc: 'Peter Maydell' <address@hidden>; QEMU Developers <qemu-
> address@hidden>; Anthony Perard <address@hidden>
> Subject: Re: [Qemu-devel] QEMU xen coverity issues
>
> Am 15.02.2019 um 17:20 hat Paul Durrant geschrieben:
> > > -----Original Message-----
> > [snip]
> > > >
> > > > (5) CID 1398649: resource leak in xen_block_drive_create():
> > > >
> > > > In hw/block/xen-block.c xen_block_drive_create() Coverity
> > > > complains that the call "driver_layer = qdict_new()" allocates
> > > > memory that's leaked because we don't save the pointer anywhere
> > > > but don't deallocate it before the end of the function either.
> > > > Coverity is not great at understanding our refcounting objects,
> > > > but this does look like either we're missing a qobject_unref()
> > > > or something should be keeping hold of the dictionary. Probably
> > > > best to ask a block layer expert.
> > >
> > > AFAICT nothing will consume the dictionary so it does appear that
> we're
> > > missing an unref here.
> >
> > Testing proves me wrong... This one is a false positive.
>
> Hm, but where is it freed?
>
> xen_block_blockdev_add() only feeds it to an input visitor, which
> doesn't take ownership of the QDict (and in the first error path, it
> hasn't even done that yet).
Agreed that error path does not free things... that's definitely a leak... but
attempting to free the QDict's on return from xen_block_blockdev_add()
certainly causes a seg fault. My assumption was that, having been fed through
the input visitor and then through the output visitor in qmp_blockdev_add()
that the BlockDriverState eventually takes ownership... but maybe that's not
true?
Paul
>
> Kevin