[dpdk-dev] DPDK security advisory: CVE-2019-14818

[dpdk-dev] DPDK security advisory: CVE-2019-14818

[Ferruh Yigit]

A vulnerability was fixed in DPDK.

Some downstream stakeholders were warned in advance in order to coordinate the
release of fixes and reduce the vulnerability window.

Problem:
A malicious container which has direct access to the vhost-user socket can keep
sending messages which may cause leaking resources until resulting a DOS.

All users of the vhost library are strongly encouraged to upgrade as soon as
possible.

CVE-2019-14818
Bugzilla: https://bugs.dpdk.org/show_bug.cgi?id=363
Severity: Medium
CVSS scores: 6.8



Commits:
main repo
https://git.dpdk.org/dpdk/commit/?id=612e17cf6d7b
https://git.dpdk.org/dpdk/commit/?id=bf472259dde6

19.08.1
https://git.dpdk.org/dpdk-stable/commit/?h=19.08&id=fa674d08985f
https://git.dpdk.org/dpdk-stable/commit/?h=19.08&id=6547dd563ea9

18.11.4 (LTS)
https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=70583a6b9b1c
https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=f8898927bb16

17.11.8 (LTS)
https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=3b1b44a1c82a
https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=8a8dbd0ec19e
https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=1f6147d9a01f

16.11.10 (LTS EOL)
https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=5fbb5c2919b6
https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=3863340f93b8
https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=8790f4c3bcd2
https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=1bf11cfb7c7c


Stable Releases download links:
DPDK 19.08.1
http://fast.dpdk.org/rel/dpdk-19.08.1.tar.xz

DPDK 18.11.4 (LTS)
http://fast.dpdk.org/rel/dpdk-18.11.4.tar.xz

DPDK 17.11.8 (LTS)
http://fast.dpdk.org/rel/dpdk-17.11.8.tar.xz

DPDK 16.11.10 (LTS EOL)
http://fast.dpdk.org/rel/dpdk-16.11.10.tar.xz


-- 
DPDK Security Team
http://core.dpdk.org/security/

[dpdk-dev] [v16.11 PATCH v2 1/4] vhost: validate virtqueue size

[Maxime Coquelin]

From: Stefan Hajnoczi <stefanha@redhat.com>

[ backported from upstream commit eb7c574b21cc92792ea5a1f219ddf6dd3cf3b1e1 ]

Check the virtqueue size constraints so that invalid values don't cause
bugs later on in the code.  For example, sometimes the virtqueue size is
stored as unsigned int and sometimes as uint16_t, so bad things happen
if it is ever larger than 65535.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 618d413fe1..8a01c295e7 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -189,6 +189,17 @@ vhost_user_set_vring_num(struct virtio_net *dev,
 
 	vq->size = state->num;
 
+	/* VIRTIO 1.0, 2.4 Virtqueues says:
+	 *
+	 *   Queue Size value is always a power of 2. The maximum Queue Size
+	 *   value is 32768.
+	 */
+	if ((vq->size & (vq->size - 1)) || vq->size > 32768) {
+		RTE_LOG(ERR, VHOST_CONFIG,
+			"invalid virtqueue size %u\n", vq->size);
+		return -1;
+	}
+
 	if (dev->dequeue_zero_copy) {
 		vq->nr_zmbuf = 0;
 		vq->last_zmbuf_idx = 0;
-- 
2.21.0

[dpdk-dev] [v16.11 PATCH v2 2/4] vhost: add number of fds to vhost-user messages

[Maxime Coquelin]

As soon as some ancillary data (fds) are received, it is copied
without checking its length.

This patch adds the number of fds received to the message,
which is set in read_vhost_message().

This is preliminary work to support sending fds to Qemu.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
(cherry picked from commit c00bb88d35fe975ede0ea35bdf4f765a2cece7e8)
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/socket.c     | 22 +++++++++++++++++-----
 lib/librte_vhost/vhost_user.c |  2 +-
 lib/librte_vhost/vhost_user.h |  4 +++-
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 805b2e5b23..4a19280fb3 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -101,17 +101,23 @@ static struct vhost_user vhost_user = {
 	.mutex = PTHREAD_MUTEX_INITIALIZER,
 };
 
-/* return bytes# of read on success or negative val on failure. */
+/*
+ * return bytes# of read on success or negative val on failure. Update fdnum
+ * with number of fds read.
+ */
 int
-read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num)
+read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds,
+		int *fd_num)
 {
 	struct iovec iov;
 	struct msghdr msgh;
-	size_t fdsize = fd_num * sizeof(int);
-	char control[CMSG_SPACE(fdsize)];
+	char control[CMSG_SPACE(max_fds * sizeof(int))];
 	struct cmsghdr *cmsg;
+	int got_fds = 0;
 	int ret;
 
+	*fd_num = 0;
+
 	memset(&msgh, 0, sizeof(msgh));
 	iov.iov_base = buf;
 	iov.iov_len  = buflen;
@@ -136,11 +142,17 @@ 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);
+			*fd_num = got_fds;
+			memcpy(fds, CMSG_DATA(cmsg), got_fds * sizeof(int));
 			break;
 		}
 	}
 
+	/* Clear out unused file descriptors */
+	while (got_fds < max_fds)
+		fds[got_fds++] = -1;
+
 	return ret;
 }
 
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 8a01c295e7..b8f6a9fba5 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -963,7 +963,7 @@ read_vhost_message(int sockfd, struct VhostUserMsg *msg)
 	int ret;
 
 	ret = read_fd_message(sockfd, (char *)msg, VHOST_USER_HDR_SIZE,
-		msg->fds, VHOST_MEMORY_MAX_NREGIONS);
+		msg->fds, VHOST_MEMORY_MAX_NREGIONS, &msg->fd_num);
 	if (ret <= 0)
 		return ret;
 
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index ba78d32684..ea9d304f82 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -110,6 +110,7 @@ typedef struct VhostUserMsg {
 		VhostUserLog    log;
 	} payload;
 	int fds[VHOST_MEMORY_MAX_NREGIONS];
+	int fd_num;
 } __attribute((packed)) VhostUserMsg;
 
 #define VHOST_USER_HDR_SIZE offsetof(VhostUserMsg, payload.u64)
@@ -122,7 +123,8 @@ typedef struct VhostUserMsg {
 int vhost_user_msg_handler(int vid, int fd);
 
 /* socket.c */
-int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num);
+int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds,
+		int *fd_num);
 int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num);
 
 #endif
-- 
2.21.0

[dpdk-dev] [v16.11 PATCH v2 3/4] vhost: fix possible denial of service on SET_VRING_NUM

[Maxime Coquelin]

vhost_user_set_vring_num() performs multiple allocations
without checking whether data were previously allocated.

It may cause a denial of service because of the memory leaks
that happen if a malicious vhost-user master keeps sending
VHOST_USER_SET_VRING_NUM request until the slave runs out
of memory.

This issue has been assigned CVE-2019-14818

Reported-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index b8f6a9fba5..cebc72f78f 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -204,6 +204,8 @@ vhost_user_set_vring_num(struct virtio_net *dev,
 		vq->nr_zmbuf = 0;
 		vq->last_zmbuf_idx = 0;
 		vq->zmbuf_size = vq->size;
+		if (vq->zmbufs)
+			rte_free(vq->zmbufs);
 		vq->zmbufs = rte_zmalloc(NULL, vq->zmbuf_size *
 					 sizeof(struct zcopy_mbuf), 0);
 		if (vq->zmbufs == NULL) {
@@ -213,7 +215,8 @@ vhost_user_set_vring_num(struct virtio_net *dev,
 			dev->dequeue_zero_copy = 0;
 		}
 	}
-
+	if (vq->shadow_used_ring)
+		rte_free(vq->shadow_used_ring);
 	vq->shadow_used_ring = rte_malloc(NULL,
 				vq->size * sizeof(struct vring_used_elem),
 				RTE_CACHE_LINE_SIZE);
-- 
2.21.0

[dpdk-dev] [v16.11 PATCH v2 4/4] vhost: fix possible denial of service by leaking FDs

[Maxime Coquelin]

A malicious Vhost-user master could send in loop hand-crafted
vhost-user messages containing more file descriptors the
vhost-user slave expects. Doing so causes the application using
the vhost-user library to run out of FDs.

This issue has been assigned CVE-2019-14818

Fixes: 8f972312b8f4 ("vhost: support vhost-user")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 87 +++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index cebc72f78f..a6ab131543 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -75,6 +75,36 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_SEND_RARP]  = "VHOST_USER_SEND_RARP",
 };
 
+static void
+close_msg_fds(struct VhostUserMsg *msg)
+{
+	int i;
+
+	for (i = 0; i < msg->fd_num; i++)
+		close(msg->fds[i]);
+}
+
+/*
+ * Ensure the expected number of FDs is received,
+ * close all FDs and return an error if this is not the case.
+ */
+static int
+validate_msg_fds(struct VhostUserMsg *msg, int expected_fds)
+{
+	if (msg->fd_num == expected_fds)
+		return 0;
+
+	RTE_LOG(ERR, VHOST_CONFIG,
+		" Expect %d FDs for request %s, received %d\n",
+		expected_fds,
+		vhost_message_str[msg->request],
+		msg->fd_num);
+
+	close_msg_fds(msg);
+
+	return -1;
+}
+
 static uint64_t
 get_blk_size(int fd)
 {
@@ -1104,35 +1134,59 @@ vhost_user_msg_handler(int vid, int fd)
 	ret = 0;
 	switch (msg.request) {
 	case VHOST_USER_GET_FEATURES:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		msg.payload.u64 = vhost_user_get_features();
 		msg.size = sizeof(msg.payload.u64);
 		send_vhost_message(fd, &msg);
 		break;
 	case VHOST_USER_SET_FEATURES:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_set_features(dev, msg.payload.u64);
 		break;
 
 	case VHOST_USER_GET_PROTOCOL_FEATURES:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		msg.payload.u64 = VHOST_USER_PROTOCOL_FEATURES;
 		msg.size = sizeof(msg.payload.u64);
 		send_vhost_message(fd, &msg);
 		break;
 	case VHOST_USER_SET_PROTOCOL_FEATURES:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_set_protocol_features(dev, msg.payload.u64);
 		break;
 
 	case VHOST_USER_SET_OWNER:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_set_owner();
 		break;
 	case VHOST_USER_RESET_OWNER:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_reset_owner(dev);
 		break;
 
 	case VHOST_USER_SET_MEM_TABLE:
+		if (validate_msg_fds(&msg, msg.payload.memory.nregions) != 0)
+			return -1;
+
 		vhost_user_set_mem_table(dev, &msg);
 		break;
 
 	case VHOST_USER_SET_LOG_BASE:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		ret = vhost_user_set_log_base(dev, &msg);
 		if (ret)
 			break;
@@ -1144,21 +1198,36 @@ vhost_user_msg_handler(int vid, int fd)
 		send_vhost_message(fd, &msg);
 		break;
 	case VHOST_USER_SET_LOG_FD:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		close(msg.fds[0]);
 		RTE_LOG(INFO, VHOST_CONFIG, "not implemented.\n");
 		break;
 
 	case VHOST_USER_SET_VRING_NUM:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_set_vring_num(dev, &msg.payload.state);
 		break;
 	case VHOST_USER_SET_VRING_ADDR:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_set_vring_addr(&dev, &msg.payload.addr);
 		break;
 	case VHOST_USER_SET_VRING_BASE:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_set_vring_base(dev, &msg.payload.state);
 		break;
 
 	case VHOST_USER_GET_VRING_BASE:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_get_vring_base(dev, &msg.payload.state);
 		if (ret)
 			break;
@@ -1167,28 +1236,46 @@ vhost_user_msg_handler(int vid, int fd)
 		break;
 
 	case VHOST_USER_SET_VRING_KICK:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		ret = vhost_user_set_vring_kick(dev, &msg);
 		break;
 	case VHOST_USER_SET_VRING_CALL:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		vhost_user_set_vring_call(dev, &msg);
 		break;
 
 	case VHOST_USER_SET_VRING_ERR:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		if (!(msg.payload.u64 & VHOST_USER_VRING_NOFD_MASK))
 			close(msg.fds[0]);
 		RTE_LOG(INFO, VHOST_CONFIG, "not implemented\n");
 		break;
 
 	case VHOST_USER_GET_QUEUE_NUM:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		msg.payload.u64 = VHOST_MAX_QUEUE_PAIRS;
 		msg.size = sizeof(msg.payload.u64);
 		send_vhost_message(fd, &msg);
 		break;
 
 	case VHOST_USER_SET_VRING_ENABLE:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_set_vring_enable(dev, &msg.payload.state);
 		break;
 	case VHOST_USER_SEND_RARP:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_send_rarp(dev, &msg);
 		break;
 
-- 
2.21.0

[dpdk-dev] [v17.11 PATCH v2 1/4] vhost: validate virtqueue size

[Maxime Coquelin]

From: Stefan Hajnoczi <stefanha@redhat.com>

[ backported from upstream commit eb7c574b21cc92792ea5a1f219ddf6dd3cf3b1e1 ]

Check the virtqueue size constraints so that invalid values don't cause
bugs later on in the code.  For example, sometimes the virtqueue size is
stored as unsigned int and sometimes as uint16_t, so bad things happen
if it is ever larger than 65535.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index bb39999aa4..93e871c5bb 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -247,6 +247,17 @@ vhost_user_set_vring_num(struct virtio_net *dev,
 
 	vq->size = msg->payload.state.num;
 
+	/* VIRTIO 1.0, 2.4 Virtqueues says:
+	 *
+	 *   Queue Size value is always a power of 2. The maximum Queue Size
+	 *   value is 32768.
+	 */
+	if ((vq->size & (vq->size - 1)) || vq->size > 32768) {
+		RTE_LOG(ERR, VHOST_CONFIG,
+			"invalid virtqueue size %u\n", vq->size);
+		return -1;
+	}
+
 	if (dev->dequeue_zero_copy) {
 		vq->nr_zmbuf = 0;
 		vq->last_zmbuf_idx = 0;
-- 
2.21.0

[dpdk-dev] [v17.11 PATCH v2 2/4] vhost: add number of fds to vhost-user messages

[Maxime Coquelin]

As soon as some ancillary data (fds) are received, it is copied
without checking its length.

This patch adds the number of fds received to the message,
which is set in read_vhost_message().

This is preliminary work to support sending fds to Qemu.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
(cherry picked from commit c00bb88d35fe975ede0ea35bdf4f765a2cece7e8)
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/socket.c     | 22 +++++++++++++++++-----
 lib/librte_vhost/vhost_user.c |  2 +-
 lib/librte_vhost/vhost_user.h |  4 +++-
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 88be697c2f..2fa7ea0e09 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -117,17 +117,23 @@ static struct vhost_user vhost_user = {
 	.mutex = PTHREAD_MUTEX_INITIALIZER,
 };
 
-/* return bytes# of read on success or negative val on failure. */
+/*
+ * return bytes# of read on success or negative val on failure. Update fdnum
+ * with number of fds read.
+ */
 int
-read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num)
+read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds,
+		int *fd_num)
 {
 	struct iovec iov;
 	struct msghdr msgh;
-	size_t fdsize = fd_num * sizeof(int);
-	char control[CMSG_SPACE(fdsize)];
+	char control[CMSG_SPACE(max_fds * sizeof(int))];
 	struct cmsghdr *cmsg;
+	int got_fds = 0;
 	int ret;
 
+	*fd_num = 0;
+
 	memset(&msgh, 0, sizeof(msgh));
 	iov.iov_base = buf;
 	iov.iov_len  = buflen;
@@ -152,11 +158,17 @@ 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);
+			*fd_num = got_fds;
+			memcpy(fds, CMSG_DATA(cmsg), got_fds * sizeof(int));
 			break;
 		}
 	}
 
+	/* Clear out unused file descriptors */
+	while (got_fds < max_fds)
+		fds[got_fds++] = -1;
+
 	return ret;
 }
 
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 93e871c5bb..9773691097 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1248,7 +1248,7 @@ read_vhost_message(int sockfd, struct VhostUserMsg *msg)
 	int ret;
 
 	ret = read_fd_message(sockfd, (char *)msg, VHOST_USER_HDR_SIZE,
-		msg->fds, VHOST_MEMORY_MAX_NREGIONS);
+		msg->fds, VHOST_MEMORY_MAX_NREGIONS, &msg->fd_num);
 	if (ret <= 0)
 		return ret;
 
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 76d9fe2fc5..10b8cc5e1a 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -130,6 +130,7 @@ typedef struct VhostUserMsg {
 		struct vhost_iotlb_msg iotlb;
 	} payload;
 	int fds[VHOST_MEMORY_MAX_NREGIONS];
+	int fd_num;
 } __attribute((packed)) VhostUserMsg;
 
 #define VHOST_USER_HDR_SIZE offsetof(VhostUserMsg, payload.u64)
@@ -143,7 +144,8 @@ int vhost_user_msg_handler(int vid, int fd);
 int vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm);
 
 /* socket.c */
-int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num);
+int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds,
+		int *fd_num);
 int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num);
 
 #endif
-- 
2.21.0

[dpdk-dev] [v17.11 PATCH v2 3/4] vhost: fix possible denial of service on SET_VRING_NUM

[Maxime Coquelin]

vhost_user_set_vring_num() performs multiple allocations
without checking whether data were previously allocated.

It may cause a denial of service because of the memory leaks
that happen if a malicious vhost-user master keeps sending
VHOST_USER_SET_VRING_NUM request until the slave runs out
of memory.

This issue has been assigned CVE-2019-14818

Reported-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 9773691097..781734e9e3 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -262,6 +262,8 @@ vhost_user_set_vring_num(struct virtio_net *dev,
 		vq->nr_zmbuf = 0;
 		vq->last_zmbuf_idx = 0;
 		vq->zmbuf_size = vq->size;
+		if (vq->zmbufs)
+			rte_free(vq->zmbufs);
 		vq->zmbufs = rte_zmalloc(NULL, vq->zmbuf_size *
 					 sizeof(struct zcopy_mbuf), 0);
 		if (vq->zmbufs == NULL) {
@@ -271,7 +273,8 @@ vhost_user_set_vring_num(struct virtio_net *dev,
 			dev->dequeue_zero_copy = 0;
 		}
 	}
-
+	if (vq->shadow_used_ring)
+		rte_free(vq->shadow_used_ring);
 	vq->shadow_used_ring = rte_malloc(NULL,
 				vq->size * sizeof(struct vring_used_elem),
 				RTE_CACHE_LINE_SIZE);
@@ -281,6 +284,8 @@ vhost_user_set_vring_num(struct virtio_net *dev,
 		return -1;
 	}
 
+	if (vq->batch_copy_elems)
+		rte_free(vq->batch_copy_elems);
 	vq->batch_copy_elems = rte_malloc(NULL,
 				vq->size * sizeof(struct batch_copy_elem),
 				RTE_CACHE_LINE_SIZE);
-- 
2.21.0

[dpdk-dev] [v17.11 PATCH v2 4/4] vhost: fix possible denial of service by leaking FDs

[Maxime Coquelin]

A malicious Vhost-user master could send in loop hand-crafted
vhost-user messages containing more file descriptors the
vhost-user slave expects. Doing so causes the application using
the vhost-user library to run out of FDs.

This issue has been assigned CVE-2019-14818

Fixes: 8f972312b8f4 ("vhost: support vhost-user")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 95 +++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 781734e9e3..d4643dc350 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -81,6 +81,36 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_IOTLB_MSG]  = "VHOST_USER_IOTLB_MSG",
 };
 
+static void
+close_msg_fds(struct VhostUserMsg *msg)
+{
+	int i;
+
+	for (i = 0; i < msg->fd_num; i++)
+		close(msg->fds[i]);
+}
+
+/*
+ * Ensure the expected number of FDs is received,
+ * close all FDs and return an error if this is not the case.
+ */
+static int
+validate_msg_fds(struct VhostUserMsg *msg, int expected_fds)
+{
+	if (msg->fd_num == expected_fds)
+		return 0;
+
+	RTE_LOG(ERR, VHOST_CONFIG,
+		" Expect %d FDs for request %s, received %d\n",
+		expected_fds,
+		vhost_message_str[msg->request.master],
+		msg->fd_num);
+
+	close_msg_fds(msg);
+
+	return -1;
+}
+
 static uint64_t
 get_blk_size(int fd)
 {
@@ -1458,34 +1488,58 @@ vhost_user_msg_handler(int vid, int fd)
 
 	switch (msg.request.master) {
 	case VHOST_USER_GET_FEATURES:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		msg.payload.u64 = vhost_user_get_features(dev);
 		msg.size = sizeof(msg.payload.u64);
 		send_vhost_reply(fd, &msg);
 		break;
 	case VHOST_USER_SET_FEATURES:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_set_features(dev, msg.payload.u64);
 		break;
 
 	case VHOST_USER_GET_PROTOCOL_FEATURES:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_get_protocol_features(dev, &msg);
 		send_vhost_reply(fd, &msg);
 		break;
 	case VHOST_USER_SET_PROTOCOL_FEATURES:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_set_protocol_features(dev, msg.payload.u64);
 		break;
 
 	case VHOST_USER_SET_OWNER:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_set_owner();
 		break;
 	case VHOST_USER_RESET_OWNER:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_reset_owner(dev);
 		break;
 
 	case VHOST_USER_SET_MEM_TABLE:
+		if (validate_msg_fds(&msg, msg.payload.memory.nregions) != 0)
+			return -1;
+
 		ret = vhost_user_set_mem_table(&dev, &msg);
 		break;
 
 	case VHOST_USER_SET_LOG_BASE:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		vhost_user_set_log_base(dev, &msg);
 
 		/*
@@ -1496,61 +1550,102 @@ vhost_user_msg_handler(int vid, int fd)
 		send_vhost_reply(fd, &msg);
 		break;
 	case VHOST_USER_SET_LOG_FD:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		close(msg.fds[0]);
 		RTE_LOG(INFO, VHOST_CONFIG, "not implemented.\n");
 		break;
 
 	case VHOST_USER_SET_VRING_NUM:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_set_vring_num(dev, &msg);
 		break;
 	case VHOST_USER_SET_VRING_ADDR:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_set_vring_addr(&dev, &msg);
 		break;
 	case VHOST_USER_SET_VRING_BASE:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_set_vring_base(dev, &msg);
 		break;
 
 	case VHOST_USER_GET_VRING_BASE:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_get_vring_base(dev, &msg);
 		msg.size = sizeof(msg.payload.state);
 		send_vhost_reply(fd, &msg);
 		break;
 
 	case VHOST_USER_SET_VRING_KICK:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		vhost_user_set_vring_kick(&dev, &msg);
 		break;
 	case VHOST_USER_SET_VRING_CALL:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		vhost_user_set_vring_call(dev, &msg);
 		break;
 
 	case VHOST_USER_SET_VRING_ERR:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		if (!(msg.payload.u64 & VHOST_USER_VRING_NOFD_MASK))
 			close(msg.fds[0]);
 		RTE_LOG(INFO, VHOST_CONFIG, "not implemented\n");
 		break;
 
 	case VHOST_USER_GET_QUEUE_NUM:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
 		msg.payload.u64 = VHOST_MAX_QUEUE_PAIRS;
 		msg.size = sizeof(msg.payload.u64);
 		send_vhost_reply(fd, &msg);
 		break;
 
 	case VHOST_USER_SET_VRING_ENABLE:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_set_vring_enable(dev, &msg);
 		break;
 	case VHOST_USER_SEND_RARP:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		vhost_user_send_rarp(dev, &msg);
 		break;
 
 	case VHOST_USER_NET_SET_MTU:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_net_set_mtu(dev, &msg);
 		break;
 
 	case VHOST_USER_SET_SLAVE_REQ_FD:
+		if (validate_msg_fds(&msg, 1) != 0)
+			return -1;
+
 		ret = vhost_user_set_req_fd(dev, &msg);
 		break;
 
 	case VHOST_USER_IOTLB_MSG:
+		if (validate_msg_fds(&msg, 0) != 0)
+			return -1;
+
 		ret = vhost_user_iotlb_msg(&dev, &msg);
 		break;
 
-- 
2.21.0

[dpdk-dev] [v18.11 PATCH v2 1/2] vhost: fix possible denial of service on SET_VRING_NUM

[Maxime Coquelin]

vhost_user_set_vring_num() performs multiple allocations
without checking whether data were previously allocated.

It may cause a denial of service because of the memory leaks
that happen if a malicious vhost-user master keeps sending
VHOST_USER_SET_VRING_NUM request until the slave runs out
of memory.

This issue has been assigned CVE-2019-14818

Reported-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 5552f8bbfb..457e62d97e 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -346,6 +346,8 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 		vq->nr_zmbuf = 0;
 		vq->last_zmbuf_idx = 0;
 		vq->zmbuf_size = vq->size;
+		if (vq->zmbufs)
+			rte_free(vq->zmbufs);
 		vq->zmbufs = rte_zmalloc(NULL, vq->zmbuf_size *
 					 sizeof(struct zcopy_mbuf), 0);
 		if (vq->zmbufs == NULL) {
@@ -358,6 +360,8 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 	}
 
 	if (vq_is_packed(dev)) {
+		if (vq->shadow_used_packed)
+			rte_free(vq->shadow_used_packed);
 		vq->shadow_used_packed = rte_malloc(NULL,
 				vq->size *
 				sizeof(struct vring_used_elem_packed),
@@ -369,6 +373,8 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 		}
 
 	} else {
+		if (vq->shadow_used_split)
+			rte_free(vq->shadow_used_split);
 		vq->shadow_used_split = rte_malloc(NULL,
 				vq->size * sizeof(struct vring_used_elem),
 				RTE_CACHE_LINE_SIZE);
@@ -379,6 +385,8 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 		}
 	}
 
+	if (vq->batch_copy_elems)
+		rte_free(vq->batch_copy_elems);
 	vq->batch_copy_elems = rte_malloc(NULL,
 				vq->size * sizeof(struct batch_copy_elem),
 				RTE_CACHE_LINE_SIZE);
-- 
2.21.0

[dpdk-dev] [v18.11 PATCH v2 2/2] vhost: fix possible denial of service by leaking FDs

[Maxime Coquelin]

A malicious Vhost-user master could send in loop hand-crafted
vhost-user messages containing more file descriptors the
vhost-user slave expects. Doing so causes the application using
the vhost-user library to run out of FDs.

This issue has been assigned CVE-2019-14818

Fixes: 8f972312b8f4 ("vhost: support vhost-user")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 118 ++++++++++++++++++++++++++++++++--
 1 file changed, 114 insertions(+), 4 deletions(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 457e62d97e..98cd670e03 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -83,6 +83,36 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg);
 static int read_vhost_message(int sockfd, struct VhostUserMsg *msg);
 
+static void
+close_msg_fds(struct VhostUserMsg *msg)
+{
+	int i;
+
+	for (i = 0; i < msg->fd_num; i++)
+		close(msg->fds[i]);
+}
+
+/*
+ * Ensure the expected number of FDs is received,
+ * close all FDs and return an error if this is not the case.
+ */
+static int
+validate_msg_fds(struct VhostUserMsg *msg, int expected_fds)
+{
+	if (msg->fd_num == expected_fds)
+		return 0;
+
+	RTE_LOG(ERR, VHOST_CONFIG,
+		" Expect %d FDs for request %s, received %d\n",
+		expected_fds,
+		vhost_message_str[msg->request.master],
+		msg->fd_num);
+
+	close_msg_fds(msg);
+
+	return -1;
+}
+
 static uint64_t
 get_blk_size(int fd)
 {
@@ -179,18 +209,25 @@ vhost_backend_cleanup(struct virtio_net *dev)
  */
 static int
 vhost_user_set_owner(struct virtio_net **pdev __rte_unused,
-			struct VhostUserMsg *msg __rte_unused,
+			struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	return VH_RESULT_OK;
 }
 
 static int
 vhost_user_reset_owner(struct virtio_net **pdev,
-			struct VhostUserMsg *msg __rte_unused,
+			struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
 	struct virtio_net *dev = *pdev;
+
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	vhost_destroy_device_notify(dev);
 
 	cleanup_device(dev, 0);
@@ -208,6 +245,9 @@ vhost_user_get_features(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct virtio_net *dev = *pdev;
 	uint64_t features = 0;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	rte_vhost_driver_get_features(dev->ifname, &features);
 
 	msg->payload.u64 = features;
@@ -227,6 +267,9 @@ vhost_user_get_queue_num(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct virtio_net *dev = *pdev;
 	uint32_t queue_num = 0;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	rte_vhost_driver_get_queue_num(dev->ifname, &queue_num);
 
 	msg->payload.u64 = (uint64_t)queue_num;
@@ -249,6 +292,9 @@ vhost_user_set_features(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct rte_vdpa_device *vdpa_dev;
 	int did = -1;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	rte_vhost_driver_get_features(dev->ifname, &vhost_features);
 	if (features & ~vhost_features) {
 		RTE_LOG(ERR, VHOST_CONFIG,
@@ -329,6 +375,9 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 	struct virtio_net *dev = *pdev;
 	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	vq->size = msg->payload.state.num;
 
 	/* VIRTIO 1.0, 2.4 Virtqueues says:
@@ -708,6 +757,9 @@ vhost_user_set_vring_addr(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct vhost_virtqueue *vq;
 	struct vhost_vring_addr *addr = &msg->payload.addr;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	if (dev->mem == NULL)
 		return VH_RESULT_ERR;
 
@@ -746,6 +798,9 @@ vhost_user_set_vring_base(struct virtio_net **pdev,
 	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
 	uint64_t val = msg->payload.state.num;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	if (vq_is_packed(dev)) {
 		/*
 		 * Bit[0:14]: avail index
@@ -907,6 +962,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	int populate;
 	int fd;
 
+	if (validate_msg_fds(msg, memory->nregions) != 0)
+		return VH_RESULT_ERR;
+
 	if (memory->nregions > VHOST_MEMORY_MAX_NREGIONS) {
 		RTE_LOG(ERR, VHOST_CONFIG,
 			"too many memory regions (%u)\n", memory->nregions);
@@ -917,8 +975,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
 		RTE_LOG(INFO, VHOST_CONFIG,
 			"(%d) memory regions not changed\n", dev->vid);
 
-		for (i = 0; i < memory->nregions; i++)
-			close(msg->fds[i]);
+		close_msg_fds(msg);
 
 		return VH_RESULT_OK;
 	}
@@ -1061,6 +1118,10 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
 				"Failed to read qemu ack on postcopy set-mem-table\n");
 			goto err_mmap;
 		}
+
+		if (validate_msg_fds(&ack_msg, 0) != 0)
+			goto err_mmap;
+
 		if (ack_msg.request.master != VHOST_USER_SET_MEM_TABLE) {
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"Bad qemu ack on postcopy set-mem-table (%d)\n",
@@ -1181,6 +1242,9 @@ vhost_user_set_vring_call(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct vhost_vring_file file;
 	struct vhost_virtqueue *vq;
 
+	if (validate_msg_fds(msg, 1) != 0)
+		return VH_RESULT_ERR;
+
 	file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
 	if (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
 		file.fd = VIRTIO_INVALID_EVENTFD;
@@ -1202,6 +1266,9 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
 			struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
+	if (validate_msg_fds(msg, 1) != 0)
+		return VH_RESULT_ERR;
+
 	if (!(msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK))
 		close(msg->fds[0]);
 	RTE_LOG(INFO, VHOST_CONFIG, "not implemented\n");
@@ -1217,6 +1284,9 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct vhost_vring_file file;
 	struct vhost_virtqueue *vq;
 
+	if (validate_msg_fds(msg, 1) != 0)
+		return VH_RESULT_ERR;
+
 	file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
 	if (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
 		file.fd = VIRTIO_INVALID_EVENTFD;
@@ -1273,6 +1343,9 @@ vhost_user_get_vring_base(struct virtio_net **pdev,
 	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
 	uint64_t val;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	/* We have to stop the queue (virtio) if it is running. */
 	vhost_destroy_device_notify(dev);
 
@@ -1346,6 +1419,9 @@ vhost_user_set_vring_enable(struct virtio_net **pdev,
 	struct rte_vdpa_device *vdpa_dev;
 	int did = -1;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	RTE_LOG(INFO, VHOST_CONFIG,
 		"set queue enable: %d to qp idx: %d\n",
 		enable, index);
@@ -1376,6 +1452,9 @@ vhost_user_get_protocol_features(struct virtio_net **pdev,
 	struct virtio_net *dev = *pdev;
 	uint64_t features, protocol_features;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	rte_vhost_driver_get_features(dev->ifname, &features);
 	rte_vhost_driver_get_protocol_features(dev->ifname, &protocol_features);
 
@@ -1404,6 +1483,9 @@ vhost_user_set_protocol_features(struct virtio_net **pdev,
 	uint64_t protocol_features = msg->payload.u64;
 	uint64_t slave_protocol_features = 0;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	rte_vhost_driver_get_protocol_features(dev->ifname,
 			&slave_protocol_features);
 	if (protocol_features & ~slave_protocol_features) {
@@ -1427,6 +1509,9 @@ vhost_user_set_log_base(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	uint64_t size, off;
 	void *addr;
 
+	if (validate_msg_fds(msg, 1) != 0)
+		return VH_RESULT_ERR;
+
 	if (fd < 0) {
 		RTE_LOG(ERR, VHOST_CONFIG, "invalid log fd: %d\n", fd);
 		return VH_RESULT_ERR;
@@ -1490,6 +1575,9 @@ static int vhost_user_set_log_fd(struct virtio_net **pdev __rte_unused,
 			struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
+	if (validate_msg_fds(msg, 1) != 0)
+		return VH_RESULT_ERR;
+
 	close(msg->fds[0]);
 	RTE_LOG(INFO, VHOST_CONFIG, "not implemented.\n");
 
@@ -1513,6 +1601,9 @@ vhost_user_send_rarp(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct rte_vdpa_device *vdpa_dev;
 	int did = -1;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	RTE_LOG(DEBUG, VHOST_CONFIG,
 		":: mac: %02x:%02x:%02x:%02x:%02x:%02x\n",
 		mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
@@ -1540,6 +1631,10 @@ vhost_user_net_set_mtu(struct virtio_net **pdev, struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
 	struct virtio_net *dev = *pdev;
+
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	if (msg->payload.u64 < VIRTIO_MIN_MTU ||
 			msg->payload.u64 > VIRTIO_MAX_MTU) {
 		RTE_LOG(ERR, VHOST_CONFIG, "Invalid MTU size (%"PRIu64")\n",
@@ -1560,6 +1655,9 @@ vhost_user_set_req_fd(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct virtio_net *dev = *pdev;
 	int fd = msg->fds[0];
 
+	if (validate_msg_fds(msg, 1) != 0)
+		return VH_RESULT_ERR;
+
 	if (fd < 0) {
 		RTE_LOG(ERR, VHOST_CONFIG,
 				"Invalid file descriptor for slave channel (%d)\n",
@@ -1630,6 +1728,9 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	uint16_t i;
 	uint64_t vva, len;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	switch (imsg->type) {
 	case VHOST_IOTLB_UPDATE:
 		len = imsg->size;
@@ -1676,6 +1777,9 @@ vhost_user_set_postcopy_advise(struct virtio_net **pdev,
 #ifdef RTE_LIBRTE_VHOST_POSTCOPY
 	struct uffdio_api api_struct;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
 
 	if (dev->postcopy_ufd == -1) {
@@ -1711,6 +1815,9 @@ vhost_user_set_postcopy_listen(struct virtio_net **pdev,
 {
 	struct virtio_net *dev = *pdev;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	if (dev->mem && dev->mem->nregions) {
 		RTE_LOG(ERR, VHOST_CONFIG,
 			"Regions already registered at postcopy-listen\n");
@@ -1727,6 +1834,9 @@ vhost_user_postcopy_end(struct virtio_net **pdev, struct VhostUserMsg *msg,
 {
 	struct virtio_net *dev = *pdev;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return VH_RESULT_ERR;
+
 	dev->postcopy_listening = 0;
 	if (dev->postcopy_ufd >= 0) {
 		close(dev->postcopy_ufd);
-- 
2.21.0

[dpdk-dev] [master PATCH v2 1/2] vhost: fix possible denial of service on SET_VRING_NUM

[Maxime Coquelin]

vhost_user_set_vring_num() performs multiple allocations
without checking whether data were previously allocated.

It may cause a denial of service because of the memory leaks
that happen if a malicious vhost-user master keeps sending
VHOST_USER_SET_VRING_NUM request until the slave runs out
of memory.

This issue has been assigned CVE-2019-14818

Fixes: b0a985d1f340 ("vhost: add dequeue zero copy")

Reported-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index ce4e9fb32f..6d2431e604 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -348,6 +348,8 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 		vq->nr_zmbuf = 0;
 		vq->last_zmbuf_idx = 0;
 		vq->zmbuf_size = vq->size;
+		if (vq->zmbufs)
+			rte_free(vq->zmbufs);
 		vq->zmbufs = rte_zmalloc(NULL, vq->zmbuf_size *
 					 sizeof(struct zcopy_mbuf), 0);
 		if (vq->zmbufs == NULL) {
@@ -360,6 +362,8 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 	}
 
 	if (vq_is_packed(dev)) {
+		if (vq->shadow_used_packed)
+			rte_free(vq->shadow_used_packed);
 		vq->shadow_used_packed = rte_malloc(NULL,
 				vq->size *
 				sizeof(struct vring_used_elem_packed),
@@ -371,6 +375,8 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 		}
 
 	} else {
+		if (vq->shadow_used_split)
+			rte_free(vq->shadow_used_split);
 		vq->shadow_used_split = rte_malloc(NULL,
 				vq->size * sizeof(struct vring_used_elem),
 				RTE_CACHE_LINE_SIZE);
@@ -381,6 +387,8 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 		}
 	}
 
+	if (vq->batch_copy_elems)
+		rte_free(vq->batch_copy_elems);
 	vq->batch_copy_elems = rte_malloc(NULL,
 				vq->size * sizeof(struct batch_copy_elem),
 				RTE_CACHE_LINE_SIZE);
-- 
2.21.0

[dpdk-dev] [master PATCH v2 2/2] vhost: fix possible denial of service by leaking FDs

[Maxime Coquelin]

A malicious Vhost-user master could send in loop hand-crafted
vhost-user messages containing more file descriptors the
vhost-user slave expects. Doing so causes the application using
the vhost-user library to run out of FDs.

This issue has been assigned CVE-2019-14818

Fixes: 8f972312b8f4 ("vhost: support vhost-user")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 119 ++++++++++++++++++++++++++++++++--
 1 file changed, 115 insertions(+), 4 deletions(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 6d2431e604..049e37c6de 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -83,6 +83,36 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg);
 static int read_vhost_message(int sockfd, struct VhostUserMsg *msg);
 
+static void
+close_msg_fds(struct VhostUserMsg *msg)
+{
+	int i;
+
+	for (i = 0; i < msg->fd_num; i++)
+		close(msg->fds[i]);
+}
+
+/*
+ * Ensure the expected number of FDs is received,
+ * close all FDs and return an error if this is not the case.
+ */
+static int
+validate_msg_fds(struct VhostUserMsg *msg, int expected_fds)
+{
+	if (msg->fd_num == expected_fds)
+		return 0;
+
+	RTE_LOG(ERR, VHOST_CONFIG,
+		" Expect %d FDs for request %s, received %d\n",
+		expected_fds,
+		vhost_message_str[msg->request.master],
+		msg->fd_num);
+
+	close_msg_fds(msg);
+
+	return -1;
+}
+
 static uint64_t
 get_blk_size(int fd)
 {
@@ -179,18 +209,25 @@ vhost_backend_cleanup(struct virtio_net *dev)
  */
 static int
 vhost_user_set_owner(struct virtio_net **pdev __rte_unused,
-			struct VhostUserMsg *msg __rte_unused,
+			struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	return RTE_VHOST_MSG_RESULT_OK;
 }
 
 static int
 vhost_user_reset_owner(struct virtio_net **pdev,
-			struct VhostUserMsg *msg __rte_unused,
+			struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
 	struct virtio_net *dev = *pdev;
+
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	vhost_destroy_device_notify(dev);
 
 	cleanup_device(dev, 0);
@@ -208,6 +245,9 @@ vhost_user_get_features(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct virtio_net *dev = *pdev;
 	uint64_t features = 0;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	rte_vhost_driver_get_features(dev->ifname, &features);
 
 	msg->payload.u64 = features;
@@ -227,6 +267,9 @@ vhost_user_get_queue_num(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct virtio_net *dev = *pdev;
 	uint32_t queue_num = 0;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	rte_vhost_driver_get_queue_num(dev->ifname, &queue_num);
 
 	msg->payload.u64 = (uint64_t)queue_num;
@@ -249,6 +292,9 @@ vhost_user_set_features(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct rte_vdpa_device *vdpa_dev;
 	int did = -1;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	rte_vhost_driver_get_features(dev->ifname, &vhost_features);
 	if (features & ~vhost_features) {
 		RTE_LOG(ERR, VHOST_CONFIG,
@@ -331,6 +377,9 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 	struct virtio_net *dev = *pdev;
 	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	vq->size = msg->payload.state.num;
 
 	/* VIRTIO 1.0, 2.4 Virtqueues says:
@@ -718,6 +767,9 @@ vhost_user_set_vring_addr(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct vhost_vring_addr *addr = &msg->payload.addr;
 	bool access_ok;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	if (dev->mem == NULL)
 		return RTE_VHOST_MSG_RESULT_ERR;
 
@@ -759,6 +811,9 @@ vhost_user_set_vring_base(struct virtio_net **pdev,
 	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
 	uint64_t val = msg->payload.state.num;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	if (vq_is_packed(dev)) {
 		/*
 		 * Bit[0:14]: avail index
@@ -920,6 +975,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	int populate;
 	int fd;
 
+	if (validate_msg_fds(msg, memory->nregions) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	if (memory->nregions > VHOST_MEMORY_MAX_NREGIONS) {
 		RTE_LOG(ERR, VHOST_CONFIG,
 			"too many memory regions (%u)\n", memory->nregions);
@@ -930,8 +988,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
 		RTE_LOG(INFO, VHOST_CONFIG,
 			"(%d) memory regions not changed\n", dev->vid);
 
-		for (i = 0; i < memory->nregions; i++)
-			close(msg->fds[i]);
+		close_msg_fds(msg);
 
 		return RTE_VHOST_MSG_RESULT_OK;
 	}
@@ -1074,6 +1131,10 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
 				"Failed to read qemu ack on postcopy set-mem-table\n");
 			goto err_mmap;
 		}
+
+		if (validate_msg_fds(&ack_msg, 0) != 0)
+			goto err_mmap;
+
 		if (ack_msg.request.master != VHOST_USER_SET_MEM_TABLE) {
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"Bad qemu ack on postcopy set-mem-table (%d)\n",
@@ -1194,6 +1255,9 @@ vhost_user_set_vring_call(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct vhost_vring_file file;
 	struct vhost_virtqueue *vq;
 
+	if (validate_msg_fds(msg, 1) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
 	if (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
 		file.fd = VIRTIO_INVALID_EVENTFD;
@@ -1215,6 +1279,9 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
 			struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
+	if (validate_msg_fds(msg, 1) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	if (!(msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK))
 		close(msg->fds[0]);
 	RTE_LOG(INFO, VHOST_CONFIG, "not implemented\n");
@@ -1230,6 +1297,9 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct vhost_vring_file file;
 	struct vhost_virtqueue *vq;
 
+	if (validate_msg_fds(msg, 1) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
 	if (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
 		file.fd = VIRTIO_INVALID_EVENTFD;
@@ -1286,6 +1356,9 @@ vhost_user_get_vring_base(struct virtio_net **pdev,
 	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
 	uint64_t val;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	/* We have to stop the queue (virtio) if it is running. */
 	vhost_destroy_device_notify(dev);
 
@@ -1361,6 +1434,9 @@ vhost_user_set_vring_enable(struct virtio_net **pdev,
 	struct rte_vdpa_device *vdpa_dev;
 	int did = -1;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	RTE_LOG(INFO, VHOST_CONFIG,
 		"set queue enable: %d to qp idx: %d\n",
 		enable, index);
@@ -1391,6 +1467,9 @@ vhost_user_get_protocol_features(struct virtio_net **pdev,
 	struct virtio_net *dev = *pdev;
 	uint64_t features, protocol_features;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	rte_vhost_driver_get_features(dev->ifname, &features);
 	rte_vhost_driver_get_protocol_features(dev->ifname, &protocol_features);
 
@@ -1419,6 +1498,9 @@ vhost_user_set_protocol_features(struct virtio_net **pdev,
 	uint64_t protocol_features = msg->payload.u64;
 	uint64_t slave_protocol_features = 0;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	rte_vhost_driver_get_protocol_features(dev->ifname,
 			&slave_protocol_features);
 	if (protocol_features & ~slave_protocol_features) {
@@ -1445,6 +1527,9 @@ vhost_user_set_log_base(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	uint64_t size, off;
 	void *addr;
 
+	if (validate_msg_fds(msg, 1) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	if (fd < 0) {
 		RTE_LOG(ERR, VHOST_CONFIG, "invalid log fd: %d\n", fd);
 		return RTE_VHOST_MSG_RESULT_ERR;
@@ -1508,6 +1593,9 @@ static int vhost_user_set_log_fd(struct virtio_net **pdev __rte_unused,
 			struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
+	if (validate_msg_fds(msg, 1) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	close(msg->fds[0]);
 	RTE_LOG(INFO, VHOST_CONFIG, "not implemented.\n");
 
@@ -1531,6 +1619,9 @@ vhost_user_send_rarp(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct rte_vdpa_device *vdpa_dev;
 	int did = -1;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	RTE_LOG(DEBUG, VHOST_CONFIG,
 		":: mac: %02x:%02x:%02x:%02x:%02x:%02x\n",
 		mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
@@ -1558,6 +1649,10 @@ vhost_user_net_set_mtu(struct virtio_net **pdev, struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
 	struct virtio_net *dev = *pdev;
+
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	if (msg->payload.u64 < VIRTIO_MIN_MTU ||
 			msg->payload.u64 > VIRTIO_MAX_MTU) {
 		RTE_LOG(ERR, VHOST_CONFIG, "Invalid MTU size (%"PRIu64")\n",
@@ -1578,6 +1673,9 @@ vhost_user_set_req_fd(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct virtio_net *dev = *pdev;
 	int fd = msg->fds[0];
 
+	if (validate_msg_fds(msg, 1) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	if (fd < 0) {
 		RTE_LOG(ERR, VHOST_CONFIG,
 				"Invalid file descriptor for slave channel (%d)\n",
@@ -1663,6 +1761,9 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	uint16_t i;
 	uint64_t vva, len;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	switch (imsg->type) {
 	case VHOST_IOTLB_UPDATE:
 		len = imsg->size;
@@ -1709,6 +1810,9 @@ vhost_user_set_postcopy_advise(struct virtio_net **pdev,
 #ifdef RTE_LIBRTE_VHOST_POSTCOPY
 	struct uffdio_api api_struct;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
 
 	if (dev->postcopy_ufd == -1) {
@@ -1744,6 +1848,9 @@ vhost_user_set_postcopy_listen(struct virtio_net **pdev,
 {
 	struct virtio_net *dev = *pdev;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	if (dev->mem && dev->mem->nregions) {
 		RTE_LOG(ERR, VHOST_CONFIG,
 			"Regions already registered at postcopy-listen\n");
@@ -1760,6 +1867,9 @@ vhost_user_postcopy_end(struct virtio_net **pdev, struct VhostUserMsg *msg,
 {
 	struct virtio_net *dev = *pdev;
 
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
 	dev->postcopy_listening = 0;
 	if (dev->postcopy_ufd >= 0) {
 		close(dev->postcopy_ufd);
@@ -2112,6 +2222,7 @@ vhost_user_msg_handler(int vid, int fd)
 	if (!handled) {
 		RTE_LOG(ERR, VHOST_CONFIG,
 			"vhost message (req: %d) was not handled.\n", request);
+		close_msg_fds(&msg);
 		ret = RTE_VHOST_MSG_RESULT_ERR;
 	}
 
-- 
2.21.0

Re: [dpdk-dev] [master PATCH v2 1/2] vhost: fix possible denial of service on SET_VRING_NUM

[David Marchand]

On 12/11/2019 16:19, Maxime Coquelin wrote:
> vhost_user_set_vring_num() performs multiple allocations
> without checking whether data were previously allocated.
> 
> It may cause a denial of service because of the memory leaks
> that happen if a malicious vhost-user master keeps sending
> VHOST_USER_SET_VRING_NUM request until the slave runs out
> of memory.
> 
> This issue has been assigned CVE-2019-14818
> 
> Fixes: b0a985d1f340 ("vhost: add dequeue zero copy")
> 
> Reported-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Applied, thanks.


-- 
David Marchand

Re: [dpdk-dev] [master PATCH v2 2/2] vhost: fix possible denial of service by leaking FDs

[David Marchand]

On 12/11/2019 16:19, Maxime Coquelin wrote:
> A malicious Vhost-user master could send in loop hand-crafted
> vhost-user messages containing more file descriptors the
> vhost-user slave expects. Doing so causes the application using
> the vhost-user library to run out of FDs.
> 
> This issue has been assigned CVE-2019-14818
> 
> Fixes: 8f972312b8f4 ("vhost: support vhost-user")
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Applied, thanks.


-- 
David Marchand

Re: [dpdk-dev] [dpdk-stable] [v18.11 PATCH v2 1/2] vhost: fix possible denial of service on SET_VRING_NUM

[Kevin Traynor]

On 12/11/2019 15:19, Maxime Coquelin wrote:
> vhost_user_set_vring_num() performs multiple allocations
> without checking whether data were previously allocated.
> 
> It may cause a denial of service because of the memory leaks
> that happen if a malicious vhost-user master keeps sending
> VHOST_USER_SET_VRING_NUM request until the slave runs out
> of memory.
> 
> This issue has been assigned CVE-2019-14818
> 
> Reported-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---

Applied, thanks.

Re: [dpdk-dev] [dpdk-stable] [v18.11 PATCH v2 2/2] vhost: fix possible denial of service by leaking FDs

[Kevin Traynor]

On 12/11/2019 15:19, Maxime Coquelin wrote:
> A malicious Vhost-user master could send in loop hand-crafted
> vhost-user messages containing more file descriptors the
> vhost-user slave expects. Doing so causes the application using
> the vhost-user library to run out of FDs.
> 
> This issue has been assigned CVE-2019-14818
> 
> Fixes: 8f972312b8f4 ("vhost: support vhost-user")
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---

Applied, thanks.

Re: [dpdk-dev] DPDK security advisory: CVE-2019-14818

[Ferruh Yigit]

On 11/12/2019 3:15 PM, Ferruh Yigit wrote:
> A vulnerability was fixed in DPDK.
> 
> Some downstream stakeholders were warned in advance in order to coordinate the
> release of fixes and reduce the vulnerability window.
> 
> Problem:
> A malicious container which has direct access to the vhost-user socket can keep
> sending messages which may cause leaking resources until resulting a DOS.
> 
> All users of the vhost library are strongly encouraged to upgrade as soon as
> possible.
> 
> CVE-2019-14818
> Bugzilla: https://bugs.dpdk.org/show_bug.cgi?id=363
> Severity: Medium
> CVSS scores: 6.8

And thanks to the "Jason Wang" [1] for reporting the vulnerability, all credits
for discovering the issue goes to him.

[1]
Jason Wang <jasowang@redhat.com>

Re: [dpdk-dev] [dpdk-security] DPDK security advisory: CVE-2019-14818

[Ferruh Yigit]

On 11/12/2019 3:15 PM, Ferruh Yigit wrote:
> A vulnerability was fixed in DPDK.
> 
> Some downstream stakeholders were warned in advance in order to coordinate the
> release of fixes and reduce the vulnerability window.
> 
> Problem:
> A malicious container which has direct access to the vhost-user socket can keep
> sending messages which may cause leaking resources until resulting a DOS.
> 
> All users of the vhost library are strongly encouraged to upgrade as soon as
> possible.
> 
> CVE-2019-14818
> Bugzilla: https://bugs.dpdk.org/show_bug.cgi?id=363
> Severity: Medium
> CVSS scores: 6.8
> 
> 
> 
> Commits:
> main repo
> https://git.dpdk.org/dpdk/commit/?id=612e17cf6d7b
> https://git.dpdk.org/dpdk/commit/?id=bf472259dde6
> 
> 19.08.1
> https://git.dpdk.org/dpdk-stable/commit/?h=19.08&id=fa674d08985f
> https://git.dpdk.org/dpdk-stable/commit/?h=19.08&id=6547dd563ea9
> 
> 18.11.4 (LTS)
> https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=70583a6b9b1c
> https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=f8898927bb16
> 
> 17.11.8 (LTS)
> https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=3b1b44a1c82a
> https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=8a8dbd0ec19e
> https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=1f6147d9a01f
> 
> 16.11.10 (LTS EOL)
> https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=5fbb5c2919b6
> https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=3863340f93b8
> https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=8790f4c3bcd2
> https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=1bf11cfb7c7c
> 

A regression has been found on the above commits when VHOST_USER_VRING_NOFD_MASK
is set, there is a suggested fix [1], review and testing is going on.
We are planning to have an update tomorrow.

Sorry for the inconvenience caused.

[1]
https://patches.dpdk.org/patch/62956/

Re: [dpdk-dev] [dpdk-security] DPDK security advisory: CVE-2019-14818

[Ferruh Yigit]

On 11/14/2019 11:25 AM, Ferruh Yigit wrote:
> On 11/12/2019 3:15 PM, Ferruh Yigit wrote:
>> A vulnerability was fixed in DPDK.
>>
>> Some downstream stakeholders were warned in advance in order to coordinate the
>> release of fixes and reduce the vulnerability window.
>>
>> Problem:
>> A malicious container which has direct access to the vhost-user socket can keep
>> sending messages which may cause leaking resources until resulting a DOS.
>>
>> All users of the vhost library are strongly encouraged to upgrade as soon as
>> possible.
>>
>> CVE-2019-14818
>> Bugzilla: https://bugs.dpdk.org/show_bug.cgi?id=363
>> Severity: Medium
>> CVSS scores: 6.8
>>

<...>

>>
> 
> A regression has been found on the above commits when VHOST_USER_VRING_NOFD_MASK
> is set, there is a suggested fix [1], review and testing is going on.
> We are planning to have an update tomorrow.
> 
> Sorry for the inconvenience caused.
> 
> [1]
> https://patches.dpdk.org/patch/62956/
> 

Regression has been solved, please find new stable release links and updated
commits.


Commits:
main repo
https://git.dpdk.org/dpdk/commit/?id=612e17cf6d7b
https://git.dpdk.org/dpdk/commit/?id=bf472259dde6
https://git.dpdk.org/dpdk/commit/?id=1407b0752eee

19.08.2
https://git.dpdk.org/dpdk-stable/commit/?h=19.08&id=fa674d08985f
https://git.dpdk.org/dpdk-stable/commit/?h=19.08&id=6547dd563ea9
https://git.dpdk.org/dpdk-stable/commit/?h=19.08&id=7ce55a8e4f4d

18.11.5 (LTS)
https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=70583a6b9b1c
https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=f8898927bb16
https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=afc8c11865ef

17.11.9 (LTS)
https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=3b1b44a1c82a
https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=8a8dbd0ec19e
https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=1f6147d9a01f
https://git.dpdk.org/dpdk-stable/commit/?h=17.11&id=89ce028931ef

16.11.11 (LTS EOL)
https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=5fbb5c2919b6
https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=3863340f93b8
https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=8790f4c3bcd2
https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=1bf11cfb7c7c
https://git.dpdk.org/dpdk-stable/commit/?h=16.11&id=25b8ea4d8604


Stable Releases download links:
DPDK 19.08.2
http://fast.dpdk.org/rel/dpdk-19.08.2.tar.xz

DPDK 18.11.5 (LTS)
http://fast.dpdk.org/rel/dpdk-18.11.5.tar.xz

DPDK 17.11.9 (LTS)
http://fast.dpdk.org/rel/dpdk-17.11.9.tar.xz

DPDK 16.11.11 (LTS EOL)
http://fast.dpdk.org/rel/dpdk-16.11.11.tar.xz

-- 
DPDK Security Team
http://core.dpdk.org/security/