[dpdk-dev,v2] vhost_user: avoid crash when exeeding file descriptors
Commit Message
*update in v2*
- refreshing for DPDK 16.07
- Close fd on vserver->listenfd as suggested in discussion
Original From:
From: Patrik Andersson <patrik.r.andersson@ericsson.com>
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 <patrik.r.andersson@ericsson.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
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(-)
Comments
On Wed, Jul 06, 2016 at 02:24:58PM +0200, Christian Ehrhardt wrote:
> *update in v2*
> - refreshing for DPDK 16.07
> - Close fd on vserver->listenfd as suggested in discussion
>
> Original From:
> From: Patrik Andersson <patrik.r.andersson@ericsson.com>
>
> 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.
Hmmm, I have no idea why I missed this email in the beginning,
otherwise, it would have been in rc2 release.
Thanks for the re-posting, and we should have had it in last
release.
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
--yliu
2016-07-12 16:37, Yuanhan Liu:
> On Wed, Jul 06, 2016 at 02:24:58PM +0200, Christian Ehrhardt wrote:
> > *update in v2*
> > - refreshing for DPDK 16.07
> > - Close fd on vserver->listenfd as suggested in discussion
> >
> > Original From:
> > From: Patrik Andersson <patrik.r.andersson@ericsson.com>
> >
> > 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.
>
> Hmmm, I have no idea why I missed this email in the beginning,
> otherwise, it would have been in rc2 release.
>
> Thanks for the re-posting, and we should have had it in last
> release.
>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Applied, thanks
@@ -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);
@@ -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;