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

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Jan 20, 2025 at 02:32:49PM +0000, Bruce Richardson wrote:
> 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.
> 

I followed up on with with Praveen, and he went through the code and
possible solutions with me. The issue flagged by ASAN is correct, because
it turns out that the version of the _SAFE macro provided in this
particular driver is not actually safe! :-(

There are therefore two options to fixing this: 1) fix the macro/use a
different copy of the macro, or 2) rework the code as in this patch and drop
the macro. Copies of the driver in other OS use the style given in this patch,
so we will go with the second option. However, we will do a v2 to include
the removal of the bad macro, alongside fixing this. That should hopefully
prevent this issue from reoccurring.

Praveen, will review v2 when you send it.

/Bruce