From 7c9fc7bc883107eaa5842792450a565db6b6ee99 Mon Sep 17 00:00:00 2001 From: Olivier Matz Date: Fri, 29 Oct 2021 13:47:37 +0200 Subject: [PATCH] test/mbuf: fix access to freed memory 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. Reported-by: David Marchand Signed-off-by: Olivier Matz --- app/test/test_mbuf.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index 94d1cdde37..94dc3a3ea9 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c @@ -2307,16 +2307,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 __rte_unused) { - 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,34 @@ 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 == false) + GOTO_FAIL("%s: extbuf should be freed\n", __func__); rte_pktmbuf_free(m); m = NULL; -- 2.20.1