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

["Xia, Chenbo" <chenbo.xia@intel.com>]

Hi Xueming & Maxime,

> -----Original Message-----
> From: Xueming(Steven) Li <xuemingl@nvidia.com>
> Sent: Wednesday, November 11, 2020 2:02 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Ding, Xuan
> <xuan.ding@intel.com>; stephen@networkplumber.org; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>; stable@dpdk.org; Xia, Chenbo
> <chenbo.xia@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@dpdk.org> On Behalf Of Maxime Coquelin
> >Sent: Monday, November 9, 2020 8:17 PM
> >To: dev@dpdk.org; xuan.ding@intel.com; stephen@networkplumber.org;
> >NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; stable@dpdk.org;
> >chenbo.xia@intel.com
> >Cc: Maxime Coquelin <maxime.coquelin@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@dpdk.org
> >
> >Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >Reviewed-by: Chenbo Xia <chenbo.xia@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