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

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> -----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#n130

I think it is cleaner to use the MACRO instead of manually doing the loop,
linked-list iter + delete is error prone enough already :)