Re: [dpdk-dev] [PATCH] net/softnic: fix memory illegal access

["Singh, Jasvinder" <jasvinder.singh@intel.com>]

> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Friday, July 20, 2018 11:32 AM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>; dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] net/softnic: fix memory illegal access
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jasvinder Singh
> > Sent: Friday, July 20, 2018 10:45 AM
> > To: dev@dpdk.org
> > Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > Subject: [dpdk-dev] [PATCH] net/softnic: fix memory illegal access
> >
> > While deleting the elements from the linked list, TAILQ_FOREACH causes
> > read from the freed pointer. Fixes the issue by using for loop instead
> > of TAILQ_FOREACH.
> >
> > Coverity issue: 302867
> > Fixes: bef50bcb1c47 ("net/softnic: implement start and stop")
> >
> > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > ---
> >  drivers/net/softnic/rte_eth_softnic_swq.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/softnic/rte_eth_softnic_swq.c
> > b/drivers/net/softnic/rte_eth_softnic_swq.c
> > index 1944fbb..a1f1899 100644
> > --- a/drivers/net/softnic/rte_eth_softnic_swq.c
> > +++ b/drivers/net/softnic/rte_eth_softnic_swq.c
> > @@ -36,9 +36,11 @@ softnic_swq_free(struct pmd_internals *p)  void
> > softnic_softnic_swq_free_keep_rxq_txq(struct pmd_internals *p)  {
> > -	struct softnic_swq *swq;
> > +	struct softnic_swq *swq, *swq_next;
> > +
> > +	for (swq = TAILQ_FIRST(&p->swq_list); swq != NULL; swq = swq_next) {
> > +		swq_next = TAILQ_NEXT(swq, node);
> >
> > -	TAILQ_FOREACH(swq, &p->swq_list, node) {
> >  		if ((strncmp(swq->name, "RXQ", strlen("RXQ")) == 0) ||
> >  			(strncmp(swq->name, "TXQ", strlen("TXQ")) == 0))
> >  			continue;
> 
> 
> The TAILQ_FOREACH_SAFE() macro handles exactly this case. Although it is not
> in the linux TAILQ header, DPDK provides it in rte_tailq.h:
> 
> http://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/rte_tailq.h#n13
> 0
> 
> I think it is cleaner to use the MACRO instead of manually doing the loop,
> linked-list iter + delete is error prone enough already :)

Macro lessens the confusion as well  :)  thanks, Harry!