diff mbox series

examples/vhost: fix use after free

Message ID 20220714051106.1134222-1-wenwux.ma@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series examples/vhost: fix use after free |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Ma, WenwuX July 14, 2022, 5:11 a.m. UTC 
  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")
Cc: stable@dpdk.org

Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
---
 examples/vhost/main.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Ling, WeiX July 14, 2022, 7:55 a.m. UTC | #1 
> -----Original Message-----
> From: Ma, WenwuX <wenwux.ma@intel.com>
> Sent: Thursday, July 14, 2022 1:11 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> dev@dpdk.org
> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>;
> He, Xingguang <xingguang.he@intel.com>; Ling, WeiX
> <weix.ling@intel.com>; Wang, YuanX <yuanx.wang@intel.com>; Ma,
> WenwuX <wenwux.ma@intel.com>; stable@dpdk.org
> Subject: [PATCH] examples/vhost: fix use after free
> 
> 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")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
> ---

Tested-by: Wei Ling <weix.ling@intel.com>
Chenbo Xia July 15, 2022, 5:54 a.m. UTC | #2 
> -----Original Message-----
> From: Ma, WenwuX <wenwux.ma@intel.com>
> Sent: Thursday, July 14, 2022 1:11 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> dev@dpdk.org
> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He,
> Xingguang <xingguang.he@intel.com>; Ling, WeiX <weix.ling@intel.com>; Wang,
> YuanX <yuanx.wang@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] examples/vhost: fix use after free
> 
> 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")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
> ---
>  examples/vhost/main.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 

As discussed in yesterday's release meeting, this issue should have minor impact,
so the fix could be moved to next release.

Thanks,
Chenbo
Chenbo Xia Sept. 22, 2022, 1:46 p.m. UTC | #3 
> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Friday, July 15, 2022 1:55 PM
> To: Ma, WenwuX <wenwux.ma@intel.com>; maxime.coquelin@redhat.com;
> dev@dpdk.org
> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He,
> Xingguang <xingguang.he@intel.com>; Ling, WeiX <weix.ling@intel.com>; Wang,
> YuanX <yuanx.wang@intel.com>; stable@dpdk.org
> Subject: RE: [PATCH] examples/vhost: fix use after free
> 
> > -----Original Message-----
> > From: Ma, WenwuX <wenwux.ma@intel.com>
> > Sent: Thursday, July 14, 2022 1:11 PM
> > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> > dev@dpdk.org
> > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>;
> He,
> > Xingguang <xingguang.he@intel.com>; Ling, WeiX <weix.ling@intel.com>;
> Wang,
> > YuanX <yuanx.wang@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>;
> > stable@dpdk.org
> > Subject: [PATCH] examples/vhost: fix use after free
> >
> > 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")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
> > ---
> >  examples/vhost/main.c | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> >
> 

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
Chenbo Xia Sept. 29, 2022, 8:40 a.m. UTC | #4 
> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Thursday, September 22, 2022 9:47 PM
> To: Ma, WenwuX <wenwux.ma@intel.com>; maxime.coquelin@redhat.com;
> dev@dpdk.org
> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He,
> Xingguang <xingguang.he@intel.com>; Ling, WeiX <weix.ling@intel.com>; Wang,
> YuanX <yuanx.wang@intel.com>; stable@dpdk.org
> Subject: RE: [PATCH] examples/vhost: fix use after free
> 
> > -----Original Message-----
> > From: Xia, Chenbo <chenbo.xia@intel.com>
> > Sent: Friday, July 15, 2022 1:55 PM
> > To: Ma, WenwuX <wenwux.ma@intel.com>; maxime.coquelin@redhat.com;
> > dev@dpdk.org
> > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>;
> He,
> > Xingguang <xingguang.he@intel.com>; Ling, WeiX <weix.ling@intel.com>;
> Wang,
> > YuanX <yuanx.wang@intel.com>; stable@dpdk.org
> > Subject: RE: [PATCH] examples/vhost: fix use after free
> >
> > > -----Original Message-----
> > > From: Ma, WenwuX <wenwux.ma@intel.com>
> > > Sent: Thursday, July 14, 2022 1:11 PM
> > > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> > > dev@dpdk.org
> > > Cc: Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>;
> > He,
> > > Xingguang <xingguang.he@intel.com>; Ling, WeiX <weix.ling@intel.com>;
> > Wang,
> > > YuanX <yuanx.wang@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>;
> > > stable@dpdk.org
> > > Subject: [PATCH] examples/vhost: fix use after free
> > >
> > > 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")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
> > > ---
> > >  examples/vhost/main.c | 19 ++++++++++++-------
> > >  1 file changed, 12 insertions(+), 7 deletions(-)
> > >
> >
> 
> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

Applied to next-virtio/main, thanks
diff mbox series

Patch

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 7e1666f42a..7956dc4f13 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -1073,8 +1073,13 @@  drain_vhost(struct vhost_dev *vdev)
 				__ATOMIC_SEQ_CST);
 	}
 
-	if (!dma_bind[vid2socketid[vdev->vid]].dmas[VIRTIO_RXQ].async_enabled)
+	if (!dma_bind[vid2socketid[vdev->vid]].dmas[VIRTIO_RXQ].async_enabled) {
 		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
@@ -1350,17 +1355,12 @@  async_enqueue_pkts(struct vhost_dev *dev, uint16_t queue_id,
 		struct rte_mbuf **pkts, uint32_t rx_count)
 {
 	uint16_t enqueue_count;
-	uint16_t enqueue_fail = 0;
 	uint16_t dma_id = dma_bind[vid2socketid[dev->vid]].dmas[VIRTIO_RXQ].dev_id;
 
 	complete_async_pkts(dev);
 	enqueue_count = rte_vhost_submit_enqueue_burst(dev->vid, queue_id,
 					pkts, rx_count, dma_id, 0);
 
-	enqueue_fail = rx_count - enqueue_count;
-	if (enqueue_fail)
-		free_pkts(&pkts[enqueue_count], enqueue_fail);
-
 	return enqueue_count;
 }
 
@@ -1405,8 +1405,13 @@  drain_eth_rx(struct vhost_dev *vdev)
 				__ATOMIC_SEQ_CST);
 	}
 
-	if (!dma_bind[vid2socketid[vdev->vid]].dmas[VIRTIO_RXQ].async_enabled)
+	if (!dma_bind[vid2socketid[vdev->vid]].dmas[VIRTIO_RXQ].async_enabled) {
 		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);
+	}
 }
 
 uint16_t async_dequeue_pkts(struct vhost_dev *dev, uint16_t queue_id,