DPDK patches and discussions
 help / color / mirror / Atom feed
From: Fan Zhang <roy.fan.zhang@intel.com>
To: dev@dpdk.org
Cc: maxime.coquelin@redhat.com, tiwei.bie@intel.com, stable@dpdk.org
Subject: [dpdk-dev] [PATCH 2/2] vhost/crypto: fix possible out of bound access
Date: Fri,  4 Jan 2019 11:22:46 +0000	[thread overview]
Message-ID: <20190104112246.9338-3-roy.fan.zhang@intel.com> (raw)
In-Reply-To: <20190104112246.9338-1-roy.fan.zhang@intel.com>

This patch fixes a out of bound access possbility in vhost
crypto. Originally the incorrect next descriptor index may
cause the library read invalid memory content and crash
the application.

Cc: stable@dpdk.org
Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 lib/librte_vhost/vhost_crypto.c | 89 ++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 36 deletions(-)

diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index 80b83ef77..0694c0a74 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -467,13 +467,13 @@ vhost_crypto_msg_post_handler(int vid, void *msg)
 
 static __rte_always_inline struct vring_desc *
 find_write_desc(struct vring_desc *head, struct vring_desc *desc,
-		uint32_t *nb_descs)
+		uint32_t *nb_descs, uint32_t vq_size)
 {
 	if (desc->flags & VRING_DESC_F_WRITE)
 		return desc;
 
 	while (desc->flags & VRING_DESC_F_NEXT) {
-		if (unlikely(*nb_descs == 0))
+		if (unlikely(*nb_descs == 0 || desc->next >= vq_size))
 			return NULL;
 		(*nb_descs)--;
 
@@ -487,13 +487,13 @@ find_write_desc(struct vring_desc *head, struct vring_desc *desc,
 
 static struct virtio_crypto_inhdr *
 reach_inhdr(struct vhost_crypto_data_req *vc_req, struct vring_desc *desc,
-		uint32_t *nb_descs)
+		uint32_t *nb_descs, uint32_t vq_size)
 {
 	uint64_t dlen;
 	struct virtio_crypto_inhdr *inhdr;
 
 	while (desc->flags & VRING_DESC_F_NEXT) {
-		if (unlikely(*nb_descs == 0))
+		if (unlikely(*nb_descs == 0 || desc->next >= vq_size))
 			return NULL;
 		(*nb_descs)--;
 		desc = &vc_req->head[desc->next];
@@ -510,14 +510,14 @@ reach_inhdr(struct vhost_crypto_data_req *vc_req, struct vring_desc *desc,
 
 static __rte_always_inline int
 move_desc(struct vring_desc *head, struct vring_desc **cur_desc,
-		uint32_t size, uint32_t *nb_descs)
+		uint32_t size, uint32_t *nb_descs, uint32_t vq_size)
 {
 	struct vring_desc *desc = *cur_desc;
 	int left = size - desc->len;
 
 	while ((desc->flags & VRING_DESC_F_NEXT) && left > 0) {
 		(*nb_descs)--;
-		if (unlikely(*nb_descs == 0))
+		if (unlikely(*nb_descs == 0 || desc->next >= vq_size))
 			return -1;
 
 		desc = &head[desc->next];
@@ -530,8 +530,12 @@ move_desc(struct vring_desc *head, struct vring_desc **cur_desc,
 
 	if (unlikely(*nb_descs == 0))
 		*cur_desc = NULL;
-	else
+	else {
+		if (unlikely(desc->next >= vq_size))
+			return -1;
 		*cur_desc = &head[desc->next];
+	}
+
 	return 0;
 }
 
@@ -553,7 +557,8 @@ get_data_ptr(struct vhost_crypto_data_req *vc_req, struct vring_desc *cur_desc,
 
 static int
 copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
-		struct vring_desc **cur_desc, uint32_t size, uint32_t *nb_descs)
+		struct vring_desc **cur_desc, uint32_t size,
+		uint32_t *nb_descs, uint32_t vq_size)
 {
 	struct vring_desc *desc = *cur_desc;
 	uint64_t remain, addr, dlen, len;
@@ -595,7 +600,7 @@ copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
 	left -= to_copy;
 
 	while ((desc->flags & VRING_DESC_F_NEXT) && left > 0) {
-		if (unlikely(*nb_descs == 0)) {
+		if (unlikely(*nb_descs == 0 || desc->next >= vq_size)) {
 			VC_LOG_ERR("Invalid descriptors");
 			return -1;
 		}
@@ -645,8 +650,11 @@ copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
 
 	if (unlikely(*nb_descs == 0))
 		*cur_desc = NULL;
-	else
+	else {
+		if (unlikely(desc->next >= vq_size))
+			return -1;
 		*cur_desc = &vc_req->head[desc->next];
+	}
 
 	return 0;
 }
@@ -657,7 +665,6 @@ write_back_data(struct vhost_crypto_data_req *vc_req)
 	struct vhost_crypto_writeback_data *wb_data = vc_req->wb, *wb_last;
 
 	while (wb_data) {
-		rte_prefetch0(wb_data->next);
 		rte_memcpy(wb_data->dst, wb_data->src, wb_data->len);
 		wb_last = wb_data;
 		wb_data = wb_data->next;
@@ -707,7 +714,7 @@ prepare_write_back_data(struct vhost_crypto_data_req *vc_req,
 		uint8_t *src,
 		uint32_t offset,
 		uint64_t write_back_len,
-		uint32_t *nb_descs)
+		uint32_t *nb_descs, uint32_t vq_size)
 {
 	struct vhost_crypto_writeback_data *wb_data, *head;
 	struct vring_desc *desc = *cur_desc;
@@ -754,7 +761,7 @@ prepare_write_back_data(struct vhost_crypto_data_req *vc_req,
 		offset -= desc->len;
 
 	while (write_back_len) {
-		if (unlikely(*nb_descs == 0)) {
+		if (unlikely(*nb_descs == 0 || desc->next >= vq_size)) {
 			VC_LOG_ERR("Invalid descriptors");
 			goto error_exit;
 		}
@@ -801,8 +808,11 @@ prepare_write_back_data(struct vhost_crypto_data_req *vc_req,
 
 	if (unlikely(*nb_descs == 0))
 		*cur_desc = NULL;
-	else
+	else {
+		if (unlikely(desc->next >= vq_size))
+			goto error_exit;
 		*cur_desc = &vc_req->head[desc->next];
+	}
 
 	*end_wb_data = wb_data;
 
@@ -820,7 +830,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		struct vhost_crypto_data_req *vc_req,
 		struct virtio_crypto_cipher_data_req *cipher,
 		struct vring_desc *cur_desc,
-		uint32_t *nb_descs)
+		uint32_t *nb_descs, uint32_t vq_size)
 {
 	struct vring_desc *desc = cur_desc;
 	struct vhost_crypto_writeback_data *ewb = NULL;
@@ -831,7 +841,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 	/* prepare */
 	/* iv */
 	if (unlikely(copy_data(iv_data, vc_req, &desc, cipher->para.iv_len,
-			nb_descs) < 0)) {
+			nb_descs, vq_size) < 0)) {
 		ret = VIRTIO_CRYPTO_BADMSG;
 		goto error_exit;
 	}
@@ -851,7 +861,8 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		}
 
 		if (unlikely(move_desc(vc_req->head, &desc,
-				cipher->para.src_data_len, nb_descs) < 0)) {
+				cipher->para.src_data_len, nb_descs,
+				vq_size) < 0)) {
 			VC_LOG_ERR("Incorrect descriptor");
 			ret = VIRTIO_CRYPTO_ERR;
 			goto error_exit;
@@ -869,7 +880,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		}
 		if (unlikely(copy_data(rte_pktmbuf_mtod(m_src, uint8_t *),
 				vc_req, &desc, cipher->para.src_data_len,
-				nb_descs) < 0)) {
+				nb_descs, vq_size) < 0)) {
 			ret = VIRTIO_CRYPTO_BADMSG;
 			goto error_exit;
 		}
@@ -880,7 +891,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 	}
 
 	/* dst */
-	desc = find_write_desc(vc_req->head, desc, nb_descs);
+	desc = find_write_desc(vc_req->head, desc, nb_descs, vq_size);
 	if (unlikely(!desc)) {
 		VC_LOG_ERR("Cannot find write location");
 		ret = VIRTIO_CRYPTO_BADMSG;
@@ -899,7 +910,8 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		}
 
 		if (unlikely(move_desc(vc_req->head, &desc,
-				cipher->para.dst_data_len, nb_descs) < 0)) {
+				cipher->para.dst_data_len,
+				nb_descs, vq_size) < 0)) {
 			VC_LOG_ERR("Incorrect descriptor");
 			ret = VIRTIO_CRYPTO_ERR;
 			goto error_exit;
@@ -910,7 +922,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 	case RTE_VHOST_CRYPTO_ZERO_COPY_DISABLE:
 		vc_req->wb = prepare_write_back_data(vc_req, &desc, &ewb,
 				rte_pktmbuf_mtod(m_src, uint8_t *), 0,
-				cipher->para.dst_data_len, nb_descs);
+				cipher->para.dst_data_len, nb_descs, vq_size);
 		if (unlikely(vc_req->wb == NULL)) {
 			ret = VIRTIO_CRYPTO_ERR;
 			goto error_exit;
@@ -953,7 +965,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		struct vhost_crypto_data_req *vc_req,
 		struct virtio_crypto_alg_chain_data_req *chain,
 		struct vring_desc *cur_desc,
-		uint32_t *nb_descs)
+		uint32_t *nb_descs, uint32_t vq_size)
 {
 	struct vring_desc *desc = cur_desc, *digest_desc;
 	struct vhost_crypto_writeback_data *ewb = NULL, *ewb2 = NULL;
@@ -966,7 +978,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 	/* prepare */
 	/* iv */
 	if (unlikely(copy_data(iv_data, vc_req, &desc,
-			chain->para.iv_len, nb_descs) < 0)) {
+			chain->para.iv_len, nb_descs, vq_size) < 0)) {
 		ret = VIRTIO_CRYPTO_BADMSG;
 		goto error_exit;
 	}
@@ -987,7 +999,8 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		}
 
 		if (unlikely(move_desc(vc_req->head, &desc,
-				chain->para.src_data_len, nb_descs) < 0)) {
+				chain->para.src_data_len,
+				nb_descs, vq_size) < 0)) {
 			VC_LOG_ERR("Incorrect descriptor");
 			ret = VIRTIO_CRYPTO_ERR;
 			goto error_exit;
@@ -1004,7 +1017,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		}
 		if (unlikely(copy_data(rte_pktmbuf_mtod(m_src, uint8_t *),
 				vc_req, &desc, chain->para.src_data_len,
-				nb_descs)) < 0) {
+				nb_descs, vq_size)) < 0) {
 			ret = VIRTIO_CRYPTO_BADMSG;
 			goto error_exit;
 		}
@@ -1016,7 +1029,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 	}
 
 	/* dst */
-	desc = find_write_desc(vc_req->head, desc, nb_descs);
+	desc = find_write_desc(vc_req->head, desc, nb_descs, vq_size);
 	if (unlikely(!desc)) {
 		VC_LOG_ERR("Cannot find write location");
 		ret = VIRTIO_CRYPTO_BADMSG;
@@ -1035,7 +1048,8 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		}
 
 		if (unlikely(move_desc(vc_req->head, &desc,
-				chain->para.dst_data_len, nb_descs) < 0)) {
+				chain->para.dst_data_len,
+				nb_descs, vq_size) < 0)) {
 			VC_LOG_ERR("Incorrect descriptor");
 			ret = VIRTIO_CRYPTO_ERR;
 			goto error_exit;
@@ -1052,7 +1066,8 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		}
 
 		if (unlikely(move_desc(vc_req->head, &desc,
-				chain->para.hash_result_len, nb_descs) < 0)) {
+				chain->para.hash_result_len,
+				nb_descs, vq_size) < 0)) {
 			VC_LOG_ERR("Incorrect descriptor");
 			ret = VIRTIO_CRYPTO_ERR;
 			goto error_exit;
@@ -1064,7 +1079,8 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 				rte_pktmbuf_mtod(m_src, uint8_t *),
 				chain->para.cipher_start_src_offset,
 				chain->para.dst_data_len -
-				chain->para.cipher_start_src_offset, nb_descs);
+				chain->para.cipher_start_src_offset,
+				nb_descs, vq_size);
 		if (unlikely(vc_req->wb == NULL)) {
 			ret = VIRTIO_CRYPTO_ERR;
 			goto error_exit;
@@ -1078,14 +1094,15 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		/** create a wb_data for digest */
 		ewb->next = prepare_write_back_data(vc_req, &desc, &ewb2,
 				digest_addr, 0, chain->para.hash_result_len,
-				nb_descs);
+				nb_descs, vq_size);
 		if (unlikely(ewb->next == NULL)) {
 			ret = VIRTIO_CRYPTO_ERR;
 			goto error_exit;
 		}
 
 		if (unlikely(copy_data(digest_addr, vc_req, &digest_desc,
-				chain->para.hash_result_len, nb_descs)) < 0) {
+				chain->para.hash_result_len,
+				nb_descs, vq_size)) < 0) {
 			ret = VIRTIO_CRYPTO_BADMSG;
 			goto error_exit;
 		}
@@ -1180,7 +1197,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
 		case RTE_VHOST_CRYPTO_ZERO_COPY_DISABLE:
 			req = &tmp_req;
 			if (unlikely(copy_data(req, vc_req, &desc, sizeof(*req),
-					&nb_descs) < 0)) {
+					&nb_descs, vq->size) < 0)) {
 				err = VIRTIO_CRYPTO_BADMSG;
 				VC_LOG_ERR("Invalid descriptor");
 				goto error_exit;
@@ -1193,7 +1210,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
 		}
 	} else {
 		if (unlikely(move_desc(vc_req->head, &desc,
-				sizeof(*req), &nb_descs) < 0)) {
+				sizeof(*req), &nb_descs, vq->size) < 0)) {
 			VC_LOG_ERR("Incorrect descriptor");
 			goto error_exit;
 		}
@@ -1235,12 +1252,12 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
 		case VIRTIO_CRYPTO_SYM_OP_CIPHER:
 			err = prepare_sym_cipher_op(vcrypto, op, vc_req,
 					&req->u.sym_req.u.cipher, desc,
-					&nb_descs);
+					&nb_descs, vq->size);
 			break;
 		case VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING:
 			err = prepare_sym_chain_op(vcrypto, op, vc_req,
 					&req->u.sym_req.u.chain, desc,
-					&nb_descs);
+					&nb_descs, vq->size);
 			break;
 		}
 		if (unlikely(err != 0)) {
@@ -1258,7 +1275,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
 
 error_exit:
 
-	inhdr = reach_inhdr(vc_req, desc, &nb_descs);
+	inhdr = reach_inhdr(vc_req, desc, &nb_descs, vq->size);
 	if (likely(inhdr != NULL))
 		inhdr->status = (uint8_t)err;
 
-- 
2.13.6

  parent reply	other threads:[~2019-01-04 11:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-04 11:22 [dpdk-dev] [PATCH 0/2] vhost/crypto: some fixes Fan Zhang
2019-01-04 11:22 ` [dpdk-dev] [PATCH 1/2] vhost/crypto: fix possible dead loop Fan Zhang
2019-01-04 17:23   ` Maxime Coquelin
2019-01-04 11:22 ` Fan Zhang [this message]
2019-01-04 17:24   ` [dpdk-dev] [PATCH 2/2] vhost/crypto: fix possible out of bound access Maxime Coquelin
2019-01-04 18:00 ` [dpdk-dev] [PATCH 0/2] vhost/crypto: some fixes Maxime Coquelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190104112246.9338-3-roy.fan.zhang@intel.com \
    --to=roy.fan.zhang@intel.com \
    --cc=dev@dpdk.org \
    --cc=maxime.coquelin@redhat.com \
    --cc=stable@dpdk.org \
    --cc=tiwei.bie@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).