On Wed, Oct 13, 2021 at 11:51:24AM +0200, David Hildenbrand wrote:
> On 13.10.21 11:48, Stefan Hajnoczi wrote:
> > On Tue, Oct 12, 2021 at 08:38:32PM +0200, David Hildenbrand wrote:
> > > We end up not closing the file descriptor, resulting in leaking one
> > > file descriptor for each VHOST_USER_REM_MEM_REG message.
> > >
> > > Fixes: 875b9fd97b34 ("Support individual region unmap in libvhost-user")
> > > Cc: Michael S. Tsirkin <m...@redhat.com>
> > > Cc: Raphael Norwitz <raphael.norw...@nutanix.com>
> > > Cc: "Marc-André Lureau" <marcandre.lur...@redhat.com>
> > > Cc: Stefan Hajnoczi <stefa...@redhat.com>
> > > Cc: Paolo Bonzini <pbonz...@redhat.com>
> > > Cc: Coiby Xu <coiby...@gmail.com>
> > > Signed-off-by: David Hildenbrand <da...@redhat.com>
> > > ---
> > > subprojects/libvhost-user/libvhost-user.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/subprojects/libvhost-user/libvhost-user.c
> > > b/subprojects/libvhost-user/libvhost-user.c
> > > index bf09693255..bb5c3b3280 100644
> > > --- a/subprojects/libvhost-user/libvhost-user.c
> > > +++ b/subprojects/libvhost-user/libvhost-user.c
> > > @@ -839,6 +839,8 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
> > > vu_panic(dev, "Specified region not found\n");
> > > }
> > > + close(vmsg->fds[0]);
> >
> > Does anything check that exactly 1 fd was received? For example,
> > vu_set_log_fd_exec() does:
> >
> > if (vmsg->fd_num != 1) {
> > vu_panic(dev, "Invalid log_fd message");
> > return false;
> > }
> >
> > I think that's necessary both to make vhost-user master development
> > easier and because fds[] is not initialized to -1.
Ack - will add that.
>
> Similarly, vu_add_mem_reg() assumes exactly one was sent AFAIKS.
Ack
>
> If we panic, do we still have to call vmsg_close_fds() ?
>
I think so. What else will close the FDs?
AFAICT a vu_panic does not imply that the overall process has to die if that's
what you mean. What if one process is exposing multiple devices and only one of
them panics?
> --
> Thanks,
>
> David / dhildenb
>
