From eb5ab924b5b9e517e71c1894d3efde6f2d56efe8 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Wed, 3 Jun 2026 15:57:29 +0000 Subject: [PATCH 1/2] DAOS-19019 spdk: free opts.pci_allowed after spdk_env_init() opts_add_pci_addr() allocates opts.pci_allowed via realloc() but daos_spdk_init() never frees it after calling spdk_env_init(). Add free(opts.pci_allowed) at the out: label so both the success and error paths release the buffer. Add test_daos_spdk_init_pci_freed to nvme_control_ut.c to verify under ASan that opts.pci_allowed is freed on return. Also fix the mock_spdk_nvme_ctrlr_get_pci_device() mock to use a static local pool instead of calloc(), which was itself leaking under ASan. Signed-off-by: Cedric Koch-Hofer --- src/control/lib/spdk/ctests/nvme_control_ut.c | 31 +++++++++++-------- src/control/lib/spdk/src/nvme_control.c | 2 ++ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/control/lib/spdk/ctests/nvme_control_ut.c b/src/control/lib/spdk/ctests/nvme_control_ut.c index 94eb5745ab7..31505257cb2 100644 --- a/src/control/lib/spdk/ctests/nvme_control_ut.c +++ b/src/control/lib/spdk/ctests/nvme_control_ut.c @@ -17,12 +17,16 @@ #include "nvme_internal.h" #include "../include/nvme_control_common.h" +#include "../include/nvme_control.h" #if D_HAS_WARNING(4, "-Wframe-larger-than=") #pragma GCC diagnostic ignored "-Wframe-larger-than=" #endif +#define MAX_MOCK_PCI_DEVS 16 + static struct ret_t *test_ret; +static int mock_pci_dev_count; /** * ============================== @@ -85,15 +89,18 @@ mock_copy_ctrlr_data(struct nvme_ctrlr_t *ctrlr, const struct spdk_nvme_ctrlr_da return 0; } +/* + * Return a unique device per call from a static local pool (indexed by + * mock_pci_dev_count, reset in teardown). + */ static struct spdk_pci_device * mock_spdk_nvme_ctrlr_get_pci_device(struct spdk_nvme_ctrlr *ctrlr) { - struct spdk_pci_device *dev; - - dev = calloc(1, sizeof(struct spdk_pci_device)); + static struct spdk_pci_device devs[MAX_MOCK_PCI_DEVS]; (void)ctrlr; - return dev; + assert_true(mock_pci_dev_count < MAX_MOCK_PCI_DEVS); + return &devs[mock_pci_dev_count++]; } static int @@ -291,6 +298,7 @@ teardown(void **state) free(test_ret); test_ret = NULL; cleanup(false); + mock_pci_dev_count = 0; return 0; } @@ -299,15 +307,12 @@ int main(void) { const struct CMUnitTest tests[] = { - cmocka_unit_test_setup_teardown(test_discover_null_controllers, - setup, teardown), - cmocka_unit_test_setup_teardown(test_discover_set_controllers, - setup, teardown), - cmocka_unit_test_setup_teardown(test_discover_probe_fail, setup, - teardown), - cmocka_unit_test_setup_teardown(test_collect, setup, teardown), - cmocka_unit_test_setup_teardown(test_get_controller, setup, - teardown), + cmocka_unit_test_setup_teardown(test_discover_null_controllers, setup, teardown), + cmocka_unit_test_setup_teardown(test_discover_set_controllers, setup, teardown), + cmocka_unit_test_setup_teardown(test_discover_probe_fail, setup, teardown), + cmocka_unit_test_setup_teardown(test_collect, setup, teardown), + cmocka_unit_test_setup_teardown(test_get_controller, setup, teardown), + cmocka_unit_test_setup_teardown(test_daos_spdk_init_pci_freed, setup, teardown), }; return cmocka_run_group_tests_name("control_nvme_control_ut", tests, diff --git a/src/control/lib/spdk/src/nvme_control.c b/src/control/lib/spdk/src/nvme_control.c index d609e485726..bc2b0019ab5 100644 --- a/src/control/lib/spdk/src/nvme_control.c +++ b/src/control/lib/spdk/src/nvme_control.c @@ -1,5 +1,6 @@ /** * (C) Copyright 2018-2022 Intel Corporation. + * (C) Copyright 2026 Hewlett Packard Enterprise Development LP * (C) Copyright 2025 Google LLC * * SPDX-License-Identifier: BSD-2-Clause-Patent @@ -522,6 +523,7 @@ daos_spdk_init(int mem_sz, char *env_ctx, size_t nr_pcil, char **pcil) } out: + free(opts.pci_allowed); ret->rc = rc; return ret; } From 99e6fa3a4d0d1647a7af1440b6967e1e97d3b621 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Wed, 3 Jun 2026 15:57:41 +0000 Subject: [PATCH 2/2] DAOS-19019 spdk: free pci_addr in free_ctrlr_fields() _collect() allocates ctrlr->pci_addr via calloc() for each controller, but free_ctrlr_fields() freed all other heap fields (model, serial, fw_rev, vendor_id, pci_type) while omitting pci_addr. Add the missing free(). Add test_collect_pci_addr_freed to nvme_control_ut.c to verify under ASan that pci_addr is freed by teardown via clean_ret() -> free_ctrlr_fields(). Signed-off-by: Cedric Koch-Hofer --- src/control/lib/spdk/ctests/nvme_control_ut.c | 41 +++++++++++++++++++ .../lib/spdk/src/nvme_control_common.c | 2 + 2 files changed, 43 insertions(+) diff --git a/src/control/lib/spdk/ctests/nvme_control_ut.c b/src/control/lib/spdk/ctests/nvme_control_ut.c index 31505257cb2..ac5727af821 100644 --- a/src/control/lib/spdk/ctests/nvme_control_ut.c +++ b/src/control/lib/spdk/ctests/nvme_control_ut.c @@ -281,6 +281,46 @@ test_get_controller(void **state) assert_return_code(rc, 0); } +/* + * This test should be run with ASan to detect the memory leak fixed by DAOS-19019. It verifies + * that the opts.pci_allowed buffer built by opts_add_pci_addr() is freed before daos_spdk_init() + * returns. + */ +static void +test_daos_spdk_init_pci_freed(void **state) +{ + char *pcil[] = {"0000:01:00.0"}; + + (void)state; + + test_ret = daos_spdk_init(0, NULL, 1, pcil); + /* spdk_env_init() is expected to fail in a unit-test context (no hugepages) */ + assert_non_null(test_ret); +} + +/* + * This test should be run with ASan to detect the memory leak fixed by DAOS-19019. It verifies + * that pci_addr is freed by teardown via clean_ret() -> free_ctrlr_fields(). + */ +static void +test_collect_pci_addr_freed(void **state) +{ + (void)state; + + /* + * Inject two mock controllers and drive _collect() via function pointers instead of real + * SPDK calls. + */ + attach_mock_controllers(); + test_ret = init_ret(); + _collect(test_ret, &mock_copy_ctrlr_data, &mock_spdk_nvme_ctrlr_get_pci_device, + &mock_spdk_pci_device_get_numa_id, &mock_spdk_pci_device_get_type); + assert_int_equal(test_ret->rc, 0); + assert_non_null(test_ret->ctrlrs); + assert_non_null(test_ret->ctrlrs->pci_addr); + assert_non_null(test_ret->ctrlrs->next->pci_addr); +} + static int setup(void **state) { @@ -313,6 +353,7 @@ main(void) cmocka_unit_test_setup_teardown(test_collect, setup, teardown), cmocka_unit_test_setup_teardown(test_get_controller, setup, teardown), cmocka_unit_test_setup_teardown(test_daos_spdk_init_pci_freed, setup, teardown), + cmocka_unit_test_setup_teardown(test_collect_pci_addr_freed, setup, teardown), }; return cmocka_run_group_tests_name("control_nvme_control_ut", tests, diff --git a/src/control/lib/spdk/src/nvme_control_common.c b/src/control/lib/spdk/src/nvme_control_common.c index 11bd00b4bc4..ac0d8eaea88 100644 --- a/src/control/lib/spdk/src/nvme_control_common.c +++ b/src/control/lib/spdk/src/nvme_control_common.c @@ -138,6 +138,8 @@ free_ctrlr_fields(struct nvme_ctrlr_t *ctrlr) free(ctrlr->vendor_id); if (ctrlr->pci_type != NULL) free(ctrlr->pci_type); + if (ctrlr->pci_addr != NULL) + free(ctrlr->pci_addr); } void