From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 27F7B1B34C for ; Mon, 5 Feb 2018 13:17:13 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8C36F49016; Mon, 5 Feb 2018 12:17:12 +0000 (UTC) Received: from localhost (ovpn-117-200.ams2.redhat.com [10.36.117.200]) by smtp.corp.redhat.com (Postfix) with ESMTP id 205365D726; Mon, 5 Feb 2018 12:17:06 +0000 (UTC) From: Stefan Hajnoczi To: dev@dpdk.org Cc: Maxime Coquelin , Yuanhan Liu , Stefan Hajnoczi Date: Mon, 5 Feb 2018 12:16:38 +0000 Message-Id: <20180205121642.26428-5-stefanha@redhat.com> In-Reply-To: <20180205121642.26428-1-stefanha@redhat.com> References: <20180205121642.26428-1-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 05 Feb 2018 12:17:12 +0000 (UTC) Subject: [dpdk-dev] [PATCH 4/8] vhost: clear out unused SCM_RIGHTS file descriptors X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Feb 2018 12:17:13 -0000 The number of file descriptors received is not stored by vhost_user.c. vhost_user_set_mem_table() assumes that memory.nregions matches the number of file descriptors received, but nothing guarantees this: for (i = 0; i < memory.nregions; i++) close(pmsg->fds[i]); Another questionable code snippet is: case VHOST_USER_SET_LOG_FD: close(msg.fds[0]); If not enough file descriptors were received then fds[] contains uninitialized data from the stack (see read_fd_message()). This might cause non-vhost file descriptors to be closed if the uninitialized data happens to match. Refactoring vhost_user.c to pass around and check the number of file descriptors everywhere would make the code more complex. It is simpler for read_fd_message() to set unused elements in fds[] to -1. This way close(-1) is called and no harm is done. Signed-off-by: Stefan Hajnoczi --- lib/librte_vhost/socket.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c index 6e3857e7a..4e3f8abb9 100644 --- a/lib/librte_vhost/socket.c +++ b/lib/librte_vhost/socket.c @@ -96,6 +96,7 @@ read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num) size_t fdsize = fd_num * sizeof(int); char control[CMSG_SPACE(fdsize)]; struct cmsghdr *cmsg; + int got_fds = 0; int ret; memset(&msgh, 0, sizeof(msgh)); @@ -122,11 +123,16 @@ read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num) cmsg = CMSG_NXTHDR(&msgh, cmsg)) { if ((cmsg->cmsg_level == SOL_SOCKET) && (cmsg->cmsg_type == SCM_RIGHTS)) { - memcpy(fds, CMSG_DATA(cmsg), fdsize); + got_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); + memcpy(fds, CMSG_DATA(cmsg), got_fds * sizeof(int)); break; } } + /* Clear out unused file descriptors */ + while (got_fds < fd_num) + fds[got_fds++] = -1; + return ret; } -- 2.14.3