On 20-Sep-17 3:34 PM, Patrick MacArthur wrote:
On 09/20/2017 05:59 AM, Kuba Kozak wrote:
Add file descriptor value check before calling close() function.
Coverity issue: 141297
Fixes: 811b6b25060f ("vfio: fix file descriptor leak in multi-process")
Cc: patr...@patrickmacarthur.net
Cc: sta...@dpdk.org
Signed-off-by: Kuba Kozak <kubax.ko...@intel.com>
---
lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
index 7e8095c..c04f548 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -301,7 +301,8 @@ vfio_mp_sync_thread(void __rte_unused * arg)
vfio_mp_sync_send_request(conn_sock, SOCKET_ERR);
else
vfio_mp_sync_send_fd(conn_sock, fd);
- close(fd);
+ if (fd != -1)
+ close(fd);
IMHO this should be:
if (fd >= 0)
What specifically is Coverity complaining about here? Is there a
specific code path that leads to fd being -1 here?
Hi Patrick,
There's no way the fd will be 0 - the function we get the value from
returns a valid fd, or a -1 in case of error. In this particular case,
the "specific code path that leads to fd being -1" is when we can't get
a container fd for some reason. I believe this is a very remote
possibility as by the time we're spinning up the socket listening thread
we're pretty sure we have a working VFIO container, but this is a valid
fix nevertheless. Maybe having it >= 0 (or > 0, to be precise) would be
cleaner, but it really makes no difference here.
--
Thanks,
Anatoly
