DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/3] vhost: fix split ring potential buffer overflow
@ 2021-03-31  6:49 Marvin Liu
  2021-03-31  6:49 ` [dpdk-dev] [PATCH 2/3] vhost: fix packed " Marvin Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Marvin Liu @ 2021-03-31  6:49 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia; +Cc: dev, Marvin Liu, stable

In vhost datapath, descriptor's length are mostly used in two coherent
operations. First step is used for address translation, second step is
used for memory transaction from guest to host. But the iterval between
two steps will give a window for malicious guest, in which can change
descriptor length after vhost calcuated buffer size. Thus may lead to
buffer overflow in vhost side. This potential risk can be eliminated by
accessing the descriptor length once.

Fixes: 1be4ebb1c464 ("vhost: support indirect descriptor in mergeable Rx")
Cc: stable@dpdk.org

Signed-off-by: Marvin Liu <yong.liu@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 583bf379c6..576a0a20c0 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -548,10 +548,11 @@ fill_vec_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 			return -1;
 		}
 
-		len += descs[idx].len;
+		dlen = descs[idx].len;
+		len += dlen;
 
 		if (unlikely(map_one_desc(dev, vq, buf_vec, &vec_id,
-						descs[idx].addr, descs[idx].len,
+						descs[idx].addr, dlen,
 						perm))) {
 			free_ind_table(idesc);
 			return -1;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dpdk-dev] [PATCH 2/3] vhost: fix packed ring potential buffer overflow
  2021-03-31  6:49 [dpdk-dev] [PATCH 1/3] vhost: fix split ring potential buffer overflow Marvin Liu
@ 2021-03-31  6:49 ` Marvin Liu
  2021-03-31  6:49 ` [dpdk-dev] [PATCH 3/3] vhost: fix potential buffer overflow when batch dequeue Marvin Liu
  2021-03-31  7:48 ` [dpdk-dev] [PATCH 1/3] vhost: fix split ring potential buffer overflow Xia, Chenbo
  2 siblings, 0 replies; 5+ messages in thread
From: Marvin Liu @ 2021-03-31  6:49 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia; +Cc: dev, Marvin Liu, stable

Similar as split ring, the multiple accesses of descriptor length will
lead to potential risk. One-time access of descriptor length can
eliminate this risk.

Fixes: 2f3225a7d69b ("vhost: add vector filling support for packed ring")
Cc: stable@dpdk.org

Signed-off-by: Marvin Liu <yong.liu@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 576a0a20c0..de43686522 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -669,9 +669,10 @@ fill_vec_buf_packed_indirect(struct virtio_net *dev,
 			return -1;
 		}
 
-		*len += descs[i].len;
+		dlen = descs[i].len;
+		*len += dlen;
 		if (unlikely(map_one_desc(dev, vq, buf_vec, &vec_id,
-						descs[i].addr, descs[i].len,
+						descs[i].addr, dlen,
 						perm)))
 			return -1;
 	}
@@ -692,6 +693,7 @@ fill_vec_buf_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	bool wrap_counter = vq->avail_wrap_counter;
 	struct vring_packed_desc *descs = vq->desc_packed;
 	uint16_t vec_id = *vec_idx;
+	uint64_t dlen;
 
 	if (avail_idx < vq->last_avail_idx)
 		wrap_counter ^= 1;
@@ -724,11 +726,12 @@ fill_vec_buf_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
 							len, perm) < 0))
 				return -1;
 		} else {
-			*len += descs[avail_idx].len;
+			dlen = descs[avail_idx].len;
+			*len += dlen;
 
 			if (unlikely(map_one_desc(dev, vq, buf_vec, &vec_id,
 							descs[avail_idx].addr,
-							descs[avail_idx].len,
+							dlen,
 							perm)))
 				return -1;
 		}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dpdk-dev] [PATCH 3/3] vhost: fix potential buffer overflow when batch dequeue
  2021-03-31  6:49 [dpdk-dev] [PATCH 1/3] vhost: fix split ring potential buffer overflow Marvin Liu
  2021-03-31  6:49 ` [dpdk-dev] [PATCH 2/3] vhost: fix packed " Marvin Liu
@ 2021-03-31  6:49 ` Marvin Liu
  2021-03-31  7:48 ` [dpdk-dev] [PATCH 1/3] vhost: fix split ring potential buffer overflow Xia, Chenbo
  2 siblings, 0 replies; 5+ messages in thread
From: Marvin Liu @ 2021-03-31  6:49 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia; +Cc: dev, Marvin Liu, stable

Similar as single dequeue, the multiple accesses of descriptor length
will lead to potential risk. One-time access of descriptor length can
eliminate this risk.

Fixes: 75ed51697820 ("vhost: add packed ring batch dequeue")
Cc: stable@dpdk.org

Signed-off-by: Marvin Liu <yong.liu@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index de43686522..0a7d008a91 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -2318,7 +2318,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,
 	}
 
 	vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) {
-		pkts[i]->pkt_len = descs[avail_idx + i].len - buf_offset;
+		pkts[i]->pkt_len = lens[i] - buf_offset;
 		pkts[i]->data_len = pkts[i]->pkt_len;
 		ids[i] = descs[avail_idx + i].id;
 	}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] vhost: fix split ring potential buffer overflow
  2021-03-31  6:49 [dpdk-dev] [PATCH 1/3] vhost: fix split ring potential buffer overflow Marvin Liu
  2021-03-31  6:49 ` [dpdk-dev] [PATCH 2/3] vhost: fix packed " Marvin Liu
  2021-03-31  6:49 ` [dpdk-dev] [PATCH 3/3] vhost: fix potential buffer overflow when batch dequeue Marvin Liu
@ 2021-03-31  7:48 ` Xia, Chenbo
  2 siblings, 0 replies; 5+ messages in thread
From: Xia, Chenbo @ 2021-03-31  7:48 UTC (permalink / raw)
  To: Liu, Yong, maxime.coquelin; +Cc: dev, stable

> -----Original Message-----
> From: Liu, Yong <yong.liu@intel.com>
> Sent: Wednesday, March 31, 2021 2:50 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Liu, Yong <yong.liu@intel.com>; stable@dpdk.org
> Subject: [PATCH 1/3] vhost: fix split ring potential buffer overflow
> 
> In vhost datapath, descriptor's length are mostly used in two coherent
> operations. First step is used for address translation, second step is
> used for memory transaction from guest to host. But the iterval between
> two steps will give a window for malicious guest, in which can change
> descriptor length after vhost calcuated buffer size. Thus may lead to
> buffer overflow in vhost side. This potential risk can be eliminated by
> accessing the descriptor length once.
> 
> Fixes: 1be4ebb1c464 ("vhost: support indirect descriptor in mergeable Rx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> --
> 2.17.1

Series applied to next-virtio/main, Thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dpdk-dev] [PATCH 3/3] vhost: fix potential buffer overflow when batch dequeue
  2021-03-25  3:01 ` [dpdk-dev] [PATCH 1/3] vhost: fix split ring " Marvin Liu
@ 2021-03-25  3:01   ` Marvin Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Marvin Liu @ 2021-03-25  3:01 UTC (permalink / raw)
  To: maxime.coquelin, chenbo.xia; +Cc: dev, Marvin Liu, stable

Similar as single dequeue, the multiple accesses of descriptor length
will lead to potential risk. One-time access of descriptor length can
eliminate this risk.

Fixes: 75ed51697820 ("vhost: add packed ring batch dequeue")
Cc: stable@dpdk.org

Signed-off-by: Marvin Liu <yong.liu@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index de43686522..0a7d008a91 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -2318,7 +2318,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,
 	}
 
 	vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) {
-		pkts[i]->pkt_len = descs[avail_idx + i].len - buf_offset;
+		pkts[i]->pkt_len = lens[i] - buf_offset;
 		pkts[i]->data_len = pkts[i]->pkt_len;
 		ids[i] = descs[avail_idx + i].id;
 	}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-31  7:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31  6:49 [dpdk-dev] [PATCH 1/3] vhost: fix split ring potential buffer overflow Marvin Liu
2021-03-31  6:49 ` [dpdk-dev] [PATCH 2/3] vhost: fix packed " Marvin Liu
2021-03-31  6:49 ` [dpdk-dev] [PATCH 3/3] vhost: fix potential buffer overflow when batch dequeue Marvin Liu
2021-03-31  7:48 ` [dpdk-dev] [PATCH 1/3] vhost: fix split ring potential buffer overflow Xia, Chenbo
  -- strict thread matches above, loose matches on Subject: below --
2021-02-26  7:33 [dpdk-dev] [PATCH] vhost: fix " Marvin Liu
2021-03-25  3:01 ` [dpdk-dev] [PATCH 1/3] vhost: fix split ring " Marvin Liu
2021-03-25  3:01   ` [dpdk-dev] [PATCH 3/3] vhost: fix potential buffer overflow when batch dequeue Marvin Liu

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git