[dpdk-dev,v4,3/3] vhost: Check offset value

Message ID 1415171435-24252-4-git-send-email-changchun.ouyang@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Ouyang Changchun Nov. 5, 2014, 7:10 a.m. UTC
  This patch checks the packet length offset value, and checks if the extra bytes inside buffer
cross page boundary.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
 examples/vhost/main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
  

Comments

Huawei Xie Nov. 5, 2014, 4:52 p.m. UTC | #1
> -----Original Message-----
> From: Ouyang, Changchun
> Sent: Wednesday, November 05, 2014 12:11 AM
> To: dev@dpdk.org
> Cc: Xie, Huawei; Ananyev, Konstantin; Cao, Waterman; Ouyang, Changchun
> Subject: [PATCH v4 3/3] vhost: Check offset value
> 
> This patch checks the packet length offset value, and checks if the extra bytes
> inside buffer
> cross page boundary.
> 
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
>  examples/vhost/main.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 2916313..a93f7a0 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -1110,7 +1110,8 @@ virtio_tx_route(struct vhost_dev *vdev, struct
> rte_mbuf *m, uint16_t vlan_tag)
>  	}
> 
>  	if (vm2vm_mode == VM2VM_HARDWARE) {
> -		if (find_local_dest(dev, m, &offset, &vlan_tag) != 0) {
> +		if (find_local_dest(dev, m, &offset, &vlan_tag) != 0 ||
> +			offset > rte_pktmbuf_tailroom(m)) {
>  			rte_pktmbuf_free(m);
>  			return;
>  		}
> @@ -1896,7 +1897,9 @@ virtio_dev_tx_zcp(struct virtio_net *dev)
> 
>  		/* Buffer address translation. */
>  		buff_addr = gpa_to_vva(dev, desc->addr);
> -		phys_addr = gpa_to_hpa(vdev, desc->addr, desc->len,
> &addr_type);
> +		/* Need check extra VLAN_HLEN size for inserting VLAN tag */
> +		phys_addr = gpa_to_hpa(vdev, desc->addr, desc->len +
> VLAN_HLEN,
> +			&addr_type);
> 
>  		if (likely(packet_success < (free_entries - 1)))
>  			/* Prefetch descriptor index. */
> --
> 1.8.4.2
Why don't we merge 1,2,3 patches?
  
Thomas Monjalon Nov. 5, 2014, 5 p.m. UTC | #2
2014-11-05 16:52, Xie, Huawei:
> Why don't we merge 1,2,3 patches?

Because it's simpler to understand small patches with a dedicated
explanation in the commit log of each patch.
Why do you want to merge them?
  
Huawei Xie Nov. 5, 2014, 5:05 p.m. UTC | #3
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, November 05, 2014 10:01 AM
> To: Xie, Huawei
> Cc: dev@dpdk.org; Ouyang, Changchun
> Subject: Re: [dpdk-dev] [PATCH v4 3/3] vhost: Check offset value
> 
> 2014-11-05 16:52, Xie, Huawei:
> > Why don't we merge 1,2,3 patches?
> 
> Because it's simpler to understand small patches with a dedicated
> explanation in the commit log of each patch.
> Why do you want to merge them?
> 
> --
> Thomas
Got it, so we prefer each patch fixes one dedicated thing.
  
Ouyang Changchun Nov. 6, 2014, 12:44 a.m. UTC | #4
Agree with Thomas! using small patches is for easily understanding. 
Merging and mixing things together is not a good thing.

Changchun

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, November 6, 2014 1:01 AM
> To: Xie, Huawei
> Cc: dev@dpdk.org; Ouyang, Changchun
> Subject: Re: [dpdk-dev] [PATCH v4 3/3] vhost: Check offset value
> 
> 2014-11-05 16:52, Xie, Huawei:
> > Why don't we merge 1,2,3 patches?
> 
> Because it's simpler to understand small patches with a dedicated
> explanation in the commit log of each patch.
> Why do you want to merge them?
> 
> --
> Thomas
  

Patch

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 2916313..a93f7a0 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -1110,7 +1110,8 @@  virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 	}
 
 	if (vm2vm_mode == VM2VM_HARDWARE) {
-		if (find_local_dest(dev, m, &offset, &vlan_tag) != 0) {
+		if (find_local_dest(dev, m, &offset, &vlan_tag) != 0 ||
+			offset > rte_pktmbuf_tailroom(m)) {
 			rte_pktmbuf_free(m);
 			return;
 		}
@@ -1896,7 +1897,9 @@  virtio_dev_tx_zcp(struct virtio_net *dev)
 
 		/* Buffer address translation. */
 		buff_addr = gpa_to_vva(dev, desc->addr);
-		phys_addr = gpa_to_hpa(vdev, desc->addr, desc->len, &addr_type);
+		/* Need check extra VLAN_HLEN size for inserting VLAN tag */
+		phys_addr = gpa_to_hpa(vdev, desc->addr, desc->len + VLAN_HLEN,
+			&addr_type);
 
 		if (likely(packet_success < (free_entries - 1)))
 			/* Prefetch descriptor index. */