[dpdk-dev] [PATCH] test/mbuf: fix access to freed memory

[Olivier Matz]

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")
Cc: sta...@dpdk.org

Reported-by: David Marchand <david.march...@redhat.com>
Signed-off-by: Olivier Matz <olivier.m...@6wind.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 94d1cdde37..f93bcef8a9 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -2307,16 +2307,16 @@ test_pktmbuf_read_from_chain(struct rte_mempool 
*pktmbuf_pool)
 
 /* 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");
 }
 
@@ -2340,6 +2340,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);
@@ -2355,7 +2356,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__);
 
@@ -2388,26 +2389,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.30.2