From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id F3DD7A0547; Fri, 29 Oct 2021 14:16:28 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E586E41180; Fri, 29 Oct 2021 14:16:28 +0200 (CEST) Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by mails.dpdk.org (Postfix) with ESMTP id 5DD3C4111F for ; Fri, 29 Oct 2021 14:16:27 +0200 (CEST) Received: by mail-wm1-f42.google.com with SMTP id b2-20020a1c8002000000b0032fb900951eso4228620wmd.4 for ; Fri, 29 Oct 2021 05:16:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=gCIPkXfIa81Ejq7ah/bOghlR0pwQx6vz4dpfxpLTQ1E=; b=MP4tKB8LkG7BmE0Gs4xcSTJM2Vifwyer2O3AoUl5qTr+PivZWl2v9z5nCsFoCGHzfU NovFcS7TAF4Fn9SNXI5ydC8tUi85mAsSnoX1D56nZTdGB5/vdPDvOEanWC9JR5SsvyX4 6tbIdlcXyGY2y2dKicj85OWfB+abK26D/1VXvdoi+BA2vc79Hh70DW4ByEBKz2pwaskK TZPE1QKYl5v0yeXWXUy3lOjt5SD5/lEVDVipGsvxAy1l0SrWIShF6K1Y/oqmhNjdQPUi 0dhEmmMZW/A+NIRd32AMzqgB/umxihr+bUXXSBCPYaUE+OJstX/uGOqKjiJWuzkjQKdT +/hw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=gCIPkXfIa81Ejq7ah/bOghlR0pwQx6vz4dpfxpLTQ1E=; b=4yRT4NW7aJN6RQolfdM78maVAYlpSivVUDs68spZTFXgxQKzX/r2iicAYs9QHEILSu m8iRBY8JoNMkd+2w3C4KZSC6PkojT3Uv50RedKZX5YbcZUGBayRKLdwbmOK4xY+46FT2 5/AjFRKi/3b+gGEehxXtjWu4+7c8p9IHVSb7hTsvEqaqqgkZqNGDcCeAN0RpWOlRiM4d dhQY8+fo4tJ7Y1eDIqNTRRQHN8QdmF1eL7jp5puq5HaP/+hCJmbI5dRPOGbdjrf6+ltI XpCzJxvIKADr3fR9ue8DNukj92ZptPGlXoKwkVrDNEm6gPTWWLRfqZidO0Ru8q7M+10K YcZg== X-Gm-Message-State: AOAM532f8SYBMpEOwvqN+DzKw8q2gioHMhPAjG0rq2+tYXArt0Ow9QnI kRh0tSmDEUcCxF+1AAYKElg7iIOpGgTdqQ== X-Google-Smtp-Source: ABdhPJwjg9nPBtPqNq6vO0I1U4V5EEMUojkQcezip7iqhhRPPUHxdj1h1ee3oCOy16PFN0jmwN9fqg== X-Received: by 2002:a7b:cd91:: with SMTP id y17mr320668wmj.66.1635509787098; Fri, 29 Oct 2021 05:16:27 -0700 (PDT) Received: from gojira.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id g5sm2592095wmi.2.2021.10.29.05.16.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Oct 2021 05:16:26 -0700 (PDT) From: Olivier Matz To: dev@dpdk.org Cc: Olivier Matz , Lavanya Govindarajan , Reshma Pattan , David Marchand , stable@dpdk.org Date: Fri, 29 Oct 2021 14:15:44 +0200 Message-Id: <20211029121544.13298-1-olivier.matz@6wind.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH] test/mbuf: fix access to freed memory X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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: stable@dpdk.org Reported-by: David Marchand Signed-off-by: Olivier Matz --- 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