[RFC] vhost/crypto: fix descriptor index validation

[Stephen Hemminger]

The descriptor index from the available ring was not validated and
was re-read from shared memory in the finalize path. Save the
descriptor index at fetch time, validate it, and use a compiler
barrier to prevent reloading from shared memory.

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

Reported-by: Adiel Sol <adiel.sol at dream-security.com>
Reported-by: Arad Inbar <arad.inbar at dream-security.com>
Reported-by: Erez Cohen <erez.cohen at dream-security.com>
Reported-by: Nir Somech <nir.somech at dream-security.com>
Reported-by: Ben Grinberg <ben.grinberg at dream-security.com>
Reported-by: Daniel Lubel <daniel.lubel at dream-security.com>
Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
---
This patch was generated by usin AI to analyze bug report.
IT HAS NOT BEEN TESTED.

 lib/vhost/vhost_crypto.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
index 648e2d731b..f1d3702070 100644
--- a/lib/vhost/vhost_crypto.c
+++ b/lib/vhost/vhost_crypto.c
@@ -242,6 +242,7 @@ struct vhost_crypto_data_req {
 	struct vhost_virtqueue *vq;
 	struct vhost_crypto_writeback_data *wb;
 	struct rte_mempool *wb_pool;
+	uint16_t used_idx;
 	uint16_t desc_idx;
 	uint16_t len;
 	uint16_t zero_copy;
@@ -1486,7 +1487,7 @@ static __rte_always_inline int
 vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
 		struct vhost_virtqueue *vq, struct rte_crypto_op *op,
 		struct vring_desc *head, struct vhost_crypto_desc *descs,
-		uint16_t desc_idx)
+		uint16_t used_idx, uint16_t desc_idx)
 	__rte_requires_shared_capability(&vq->iotlb_lock)
 {
 	struct vhost_crypto_data_req *vc_req, *vc_req_out;
@@ -1504,6 +1505,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
 	int err;
 
 	vc_req = &data_req;
+	vc_req->used_idx = used_idx;
 	vc_req->desc_idx = desc_idx;
 	vc_req->dev = vcrypto->dev;
 	vc_req->vq = vq;
@@ -1725,7 +1727,8 @@ vhost_crypto_finalize_one_request(struct rte_crypto_op *op,
 		return NULL;
 	}
 	vq = vc_req->vq;
-	used_idx = vc_req->desc_idx;
+	used_idx = vc_req->used_idx;
+	desc_idx = vc_req->desc_idx;
 
 	if (old_vq && (vq != old_vq))
 		return vq;
@@ -1737,9 +1740,8 @@ vhost_crypto_finalize_one_request(struct rte_crypto_op *op,
 			write_back_data(vc_req);
 	}
 
-	desc_idx = vq->avail->ring[used_idx];
-	vq->used->ring[desc_idx].id = vq->avail->ring[desc_idx];
-	vq->used->ring[desc_idx].len = vc_req->len;
+	vq->used->ring[used_idx].id = desc_idx;
+	vq->used->ring[used_idx].len = vc_req->len;
 
 	if (op->type == RTE_CRYPTO_OP_TYPE_SYMMETRIC) {
 		rte_mempool_put(m_src->pool, (void *)m_src);
@@ -2044,16 +2046,24 @@ rte_vhost_crypto_fetch_requests(int vid, uint32_t qid,
 		for (i = 0; i < count; i++) {
 			uint16_t used_idx = (start_idx + i) & (vq->size - 1);
 			uint16_t desc_idx = vq->avail->ring[used_idx];
-			struct vring_desc *head = &vq->desc[desc_idx];
+			struct vring_desc *head;
 			struct rte_crypto_op *op = ops[i];
 
+			rte_compiler_barrier();
+			if (unlikely(desc_idx >= vq->size)) {
+				VC_LOG_ERR("Invalid descriptor index %u",
+					   desc_idx);
+				break;
+			}
+			head = &vq->desc[desc_idx];
+
 			op->sym->m_src = mbufs[i * 2];
 			op->sym->m_dst = mbufs[i * 2 + 1];
 			op->sym->m_src->data_off = 0;
 			op->sym->m_dst->data_off = 0;
 
 			if (unlikely(vhost_crypto_process_one_req(vcrypto, vq,
-					op, head, descs, used_idx) < 0))
+					op, head, descs, used_idx, desc_idx) < 0))
 				break;
 		}
 
@@ -2074,15 +2084,23 @@ rte_vhost_crypto_fetch_requests(int vid, uint32_t qid,
 		for (i = 0; i < count; i++) {
 			uint16_t used_idx = (start_idx + i) & (vq->size - 1);
 			uint16_t desc_idx = vq->avail->ring[used_idx];
-			struct vring_desc *head = &vq->desc[desc_idx];
+			struct vring_desc *head;
 			struct rte_crypto_op *op = ops[i];
 
+			rte_compiler_barrier();
+			if (unlikely(desc_idx >= vq->size)) {
+				VC_LOG_ERR("Invalid descriptor index %u",
+					   desc_idx);
+				break;
+			}
+			head = &vq->desc[desc_idx];
+
 			op->sym->m_src = mbufs[i];
 			op->sym->m_dst = NULL;
 			op->sym->m_src->data_off = 0;
 
 			if (unlikely(vhost_crypto_process_one_req(vcrypto, vq,
-					op, head, descs, desc_idx) < 0))
+					op, head, descs, used_idx, desc_idx) < 0))
 				break;
 		}
 
-- 
2.53.0