[dpdk-stable] patch 'test/mbuf: fix access to freed memory' has been queued to stable release 20.11.4

[Xueming Li]

Hi,

FYI, your patch has been queued to stable release 20.11.4

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 11/12/21. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/steevenlee/dpdk

This queued commit can be viewed at:
https://github.com/steevenlee/dpdk/commit/91ec527f27d1988bc88678ca37e16e7f5b988799

Thanks.

Xueming Li <xuemingl at nvidia.com>

---
>From 91ec527f27d1988bc88678ca37e16e7f5b988799 Mon Sep 17 00:00:00 2001
From: Olivier Matz <olivier.matz at 6wind.com>
Date: Fri, 29 Oct 2021 14:15:44 +0200
Subject: [PATCH] test/mbuf: fix access to freed memory
Cc: Xueming Li <xuemingl at nvidia.com>

[ upstream commit 7dc627426e534c3b04e4f872343e1899df26471e ]

Seen by ASan.

In the external buffer mbuf test, we check that the buffer is freed
by checking that its refcount is 0.
This is not a valid condition, because it accesses to an already
freed area.

Fix this by setting a boolean flag in the callback when rte_free()
is actually called, and check this flag instead.

Bugzilla ID: 867
Fixes: 7b295dceea07 ("test/mbuf: add unit test cases")

Reported-by: David Marchand <david.marchand at redhat.com>
Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
Reviewed-by: David Marchand <david.marchand at redhat.com>
---
 app/test/test_mbuf.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index 69a361c2f5..a6c11dd5b1 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -2305,16 +2305,16 @@ fail:
 
 /* Define a free call back function to be used for external buffer */
 static void
-ext_buf_free_callback_fn(void *addr __rte_unused, void *opaque)
+ext_buf_free_callback_fn(void *addr, void *opaque)
 {
-	void *ext_buf_addr = opaque;
+	bool *freed = opaque;
 
-	if (ext_buf_addr == NULL) {
+	if (addr == NULL) {
 		printf("External buffer address is invalid\n");
 		return;
 	}
-	rte_free(ext_buf_addr);
-	ext_buf_addr = NULL;
+	rte_free(addr);
+	*freed = true;
 	printf("External buffer freed via callback\n");
 }
 
@@ -2338,6 +2338,7 @@ test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool *pktmbuf_pool)
 	void *ext_buf_addr = NULL;
 	uint16_t buf_len = EXT_BUF_TEST_DATA_LEN +
 				sizeof(struct rte_mbuf_ext_shared_info);
+	bool freed = false;
 
 	/* alloc a mbuf */
 	m = rte_pktmbuf_alloc(pktmbuf_pool);
@@ -2353,7 +2354,7 @@ test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool *pktmbuf_pool)
 		GOTO_FAIL("%s: External buffer allocation failed\n", __func__);
 
 	ret_shinfo = rte_pktmbuf_ext_shinfo_init_helper(ext_buf_addr, &buf_len,
-		ext_buf_free_callback_fn, ext_buf_addr);
+		ext_buf_free_callback_fn, &freed);
 	if (ret_shinfo == NULL)
 		GOTO_FAIL("%s: Shared info initialization failed!\n", __func__);
 
@@ -2386,26 +2387,35 @@ test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool *pktmbuf_pool)
 
 	if (rte_mbuf_ext_refcnt_read(ret_shinfo) != 2)
 		GOTO_FAIL("%s: Invalid ext_buf ref_cnt\n", __func__);
+	if (freed)
+		GOTO_FAIL("%s: extbuf should not be freed\n", __func__);
 
 	/* test to manually update ext_buf_ref_cnt from 2 to 3*/
 	rte_mbuf_ext_refcnt_update(ret_shinfo, 1);
 	if (rte_mbuf_ext_refcnt_read(ret_shinfo) != 3)
 		GOTO_FAIL("%s: Update ext_buf ref_cnt failed\n", __func__);
+	if (freed)
+		GOTO_FAIL("%s: extbuf should not be freed\n", __func__);
 
 	/* reset the ext_refcnt before freeing the external buffer */
 	rte_mbuf_ext_refcnt_set(ret_shinfo, 2);
 	if (rte_mbuf_ext_refcnt_read(ret_shinfo) != 2)
 		GOTO_FAIL("%s: set ext_buf ref_cnt failed\n", __func__);
+	if (freed)
+		GOTO_FAIL("%s: extbuf should not be freed\n", __func__);
 
 	/* detach the external buffer from mbufs */
 	rte_pktmbuf_detach_extbuf(m);
 	/* check if ref cnt is decremented */
 	if (rte_mbuf_ext_refcnt_read(ret_shinfo) != 1)
 		GOTO_FAIL("%s: Invalid ext_buf ref_cnt\n", __func__);
+	if (freed)
+		GOTO_FAIL("%s: extbuf should not be freed\n", __func__);
 
 	rte_pktmbuf_detach_extbuf(clone);
-	if (rte_mbuf_ext_refcnt_read(ret_shinfo) != 0)
-		GOTO_FAIL("%s: Invalid ext_buf ref_cnt\n", __func__);
+	if (!freed)
+		GOTO_FAIL("%s: extbuf should be freed\n", __func__);
+	freed = false;
 
 	rte_pktmbuf_free(m);
 	m = NULL;
-- 
2.33.0

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2021-11-10 14:17:10.980846666 +0800
+++ 0205-test-mbuf-fix-access-to-freed-memory.patch	2021-11-10 14:17:02.014078251 +0800
@@ -1 +1 @@
-From 7dc627426e534c3b04e4f872343e1899df26471e Mon Sep 17 00:00:00 2001
+From 91ec527f27d1988bc88678ca37e16e7f5b988799 Mon Sep 17 00:00:00 2001
@@ -4,0 +5,3 @@
+Cc: Xueming Li <xuemingl at nvidia.com>
+
+[ upstream commit 7dc627426e534c3b04e4f872343e1899df26471e ]
@@ -18 +20,0 @@
-Cc: stable at dpdk.org
@@ -28 +30 @@
-index 94d1cdde37..f93bcef8a9 100644
+index 69a361c2f5..a6c11dd5b1 100644
@@ -31 +33 @@
-@@ -2307,16 +2307,16 @@ fail:
+@@ -2305,16 +2305,16 @@ fail:
@@ -53 +55 @@
-@@ -2340,6 +2340,7 @@ test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool *pktmbuf_pool)
+@@ -2338,6 +2338,7 @@ test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool *pktmbuf_pool)
@@ -61 +63 @@
-@@ -2355,7 +2356,7 @@ test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool *pktmbuf_pool)
+@@ -2353,7 +2354,7 @@ test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool *pktmbuf_pool)
@@ -70 +72 @@
-@@ -2388,26 +2389,35 @@ test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool *pktmbuf_pool)
+@@ -2386,26 +2387,35 @@ test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool *pktmbuf_pool)