From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sesbmg22.ericsson.net (sesbmg22.ericsson.net [193.180.251.48]) by dpdk.org (Postfix) with ESMTP id 457B22C48 for ; Fri, 18 Mar 2016 10:14:18 +0100 (CET) X-AuditID: c1b4fb30-f79246d00000788a-90-56ebc6e96ee7 Received: from ESESSHC004.ericsson.se (Unknown_Domain [153.88.183.30]) by sesbmg22.ericsson.net (Symantec Mail Security) with SMTP id 60.53.30858.9E6CBE65; Fri, 18 Mar 2016 10:14:17 +0100 (CET) Received: from elxa6hnl062.ki.sw.ericsson.se (153.88.183.153) by smtps.internal.ericsson.com (153.88.183.30) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 18 Mar 2016 10:14:16 +0100 From: Patrik Andersson To: CC: Patrik Andersson Date: Fri, 18 Mar 2016 10:13:00 +0100 Message-ID: <1458292380-9258-1-git-send-email-patrik.r.andersson@ericsson.com> X-Mailer: git-send-email 1.9.1 MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [153.88.183.153] X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprELMWRmVeSWpSXmKPExsUyM2K7nO7LY6/DDC7O17R492k7kwOjx68F S1kDGKO4bFJSczLLUov07RK4Mm69eclYcEKh4uOWXvYGxoNSXYycHBICJhJfNrxig7DFJC7c Ww9kc3EICRxmlOi+OAXKOcAosWzKHSaQKjYBC4mVb++yg9giAkISSz9eBrOZBcwk5r/fAVYj LGAt8eHbKaBmDg4WAVWJXcuYQcK8An4SX7s+MkMsk5M4eWwyK0RcUOLkzCcsEGMkJA6+eAFW IySgI/HqzFuo45Qkrs+7zjKBkX8WkpZZSFoWMDKtYhQtTi1Oyk03MtJLLcpMLi7Oz9PLSy3Z xAgMqYNbfhvsYHz53PEQowAHoxIPb8Hy12FCrIllxZW5hxglOJiVRHg524BCvCmJlVWpRfnx RaU5qcWHGKU5WJTEeVk/XQ4TEkhPLEnNTk0tSC2CyTJxcEo1MAq6C0pIyFvvf3Rjj8qNrkUa f1x/i8oWff3VaX3SwTat9M00p4fV+y+c28zzNlZoVeuZc42hYi456+TXb6yfwrv3vEJizsu8 Buf5ljy1d6w9sx5PuLQm4l9C/9UX2k72vw+0TrJ+N3NpfVTIgr+zoue2Wm/frHjSN+tlqg5H 0Zr/p5ImpO/m6FZiKc5INNRiLipOBACaosZ4JQIAAA== Subject: [dpdk-dev] [RFC] vhost user: add error handling for fd > 1023 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: Fri, 18 Mar 2016 09:14:18 -0000 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 Signed-off-by: Patrik Andersson --- lib/librte_vhost/vhost_user/fd_man.c | 11 ++++++----- lib/librte_vhost/vhost_user/vhost-net-user.c | 22 ++++++++++++++++++++-- 2 files changed, 26 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 df2bd64..778851d 100644 --- a/lib/librte_vhost/vhost_user/vhost-net-user.c +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c @@ -288,6 +288,7 @@ vserver_new_vq_conn(int fd, void *dat, __rte_unused int *remove) int fh; struct vhost_device_ctx vdev_ctx = { (pid_t)0, 0 }; unsigned int size; + int ret; conn_fd = accept(fd, NULL, NULL); RTE_LOG(INFO, VHOST_CONFIG, @@ -317,8 +318,15 @@ vserver_new_vq_conn(int fd, void *dat, __rte_unused int *remove) ctx->vserver = vserver; ctx->fh = fh; - fdset_add(&g_vhost_server.fdset, + ret = fdset_add(&g_vhost_server.fdset, conn_fd, vserver_message_handler, NULL, ctx); + if (ret < 0) { + free(ctx); + close(conn_fd); + RTE_LOG(ERR, VHOST_CONFIG, + "failed to add fd %d into vhost server fdset\n", + conn_fd); + } } /* callback when there is message on the connfd */ @@ -453,6 +461,7 @@ int rte_vhost_driver_register(const char *path) { struct vhost_server *vserver; + int ret; pthread_mutex_lock(&g_vhost_server.server_mutex); @@ -478,8 +487,17 @@ rte_vhost_driver_register(const char *path) vserver->path = strdup(path); - fdset_add(&g_vhost_server.fdset, vserver->listenfd, + ret = fdset_add(&g_vhost_server.fdset, vserver->listenfd, vserver_new_vq_conn, NULL, vserver); + if (ret < 0) { + pthread_mutex_unlock(&g_vhost_server.server_mutex); + RTE_LOG(ERR, VHOST_CONFIG, + "failed to add listen fd %d to vhost server fdset\n", + vserver->listenfd); + free(vserver->path); + free(vserver); + return -1; + } g_vhost_server.server[g_vhost_server.vserver_cnt++] = vserver; pthread_mutex_unlock(&g_vhost_server.server_mutex); -- 1.9.1