[PATCH 21.11] examples/vhost: fix use after free

[PATCH 21.11] examples/vhost: fix use after free

[Wenwu Ma]

[ upstream commit 40abb903fe0aff0556d15d96385a4c7b647649b5 ]

In async_enqueue_pkts(), the failed pkts will
be freed before return, but, the failed pkts may be
retried later, it will cause use after free. So,
we free the failed pkts after retry.

Fixes: 1907ce4baec3 ("examples/vhost: fix retry logic on Rx path")

Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
Tested-by: Wei Ling <weix.ling@intel.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
---
 examples/vhost/main.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index f9e932061f..36464922e3 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -908,17 +908,10 @@ enqueue_pkts(struct vhost_dev *vdev, struct rte_mbuf **pkts, uint16_t rx_count)
 	if (builtin_net_driver) {
 		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ, pkts, rx_count);
 	} else if (async_vhost_driver) {
-		uint16_t enqueue_fail = 0;
-
 		complete_async_pkts(vdev);
 		enqueue_count = rte_vhost_submit_enqueue_burst(vdev->vid,
 					VIRTIO_RXQ, pkts, rx_count);
 		__atomic_add_fetch(&vdev->pkts_inflight, enqueue_count, __ATOMIC_SEQ_CST);
-
-		enqueue_fail = rx_count - enqueue_count;
-		if (enqueue_fail)
-			free_pkts(&pkts[enqueue_count], enqueue_fail);
-
 	} else {
 		enqueue_count = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
 						pkts, rx_count);
@@ -944,8 +937,13 @@ drain_vhost(struct vhost_dev *vdev)
 				__ATOMIC_SEQ_CST);
 	}
 
-	if (!async_vhost_driver)
+	if (!async_vhost_driver) {
 		free_pkts(m, nr_xmit);
+	} else {
+		uint16_t enqueue_fail = nr_xmit - ret;
+		if (enqueue_fail > 0)
+			free_pkts(&m[ret], enqueue_fail);
+	}
 }
 
 static __rte_always_inline void
@@ -1249,8 +1247,13 @@ drain_eth_rx(struct vhost_dev *vdev)
 				__ATOMIC_SEQ_CST);
 	}
 
-	if (!async_vhost_driver)
+	if (!async_vhost_driver) {
 		free_pkts(pkts, rx_count);
+	} else {
+		uint16_t enqueue_fail = rx_count - enqueue_count;
+		if (enqueue_fail > 0)
+			free_pkts(&pkts[enqueue_count], enqueue_fail);
+	}
 }
 
 static __rte_always_inline void
-- 
2.25.1

Re: [PATCH 21.11] examples/vhost: fix use after free

[Kevin Traynor]

On 16/11/2022 01:40, Wenwu Ma wrote:
> [ upstream commit 40abb903fe0aff0556d15d96385a4c7b647649b5 ]
> 
> In async_enqueue_pkts(), the failed pkts will
> be freed before return, but, the failed pkts may be
> retried later, it will cause use after free. So,
> we free the failed pkts after retry.
> 
> Fixes: 1907ce4baec3 ("examples/vhost: fix retry logic on Rx path")
> 
> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
> Tested-by: Wei Ling <weix.ling@intel.com>
> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
> ---

Thanks for the backport. This is not pushed to 21.11 branch.

>   examples/vhost/main.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index f9e932061f..36464922e3 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -908,17 +908,10 @@ enqueue_pkts(struct vhost_dev *vdev, struct rte_mbuf **pkts, uint16_t rx_count)
>   	if (builtin_net_driver) {
>   		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ, pkts, rx_count);
>   	} else if (async_vhost_driver) {
> -		uint16_t enqueue_fail = 0;
> -
>   		complete_async_pkts(vdev);
>   		enqueue_count = rte_vhost_submit_enqueue_burst(vdev->vid,
>   					VIRTIO_RXQ, pkts, rx_count);
>   		__atomic_add_fetch(&vdev->pkts_inflight, enqueue_count, __ATOMIC_SEQ_CST);
> -
> -		enqueue_fail = rx_count - enqueue_count;
> -		if (enqueue_fail)
> -			free_pkts(&pkts[enqueue_count], enqueue_fail);
> -
>   	} else {
>   		enqueue_count = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
>   						pkts, rx_count);
> @@ -944,8 +937,13 @@ drain_vhost(struct vhost_dev *vdev)
>   				__ATOMIC_SEQ_CST);
>   	}
>   
> -	if (!async_vhost_driver)
> +	if (!async_vhost_driver) {
>   		free_pkts(m, nr_xmit);
> +	} else {
> +		uint16_t enqueue_fail = nr_xmit - ret;
> +		if (enqueue_fail > 0)
> +			free_pkts(&m[ret], enqueue_fail);
> +	}
>   }
>   
>   static __rte_always_inline void
> @@ -1249,8 +1247,13 @@ drain_eth_rx(struct vhost_dev *vdev)
>   				__ATOMIC_SEQ_CST);
>   	}
>   
> -	if (!async_vhost_driver)
> +	if (!async_vhost_driver) {
>   		free_pkts(pkts, rx_count);
> +	} else {
> +		uint16_t enqueue_fail = rx_count - enqueue_count;
> +		if (enqueue_fail > 0)
> +			free_pkts(&pkts[enqueue_count], enqueue_fail);
> +	}
>   }
>   
>   static __rte_always_inline void