On Mon, Nov 03, 2025 at 11:34:36AM +0000, Peter Maydell wrote:
> > > > Hi; Coverity suggests there are some issues with this code
> > > > (CID 1611807, 1611808, 1611809):
> > >
> > > Hi; it looks like 1611807 and 1611808 (the resource leaks)
> > > are still present in this code in current git; would somebody
> > > like to have a look at this?
> >
> > Please see
> > https://lore.kernel.org/qemu-devel/[email protected]/
> >
> > I believe them to be false positives.
>
> That email only seems to talk about the locking issue (1611806,
> 1611809) which we've marked as false-positives in Coverity.
> But 1611807 and 1611808 are "failed to free resources" issues:
> we allocate memory into reqfds and msg, but in the error
> exit path (e.g. if we "goto fatal" because qio_channel_read()
> failed) it doesn't look like we ever free that memory.
Sorry, I should have pasted: Here was my reply to Cédric below.
Looking at it with fresh eyes, though, I'm not so sure, at least in the case we
alloc instead of freelist, we need some form of recycle on the error path. The
whole function needs a big refactor for clarity...
> *** CID 1611808: Resource leaks (RESOURCE_LEAK)
> /builds/qemu-project/qemu/hw/vfio-user/proxy.c: 411 in
> vfio_user_recv_one()
> 405 if (isreply && msg != NULL) {
> 406 /* force an error to keep sending thread from hanging */
> 407 vfio_user_set_error(msg->hdr, EINVAL);
> 408 msg->complete = true;
> 409 qemu_cond_signal(&msg->cv);
> 410 }
> > > > CID 1611808: Resource leaks (RESOURCE_LEAK)
> > > > Variable "reqfds" going out of scope leaks the storage it points to.
> 411 return -1;
vfio_user_getmsg() I think takes ownership of reqfds so this looks OK.
It's hard to say without the full analysis.
> /builds/qemu-project/qemu/hw/vfio-user/proxy.c: 411 in
> vfio_user_recv_one()
> 405 if (isreply && msg != NULL) {
> 406 /* force an error to keep sending thread from hanging */
> 407 vfio_user_set_error(msg->hdr, EINVAL);
> 408 msg->complete = true;
> 409 qemu_cond_signal(&msg->cv);
> 410 }
> > > > CID 1611807: Resource leaks (RESOURCE_LEAK)
> > > > Variable "msg" going out of scope leaks the storage it points to.
> 411 return -1;
> 412 }
Same as above.
regards
john
