On Thu, Jan 23, 2025 at 12:18 PM Bruce Richardson
<bruce.richard...@intel.com> wrote:
>
> 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.she...@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: sta...@dpdk.org
> > > >
> > > > Signed-off-by: Praveen Shetty <praveen.she...@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.
Sorry, I am not following.
1) seems the best way as it does not require touching base driver code.
Afaiu, the LIST_FOR_EACH_ENTRY_SAFE macro is defined in the
"abstraction" header that is DPDK specific
(drivers/common/idpf/base/idpf_osdep.h).
There is already an implementation of LIST_FOR_EACH_ENTRY_SAFE in
driver/net/ice/base/ice_osdep.h.
(note that it may be worth providing such a macro in a common place in
DPDK and remove copies of it in various drivers).
--
David Marchand
