vhost: fix corner case for split ring enqueue operation

Message ID 1536305094-126439-1-git-send-email-jiayu.hu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: fix corner case for split ring enqueue operation |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Hu, Jiayu Sept. 7, 2018, 7:24 a.m. UTC
  When perform enqueue operations on the split ring, if the
reserved buffer length from the descriptor table execeeds 65535,
the returned length by fill_vec_buf_split() is overflowed.
This patch is to avoid this corner case.

Fixes: f689586bc060 ("vhost: shadow used ring update")

Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
---
 lib/librte_vhost/virtio_net.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
  

Comments

Gavin Hu Sept. 7, 2018, 9:51 a.m. UTC | #1
This is a good catch for split vring, I advise to change also for packed vring. It also has this issue.

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Jiayu Hu
> Sent: Friday, September 7, 2018 3:25 PM
> To: dev@dpdk.org
> Cc: zhihong.wang@intel.com; tiwei.bie@intel.com;
> maxime.coquelin@redhat.com; Jiayu Hu <jiayu.hu@intel.com>
> Subject: [dpdk-dev] [PATCH] vhost: fix corner case for split ring enqueue
> operation
>
> When perform enqueue operations on the split ring, if the reserved buffer
> length from the descriptor table execeeds 65535, the returned length by
> fill_vec_buf_split() is overflowed.
> This patch is to avoid this corner case.
>
> Fixes: f689586bc060 ("vhost: shadow used ring update")
>
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> ---
>  lib/librte_vhost/virtio_net.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index
> 99c7afc..9f3c88f 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -122,7 +122,7 @@ flush_shadow_used_ring_split(struct virtio_net *dev,
> struct vhost_virtqueue *vq)
>
>  static __rte_always_inline void
>  update_shadow_used_ring_split(struct vhost_virtqueue *vq,
> - uint16_t desc_idx, uint16_t len)
> + uint16_t desc_idx, uint32_t len)
>  {
>  uint16_t i = vq->shadow_used_idx++;
>
> @@ -329,7 +329,7 @@ static __rte_always_inline int  fill_vec_buf_split(struct
> virtio_net *dev, struct vhost_virtqueue *vq,
>   uint32_t avail_idx, uint16_t *vec_idx,
>   struct buf_vector *buf_vec, uint16_t
> *desc_chain_head,
> - uint16_t *desc_chain_len, uint8_t perm)
> + uint32_t *desc_chain_len, uint8_t perm)
>  {
>  uint16_t idx = vq->avail->ring[avail_idx & (vq->size - 1)];
>  uint16_t vec_id = *vec_idx;
> @@ -409,7 +409,7 @@ reserve_avail_buf_split(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  uint16_t max_tries, tries = 0;
>
>  uint16_t head_idx = 0;
> -uint16_t len = 0;
> +uint32_t len = 0;
>
>  *num_buffers = 0;
>  cur_idx  = vq->last_avail_idx;
> @@ -1378,7 +1378,8 @@ virtio_dev_tx_split(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>
>  for (i = 0; i < count; i++) {
>  struct buf_vector buf_vec[BUF_VECTOR_MAX];
> -uint16_t head_idx, dummy_len;
> +uint16_t head_idx;
> +uint32_t dummy_len;
>  uint16_t nr_vec = 0;
>  int err;
>
> --
> 2.7.4

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  

Patch

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 99c7afc..9f3c88f 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -122,7 +122,7 @@  flush_shadow_used_ring_split(struct virtio_net *dev, struct vhost_virtqueue *vq)
 
 static __rte_always_inline void
 update_shadow_used_ring_split(struct vhost_virtqueue *vq,
-			 uint16_t desc_idx, uint16_t len)
+			 uint16_t desc_idx, uint32_t len)
 {
 	uint16_t i = vq->shadow_used_idx++;
 
@@ -329,7 +329,7 @@  static __rte_always_inline int
 fill_vec_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 			 uint32_t avail_idx, uint16_t *vec_idx,
 			 struct buf_vector *buf_vec, uint16_t *desc_chain_head,
-			 uint16_t *desc_chain_len, uint8_t perm)
+			 uint32_t *desc_chain_len, uint8_t perm)
 {
 	uint16_t idx = vq->avail->ring[avail_idx & (vq->size - 1)];
 	uint16_t vec_id = *vec_idx;
@@ -409,7 +409,7 @@  reserve_avail_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	uint16_t max_tries, tries = 0;
 
 	uint16_t head_idx = 0;
-	uint16_t len = 0;
+	uint32_t len = 0;
 
 	*num_buffers = 0;
 	cur_idx  = vq->last_avail_idx;
@@ -1378,7 +1378,8 @@  virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 
 	for (i = 0; i < count; i++) {
 		struct buf_vector buf_vec[BUF_VECTOR_MAX];
-		uint16_t head_idx, dummy_len;
+		uint16_t head_idx;
+		uint32_t dummy_len;
 		uint16_t nr_vec = 0;
 		int err;