From: Maxime Coquelin <maxime.coquelin@redhat.com> Marvin Liu (1): vhost: fix translated address not checked Maxime Coquelin (3): vhost: check log mmap offset and size overflow vhost: fix vring index check vhost/crypto: validate keys lengths Xiaolong Ye (1): vhost: fix potential memory space leak Xuan Ding (1): vhost: fix potential fd leak lib/librte_vhost/vhost_crypto.c | 17 +++++++++++++++++ lib/librte_vhost/vhost_user.c | 30 +++++++++++++++++++++++------- lib/librte_vhost/virtio_net.c | 4 ++++ 3 files changed, 44 insertions(+), 7 deletions(-) -- 2.25.2
From: Maxime Coquelin <maxime.coquelin@redhat.com> vhost_user_set_log_base() is a message handler that is called to handle the VHOST_USER_SET_LOG_BASE message. Its payload contains a 64 bit size and offset. Both are added up and used as a size when calling mmap(). There is no integer overflow check. If an integer overflow occurs a smaller memory map would be created than requested. Since the returned mapping is mapped as writable and used for logging, a memory corruption could occur. Fixes: fbc4d248b198 ("vhost: fix offset while mmaping log base address") Cc: stable@dpdk.org This issue has been assigned CVE-2020-10722 Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com> Reviewed-by: Ilja Van Sprundel <ivansprundel@ioactive.com> --- lib/librte_vhost/vhost_user.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 40c4520c08..02962fcdbc 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -2059,10 +2059,10 @@ vhost_user_set_log_base(struct virtio_net **pdev, struct VhostUserMsg *msg, size = msg->payload.log.mmap_size; off = msg->payload.log.mmap_offset; - /* Don't allow mmap_offset to point outside the mmap region */ - if (off > size) { + /* Check for mmap size and offset overflow. */ + if (off >= -size) { RTE_LOG(ERR, VHOST_CONFIG, - "log offset %#"PRIx64" exceeds log size %#"PRIx64"\n", + "log offset %#"PRIx64" and log size %#"PRIx64" overflow\n", off, size); return RTE_VHOST_MSG_RESULT_ERR; } -- 2.25.2
From: Maxime Coquelin <maxime.coquelin@redhat.com> vhost_user_check_and_alloc_queue_pair() is used to extract a vring index from a payload. This function validates the index and is called early on in when performing message handling. Most message handlers depend on it correctly validating the vring index. Depending on the message type the vring index is in different parts of the payload. The function contains a switch/case for each type and copies the index. This is stored in a uint16. This index is then validated. Depending on the message, the source index is an unsigned int. If integer truncation occurs (uint->uint16) the top 16 bits of the index are never validated. When they are used later on (e.g. in vhost_user_set_vring_num() or vhost_user_set_vring_addr()) it can lead to out of bound indexing. The out of bound indexed data gets written to, and hence this can cause memory corruption. This patch fixes this vulnerability by declaring vring index as an unsigned int in vhost_user_check_and_alloc_queue_pair(). Fixes: 160cbc815b41 ("vhost: remove a hack on queue allocation") Cc: stable@dpdk.org This issue has been assigned CVE-2020-10723 Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com> Reviewed-by: Ilja Van Sprundel <ivansprundel@ioactive.com> --- lib/librte_vhost/vhost_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 02962fcdbc..d19614265b 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -2526,7 +2526,7 @@ static int vhost_user_check_and_alloc_queue_pair(struct virtio_net *dev, struct VhostUserMsg *msg) { - uint16_t vring_idx; + uint32_t vring_idx; switch (msg->request.master) { case VHOST_USER_SET_VRING_KICK: -- 2.25.2
From: Maxime Coquelin <maxime.coquelin@redhat.com> transform_cipher_param() and transform_chain_param() handle the payload data for the VHOST_USER_CRYPTO_CREATE_SESS message. These payloads have to be validated, since it could come from untrusted sources. Two buffers and their lenghts are defined in this payload, one the the auth key and one for the cipher key. But above functions do not validate the key length inputs, which could lead to read out of bounds, as buffers have static sizes of 64 bytes for the cipher key and 512 bytes for the auth key. This patch adds necessary checks on the key length field before being used. Fixes: e80a98708166 ("vhost/crypto: add session message handler") Cc: stable@dpdk.org This issue has been assigned CVE-2020-10724 Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com> Reviewed-by: Ilja Van Sprundel <ivansprundel@ioactive.com> --- lib/librte_vhost/vhost_crypto.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c index 68911972b6..07a4115482 100644 --- a/lib/librte_vhost/vhost_crypto.c +++ b/lib/librte_vhost/vhost_crypto.c @@ -237,6 +237,11 @@ transform_cipher_param(struct rte_crypto_sym_xform *xform, if (unlikely(ret < 0)) return ret; + if (param->cipher_key_len > VHOST_USER_CRYPTO_MAX_CIPHER_KEY_LENGTH) { + VC_LOG_DBG("Invalid cipher key length\n"); + return -VIRTIO_CRYPTO_BADMSG; + } + xform->type = RTE_CRYPTO_SYM_XFORM_CIPHER; xform->cipher.key.length = param->cipher_key_len; if (xform->cipher.key.length > 0) @@ -287,6 +292,12 @@ transform_chain_param(struct rte_crypto_sym_xform *xforms, &xform_cipher->cipher.algo); if (unlikely(ret < 0)) return ret; + + if (param->cipher_key_len > VHOST_USER_CRYPTO_MAX_CIPHER_KEY_LENGTH) { + VC_LOG_DBG("Invalid cipher key length\n"); + return -VIRTIO_CRYPTO_BADMSG; + } + xform_cipher->type = RTE_CRYPTO_SYM_XFORM_CIPHER; xform_cipher->cipher.key.length = param->cipher_key_len; xform_cipher->cipher.key.data = param->cipher_key_buf; @@ -301,6 +312,12 @@ transform_chain_param(struct rte_crypto_sym_xform *xforms, ret = auth_algo_transform(param->hash_algo, &xform_auth->auth.algo); if (unlikely(ret < 0)) return ret; + + if (param->auth_key_len > VHOST_USER_CRYPTO_MAX_HMAC_KEY_LENGTH) { + VC_LOG_DBG("Invalid auth key length\n"); + return -VIRTIO_CRYPTO_BADMSG; + } + xform_auth->auth.digest_length = param->digest_len; xform_auth->auth.key.length = param->auth_key_len; xform_auth->auth.key.data = param->auth_key_buf; -- 2.25.2
From: Marvin Liu <yong.liu@intel.com> Malicious guest can construct desc with invalid address and zero buffer length. That will request vhost to check both translated address and translated data length. This patch will add missed address check. Fixes: 75ed51697820 ("vhost: add packed ring batch dequeue") Fixes: ef861692c398 ("vhost: add packed ring batch enqueue") Cc: stable@dpdk.org This issue has been assigned CVE-2020-10725 Signed-off-by: Marvin Liu <yong.liu@intel.com> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- lib/librte_vhost/virtio_net.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index ac2842b2d2..33f10258cf 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -1086,6 +1086,8 @@ virtio_dev_rx_batch_packed(struct virtio_net *dev, VHOST_ACCESS_RW); vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) { + if (unlikely(!desc_addrs[i])) + return -1; if (unlikely(lens[i] != descs[avail_idx + i].len)) return -1; } @@ -1841,6 +1843,8 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, } vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) { + if (unlikely(!desc_addrs[i])) + return -1; if (unlikely((lens[i] != descs[avail_idx + i].len))) return -1; } -- 2.25.2
From: Xiaolong Ye <xiaolong.ye@intel.com> A malicious container which has direct access to the vhost-user socket can keep sending VHOST_USER_GET_INFLIGHT_FD messages which may cause leaking resources until resulting a DOS. Fix it by unmapping the dev->inflight_info->addr before assigning new mapped addr to it. Fixes: d87f1a1cb7b6 ("vhost: support inflight info sharing") Cc: stable@dpdk.org This issue has been assigned CVE-2020-10726 Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- lib/librte_vhost/vhost_user.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index d19614265b..2a4ba205cf 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -1433,6 +1433,11 @@ vhost_user_get_inflight_fd(struct virtio_net **pdev, } memset(addr, 0, mmap_size); + if (dev->inflight_info->addr) { + munmap(dev->inflight_info->addr, dev->inflight_info->size); + dev->inflight_info->addr = NULL; + } + dev->inflight_info->addr = addr; dev->inflight_info->size = msg->payload.inflight.mmap_size = mmap_size; dev->inflight_info->fd = msg->fds[0] = fd; @@ -1517,8 +1522,10 @@ vhost_user_set_inflight_fd(struct virtio_net **pdev, VhostUserMsg *msg, } } - if (dev->inflight_info->addr) + if (dev->inflight_info->addr) { munmap(dev->inflight_info->addr, dev->inflight_info->size); + dev->inflight_info->addr = NULL; + } addr = mmap(0, mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, mmap_offset); -- 2.25.2
From: Xuan Ding <xuan.ding@intel.com> Vhost will create temporary file when receiving VHOST_USER_GET_INFLIGHT_FD message. Malicious guest can send endless this message to drain out the resource of host. When receiving VHOST_USER_GET_INFLIGHT_FD message repeatedly, closing the file created during the last handling of this message. Fixes: d87f1a1cb7b666550 ("vhost: support inflight info sharing") Cc: stable@dpdk.org This issue has been assigned CVE-2020-10726 Signed-off-by: Xuan Ding <xuan.ding@intel.com> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- lib/librte_vhost/vhost_user.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 2a4ba205cf..8954f7930e 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -206,7 +206,7 @@ vhost_backend_cleanup(struct virtio_net *dev) dev->inflight_info->addr = NULL; } - if (dev->inflight_info->fd > 0) { + if (dev->inflight_info->fd >= 0) { close(dev->inflight_info->fd); dev->inflight_info->fd = -1; } @@ -1408,6 +1408,7 @@ vhost_user_get_inflight_fd(struct virtio_net **pdev, "failed to alloc dev inflight area\n"); return RTE_VHOST_MSG_RESULT_ERR; } + dev->inflight_info->fd = -1; } num_queues = msg->payload.inflight.num_queues; @@ -1438,6 +1439,11 @@ vhost_user_get_inflight_fd(struct virtio_net **pdev, dev->inflight_info->addr = NULL; } + if (dev->inflight_info->fd >= 0) { + close(dev->inflight_info->fd); + dev->inflight_info->fd = -1; + } + dev->inflight_info->addr = addr; dev->inflight_info->size = msg->payload.inflight.mmap_size = mmap_size; dev->inflight_info->fd = msg->fds[0] = fd; @@ -1520,6 +1526,7 @@ vhost_user_set_inflight_fd(struct virtio_net **pdev, VhostUserMsg *msg, "failed to alloc dev inflight area\n"); return RTE_VHOST_MSG_RESULT_ERR; } + dev->inflight_info->fd = -1; } if (dev->inflight_info->addr) { @@ -1534,8 +1541,10 @@ vhost_user_set_inflight_fd(struct virtio_net **pdev, VhostUserMsg *msg, return RTE_VHOST_MSG_RESULT_ERR; } - if (dev->inflight_info->fd) + if (dev->inflight_info->fd >= 0) { close(dev->inflight_info->fd); + dev->inflight_info->fd = -1; + } dev->inflight_info->fd = fd; dev->inflight_info->addr = addr; -- 2.25.2