[dpdk-dev] [PATCH] vhost: fix buffer length calculation

[dpdk-dev] [PATCH] vhost: fix buffer length calculation

[Tiwei Bie]

Fixes: fd68b4739d2c ("vhost: use buffer vectors in dequeue path")

Reported-by: Yinan Wang <yinan.wang@intel.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 lib/librte_vhost/virtio_net.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 2b7ffcf92..07cc0c845 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -720,7 +720,8 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
 				uint16_t hdr_vec_idx = 0;
 
 				while (remain) {
-					len = remain;
+					len = RTE_MIN(remain,
+						buf_vec[hdr_vec_idx].buf_len);
 					dst = buf_vec[hdr_vec_idx].buf_addr;
 					rte_memcpy((void *)(uintptr_t)dst,
 							(void *)(uintptr_t)src,
@@ -747,7 +748,7 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
 			hdr_addr = 0;
 		}
 
-		cpy_len = RTE_MIN(buf_len, mbuf_avail);
+		cpy_len = RTE_MIN(buf_avail, mbuf_avail);
 
 		if (likely(cpy_len > MAX_BATCH_LEN ||
 					vq->batch_copy_nb_elems >= vq->size)) {
@@ -1112,7 +1113,8 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
 			 * in a contiguous virtual area.
 			 */
 			while (remain) {
-				len = remain;
+				len = RTE_MIN(remain,
+					buf_vec[hdr_vec_idx].buf_len);
 				src = buf_vec[hdr_vec_idx].buf_addr;
 				rte_memcpy((void *)(uintptr_t)dst,
 						   (void *)(uintptr_t)src, len);
-- 
2.18.0

Re: [dpdk-dev] [PATCH] vhost: fix buffer length calculation

[Wang, Zhihong]

> -----Original Message-----
> From: Bie, Tiwei
> Sent: Tuesday, July 17, 2018 9:11 PM
> To: maxime.coquelin@redhat.com; Wang, Zhihong
> <zhihong.wang@intel.com>; dev@dpdk.org
> Cc: Wang, Yinan <yinan.wang@intel.com>; Yao, Lei A <lei.a.yao@intel.com>
> Subject: [PATCH] vhost: fix buffer length calculation
> 
> Fixes: fd68b4739d2c ("vhost: use buffer vectors in dequeue path")
> 
> Reported-by: Yinan Wang <yinan.wang@intel.com>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
>  lib/librte_vhost/virtio_net.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index 2b7ffcf92..07cc0c845 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -720,7 +720,8 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  				uint16_t hdr_vec_idx = 0;
> 
>  				while (remain) {
> -					len = remain;
> +					len = RTE_MIN(remain,
> +
> 	buf_vec[hdr_vec_idx].buf_len);
>  					dst = buf_vec[hdr_vec_idx].buf_addr;
>  					rte_memcpy((void *)(uintptr_t)dst,
>  							(void *)(uintptr_t)src,
> @@ -747,7 +748,7 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  			hdr_addr = 0;
>  		}
> 
> -		cpy_len = RTE_MIN(buf_len, mbuf_avail);
> +		cpy_len = RTE_MIN(buf_avail, mbuf_avail);
> 
>  		if (likely(cpy_len > MAX_BATCH_LEN ||
>  					vq->batch_copy_nb_elems >= vq-
> >size)) {
> @@ -1112,7 +1113,8 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  			 * in a contiguous virtual area.
>  			 */
>  			while (remain) {
> -				len = remain;
> +				len = RTE_MIN(remain,
> +					buf_vec[hdr_vec_idx].buf_len);
>  				src = buf_vec[hdr_vec_idx].buf_addr;
>  				rte_memcpy((void *)(uintptr_t)dst,
>  						   (void *)(uintptr_t)src, len);
> --
> 2.18.0

Acked-by: Zhihong Wang <zhihong.wang@intel.com>

Thanks

Re: [dpdk-dev] [PATCH] vhost: fix buffer length calculation

[Wang, Yinan]

On Tue, Jul 17, 2018 at 09:10:35PM +0800, Tiwei Bie wrote:
> Fixes: fd68b4739d2c ("vhost: use buffer vectors in dequeue path")
> 
> Reported-by: Yinan Wang <yinan.wang@intel.com>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>

Above fix can fix below issue:
https://mails.dpdk.org/archives/dev/2018-July/108137.html

Tested-by: Yinan Wang <yinan.wang@intel.com>

Re: [dpdk-dev] [PATCH] vhost: fix buffer length calculation

[Tiwei Bie]

On Tue, Jul 17, 2018 at 09:10:35PM +0800, Tiwei Bie wrote:
> Fixes: fd68b4739d2c ("vhost: use buffer vectors in dequeue path")
> 
> Reported-by: Yinan Wang <yinan.wang@intel.com>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>

Applied to dpdk-next-virtio/master, thanks.

Re: [dpdk-dev] [PATCH] vhost: fix buffer length calculation

[Bruce Richardson]

On Thu, Jul 19, 2018 at 11:37:31AM +0800, Tiwei Bie wrote:
> On Tue, Jul 17, 2018 at 09:10:35PM +0800, Tiwei Bie wrote:
> > Fixes: fd68b4739d2c ("vhost: use buffer vectors in dequeue path")
> > 
> > Reported-by: Yinan Wang <yinan.wang@intel.com>
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> 
> Applied to dpdk-next-virtio/master, thanks.

It would be great to get this patch into main tree ASAP, since meson build
with gcc 8 is currently broken on master. [Using Fedora 28].

../lib/librte_vhost/virtio_net.c: In function ‘rte_vhost_enqueue_burst’:
cc1: error: ‘__builtin_memcpy’ forming offset [13, 64] is out of the bounds [0, 12] of object ‘tmp_hdr’ with type ‘struct virtio_net_hdr_mrg_rxbuf’ [-Werror=array-bounds]
../lib/librte_vhost/virtio_net.c:636:34: note: ‘tmp_hdr’ declared here
  struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
                                  ^~~~~~~
cc1: error: ‘__builtin_memcpy’ forming offset [13, 64] is out of the bounds [0, 12] of object ‘tmp_hdr’ with type ‘struct virtio_net_hdr_mrg_rxbuf’ [-Werror=array-bounds]
../lib/librte_vhost/virtio_net.c:636:34: note: ‘tmp_hdr’ declared here
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.

Regards,
/Bruce