From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34698) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3oFP-0002vI-Jb for qemu-devel@nongnu.org; Tue, 01 Dec 2015 11:57:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3oFL-0001ad-Dx for qemu-devel@nongnu.org; Tue, 01 Dec 2015 11:57:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45924) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3oFL-0001a3-7V for qemu-devel@nongnu.org; Tue, 01 Dec 2015 11:57:43 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id A1A5CA87 for ; Tue, 1 Dec 2015 16:57:42 +0000 (UTC) Date: Tue, 1 Dec 2015 18:57:39 +0200 From: Victor Kaplansky Message-ID: <1448988994-3000-1-git-send-email-victork@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Subject: [Qemu-devel] [PATCH] tests/vhost-user-bridge.c: fix fd leakage List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Victor Kaplansky , Marc-Andre Lureau , "Michael S. Tsirkin" This fixes file descriptor leakage in vhost-user-bridge application. Whenever a new callfd or kickfd is set, the previous one should be explicitly closed. File descriptors used to map guest's memory are closed immediately after mmap call. Signed-off-by: Victor Kaplansky --- tests/vhost-user-bridge.c | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c index 85c4c8a..9fb09f1 100644 --- a/tests/vhost-user-bridge.c +++ b/tests/vhost-user-bridge.c @@ -113,7 +113,6 @@ dispatcher_add(Dispatcher *dispr, int sock, void *ctx, CallbackFunc cb) return 0; } -#if 0 /* dispatcher_remove() is not currently in use but may be useful * in the future. */ static int @@ -127,9 +126,9 @@ dispatcher_remove(Dispatcher *dispr, int sock) } FD_CLR(sock, &dispr->fdset); + DPRINT("Sock %d removed from dispatcher watch.\n", sock); return 0; } -#endif /* timeout in us */ static int @@ -156,11 +155,16 @@ dispatcher_wait(Dispatcher *dispr, uint32_t timeout) /* Now call callback for every ready socket. */ int sock; - for (sock = 0; sock < dispr->max_sock + 1; sock++) - if (FD_ISSET(sock, &fdset)) { + for (sock = 0; sock < dispr->max_sock + 1; sock++) { + /* The callback on a socket can remove other sockets from the + * dispatcher, thus we have to check that the socket is + * still not removed from dispatcher's list + */ + if (FD_ISSET(sock, &fdset) && FD_ISSET(sock, &dispr->fdset)) { Event *e = &dispr->events[sock]; e->callback(sock, e->ctx); } + } return 0; } @@ -837,9 +841,10 @@ vubr_set_mem_table_exec(VubrDev *dev, VhostUserMsg *vmsg) if (mmap_addr == MAP_FAILED) { vubr_die("mmap"); } - dev_region->mmap_addr = (uint64_t) mmap_addr; DPRINT(" mmap_addr: 0x%016"PRIx64"\n", dev_region->mmap_addr); + + close(vmsg->fds[i]); } return 0; @@ -950,6 +955,17 @@ vubr_get_vring_base_exec(VubrDev *dev, VhostUserMsg *vmsg) * we have to respect * VHOST_USER_SET_VRING_ENABLE request. */ dev->ready = 0; + if (dev->vq[index].call_fd != -1) { + close(dev->vq[index].call_fd); + dispatcher_remove(&dev->dispatcher, dev->vq[index].call_fd); + dev->vq[index].call_fd = -1; + } + if (dev->vq[index].kick_fd != -1) { + close(dev->vq[index].kick_fd); + dispatcher_remove(&dev->dispatcher, dev->vq[index].kick_fd); + dev->vq[index].kick_fd = -1; + } + /* Reply */ return 1; } @@ -965,6 +981,10 @@ vubr_set_vring_kick_exec(VubrDev *dev, VhostUserMsg *vmsg) assert((u64_arg & VHOST_USER_VRING_NOFD_MASK) == 0); assert(vmsg->fd_num == 1); + if (dev->vq[index].kick_fd != -1) { + close(dev->vq[index].kick_fd); + dispatcher_remove(&dev->dispatcher, dev->vq[index].kick_fd); + } dev->vq[index].kick_fd = vmsg->fds[0]; DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index); @@ -999,6 +1019,10 @@ vubr_set_vring_call_exec(VubrDev *dev, VhostUserMsg *vmsg) assert((u64_arg & VHOST_USER_VRING_NOFD_MASK) == 0); assert(vmsg->fd_num == 1); + if (dev->vq[index].call_fd != -1) { + close(dev->vq[index].call_fd); + dispatcher_remove(&dev->dispatcher, dev->vq[index].call_fd); + } dev->vq[index].call_fd = vmsg->fds[0]; DPRINT("Got call_fd: %d for vq: %d\n", vmsg->fds[0], index); -- --Victor