[dpdk-stable] [dpdk-dev] [PATCH v3 3/3] vhost: fix fd leak in kick setup

[Xia, Chenbo]

Hi Xueming & Maxime,

> -----Original Message-----
> From: Xueming(Steven) Li <xuemingl at nvidia.com>
> Sent: Wednesday, November 11, 2020 2:02 PM
> To: Maxime Coquelin <maxime.coquelin at redhat.com>; dev at dpdk.org; Ding, Xuan
> <xuan.ding at intel.com>; stephen at networkplumber.org; NBU-Contact-Thomas
> Monjalon <thomas at monjalon.net>; stable at dpdk.org; Xia, Chenbo
> <chenbo.xia at intel.com>
> Subject: RE: [dpdk-dev] [PATCH v3 3/3] vhost: fix fd leak in kick setup
> 
> Hi Maxime,
> 
> Near end of this function, if vhost_check_queue_inflights_packed() and
> vhost_check_queue_inflights_split() return with error, is the fd expected
> to be
> closed by closing vq?

I thought about this before. In theory, it will not cause fd leak because the fd
is saved in vq. It will be closed upon next kick msg or vhost device destroy. But
thinking it again, maybe it's better to close it now since anyway it's useless now😊

What do you think?

Thanks,
Chenbo

> 
> >-----Original Message-----
> >From: dev <dev-bounces at dpdk.org> On Behalf Of Maxime Coquelin
> >Sent: Monday, November 9, 2020 8:17 PM
> >To: dev at dpdk.org; xuan.ding at intel.com; stephen at networkplumber.org;
> >NBU-Contact-Thomas Monjalon <thomas at monjalon.net>; stable at dpdk.org;
> >chenbo.xia at intel.com
> >Cc: Maxime Coquelin <maxime.coquelin at redhat.com>
> >Subject: [dpdk-dev] [PATCH v3 3/3] vhost: fix fd leak in kick setup
> >
> >This patch fixes a file descriptor leak which happens in the error path
> of
> >vhost_user_set_vring_kick().
> >
> >Fixes: 4796ad63ba1f ("examples/vhost: import userspace vhost application")
> >Cc: stable at dpdk.org
> >
> >Signed-off-by: Maxime Coquelin <maxime.coquelin at redhat.com>
> >Reviewed-by: Chenbo Xia <chenbo.xia at intel.com>
> >---
> > lib/librte_vhost/vhost_user.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> >diff --git a/lib/librte_vhost/vhost_user.c
> b/lib/librte_vhost/vhost_user.c index
> >94b066f0b9..f3b2adabac 100644
> >--- a/lib/librte_vhost/vhost_user.c
> >+++ b/lib/librte_vhost/vhost_user.c
> >@@ -1855,8 +1855,12 @@ vhost_user_set_vring_kick(struct virtio_net **pdev,
> >struct VhostUserMsg *msg,
> >
> > 	/* Interpret ring addresses only when ring is started. */
> > 	dev = translate_ring_addresses(dev, file.index);
> >-	if (!dev)
> >+	if (!dev) {
> >+		if (file.fd != VIRTIO_INVALID_EVENTFD)
> >+			close(file.fd);
> >+
> > 		return RTE_VHOST_MSG_RESULT_ERR;
> >+	}
> >
> > 	*pdev = dev;
> >
> >--
> >2.26.2