From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from youngberry.canonical.com (youngberry.canonical.com [91.189.89.112]) by dpdk.org (Postfix) with ESMTP id 5B55E6CC4 for ; Wed, 6 Jul 2016 14:25:05 +0200 (CEST) Received: from 1.general.paelzer.uk.vpn ([10.172.196.172] helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1bKlt1-0002UD-W0; Wed, 06 Jul 2016 12:25:04 +0000 From: Christian Ehrhardt To: christian.ehrhardt@canonical.com, patrik.r.andersson@ericsson.com, thomas.monjalon@6wind.com, dev@dpdk.org, yuanhan.liu@linux.intel.com, huawei.xie@intel.com Date: Wed, 6 Jul 2016 14:24:58 +0200 Message-Id: <1467807898-27772-2-git-send-email-christian.ehrhardt@canonical.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1467807898-27772-1-git-send-email-christian.ehrhardt@canonical.com> References: <1467807898-27772-1-git-send-email-christian.ehrhardt@canonical.com> Subject: [dpdk-dev] [PATCH v2] vhost_user: avoid crash when exeeding file descriptors X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jul 2016 12:25:05 -0000 *update in v2* - refreshing for DPDK 16.07 - Close fd on vserver->listenfd as suggested in discussion Original From: From: Patrik Andersson Protect against DPDK crash when allocation of listen fd >= 1023. For events on fd:s >1023, the current implementation will trigger an abort due to access outside of allocated bit mask. Corrections would include: * Match fdset_add() signature in fd_man.c to fd_man.h * Handling of return codes from fdset_add() * Addition of check of fd number in fdset_add_fd() The rationale behind the suggested code change is that, fdset_event_dispatch() could attempt access outside of the FD_SET bitmask if there is an event on a file descriptor that in turn looks up a virtio file descriptor with a value > 1023. Such an attempt will lead to an abort() and a restart of any vswitch using DPDK. A discussion topic exist in the ovs-discuss mailing list that can provide a little more background: http://openvswitch.org/pipermail/discuss/2016-February/020243.html Fixes: 8f972312 ("vhost: support vhost-user") Signed-off-by: Patrik Andersson Signed-off-by: Christian Ehrhardt --- lib/librte_vhost/vhost_user/fd_man.c | 11 ++++++----- lib/librte_vhost/vhost_user/vhost-net-user.c | 19 +++++++++++++++++-- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/librte_vhost/vhost_user/fd_man.c b/lib/librte_vhost/vhost_user/fd_man.c index 087aaed..c691339 100644 --- a/lib/librte_vhost/vhost_user/fd_man.c +++ b/lib/librte_vhost/vhost_user/fd_man.c @@ -71,20 +71,22 @@ fdset_find_free_slot(struct fdset *pfdset) return fdset_find_fd(pfdset, -1); } -static void +static int fdset_add_fd(struct fdset *pfdset, int idx, int fd, fd_cb rcb, fd_cb wcb, void *dat) { struct fdentry *pfdentry; - if (pfdset == NULL || idx >= MAX_FDS) - return; + if (pfdset == NULL || idx >= MAX_FDS || fd >= FD_SETSIZE) + return -1; pfdentry = &pfdset->fd[idx]; pfdentry->fd = fd; pfdentry->rcb = rcb; pfdentry->wcb = wcb; pfdentry->dat = dat; + + return 0; } /** @@ -150,12 +152,11 @@ fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb wcb, void *dat) /* Find a free slot in the list. */ i = fdset_find_free_slot(pfdset); - if (i == -1) { + if (i == -1 || fdset_add_fd(pfdset, i, fd, rcb, wcb, dat) < 0) { pthread_mutex_unlock(&pfdset->fd_mutex); return -2; } - fdset_add_fd(pfdset, i, fd, rcb, wcb, dat); pfdset->num++; pthread_mutex_unlock(&pfdset->fd_mutex); diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c index 94f1b92..5743f52 100644 --- a/lib/librte_vhost/vhost_user/vhost-net-user.c +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c @@ -257,6 +257,7 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket) int vid; size_t size; struct vhost_user_connection *conn; + int ret; conn = malloc(sizeof(*conn)); if (conn == NULL) { @@ -278,7 +279,15 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket) conn->vsocket = vsocket; conn->vid = vid; - fdset_add(&vhost_user.fdset, fd, vhost_user_msg_handler, NULL, conn); + ret = fdset_add(&vhost_user.fdset, fd, vhost_user_msg_handler, + NULL, conn); + if (ret < 0) { + free(conn); + close(fd); + RTE_LOG(ERR, VHOST_CONFIG, + "failed to add fd %d into vhost server fdset\n", + fd); + } } /* call back when there is new vhost-user connection from client */ @@ -469,8 +478,14 @@ vhost_user_create_server(struct vhost_user_socket *vsocket) goto err; vsocket->listenfd = fd; - fdset_add(&vhost_user.fdset, fd, vhost_user_server_new_connection, + ret = fdset_add(&vhost_user.fdset, fd, vhost_user_server_new_connection, NULL, vsocket); + if (ret < 0) { + RTE_LOG(ERR, VHOST_CONFIG, + "failed to add listen fd %d to vhost server fdset\n", + fd); + goto err; + } return 0; -- 2.7.4