[PATCH v1] common/idpf: fix heap use after free error

[PATCH v1] common/idpf: fix heap use after free error

[Praveen Shetty]

Heap use after free error is detected in AddressSanitizer while quitting
the testpmd application.Issue is due to accessing the empty control
queue in the idpf_ctlq_deinit function.idpf_ctlq_deinit function is called
during the rte_eal_cleanup routine.
This patch will fix this issue.

Fixes: fb4ac04e9bfa ("common/idpf: introduce common library")
Cc: stable@dpdk.org

Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
---
 drivers/common/idpf/base/idpf_controlq.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/common/idpf/base/idpf_controlq.c b/drivers/common/idpf/base/idpf_controlq.c
index 4f47759a4f..8f404d3083 100644
--- a/drivers/common/idpf/base/idpf_controlq.c
+++ b/drivers/common/idpf/base/idpf_controlq.c
@@ -248,9 +248,10 @@ int idpf_ctlq_init(struct idpf_hw *hw, u8 num_q,
 	return 0;
 
 init_destroy_qs:
-	LIST_FOR_EACH_ENTRY_SAFE(cq, tmp, &hw->cq_list_head,
-				 idpf_ctlq_info, cq_list)
+	while (!LIST_EMPTY(&hw->cq_list_head)) {
+		cq = LIST_FIRST(&hw->cq_list_head);
 		idpf_ctlq_remove(hw, cq);
+	}
 
 	return err;
 }
@@ -263,9 +264,10 @@ void idpf_ctlq_deinit(struct idpf_hw *hw)
 {
 	struct idpf_ctlq_info *cq = NULL, *tmp = NULL;
 
-	LIST_FOR_EACH_ENTRY_SAFE(cq, tmp, &hw->cq_list_head,
-				 idpf_ctlq_info, cq_list)
+	while (!LIST_EMPTY(&hw->cq_list_head)) {
+		cq = LIST_FIRST(&hw->cq_list_head);
 		idpf_ctlq_remove(hw, cq);
+	}
 }
 
 /**
-- 
2.34.1

Re: [PATCH v1] common/idpf: fix heap use after free error

[Stephen Hemminger]

On Mon, 13 Jan 2025 08:54:04 +0000
Praveen Shetty <praveen.shetty@intel.com> wrote:

> Heap use after free error is detected in AddressSanitizer while quitting
> the testpmd application.Issue is due to accessing the empty control
> queue in the idpf_ctlq_deinit function.idpf_ctlq_deinit function is called
> during the rte_eal_cleanup routine.
> This patch will fix this issue.
> 
> Fixes: fb4ac04e9bfa ("common/idpf: introduce common library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>

This should not be needed. LIST_FOR_EACH_ENTRY_SAFE part, don't understand.

Re: [PATCH v1] common/idpf: fix heap use after free error

[Bruce Richardson]

On Mon, Jan 13, 2025 at 08:30:01AM -0800, Stephen Hemminger wrote:
> On Mon, 13 Jan 2025 08:54:04 +0000
> Praveen Shetty <praveen.shetty@intel.com> wrote:
> 
> > Heap use after free error is detected in AddressSanitizer while quitting
> > the testpmd application.Issue is due to accessing the empty control
> > queue in the idpf_ctlq_deinit function.idpf_ctlq_deinit function is called
> > during the rte_eal_cleanup routine.
> > This patch will fix this issue.
> > 
> > Fixes: fb4ac04e9bfa ("common/idpf: introduce common library")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> 
> This should not be needed. LIST_FOR_EACH_ENTRY_SAFE part, don't understand.

I would tend to agree. Is there an actual confirmed bug here? If so, then
either our standard list macros are broken, or the code using them is doing
something rather strange.

/Bruce