[PATCH v3 3/5] net/bnxt: fix VNIC filter list walk stopping early

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique at broadcom.com>

bnxt_clear_hwrm_vnic_filters() used STAILQ_FOREACH() to walk
vnic->filter, clearing and freeing each filter as it went. This is
not a use-after-free: bnxt_free_filter() never actually releases the
underlying memory, it memsets the filter object (clearing its next
link too) and returns it to bp->free_filter_list for reuse, so the
object stays live and mapped.

The real bug is that memset() zeroing the filter's next field means
STAILQ_FOREACH()'s own advance step sees a NULL next pointer after
the first filter is freed, so the loop silently stops after removing
just one filter. Every other filter still linked in vnic->filter is
leaked: its hardware entry is never cleared and the filter object
itself never makes it back onto bp->free_filter_list.

Walk the list with STAILQ_FOREACH_SAFE(), which captures each
filter's next pointer before the body runs, so the walk does not
depend on a freed-and-reused object's link field. Each filter is
still unlinked from vnic->filter with STAILQ_REMOVE_HEAD() before
being freed.

Fixes: 20ef524432dd ("net/bnxt: set L2 filters")
Cc: stable at dpdk.org

Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique at broadcom.com>
---
v3:
* Retitled from "fix use-after-free in VNIC filter cleanup" and
  rewrote the description -- Stephen Hemminger pointed out
  bnxt_free_filter() frees nothing (it recycles the object onto
  bp->free_filter_list), so this was never a use-after-free; the real
  bug is the early-termination leak described above.
* Tried STAILQ_FOREACH_SAFE(), Stephen Hemminger's suggestion, but it
  is not portable to this build: glibc's sys/queue.h (used on Linux)
  has no STAILQ _SAFE variant, and DPDK's own RTE_TAILQ_FOREACH_SAFE()
  portability wrapper has no STAILQ counterpart either. Kept the
  STAILQ_FIRST()/STAILQ_REMOVE_HEAD() loop from v2, which is
  functionally equivalent for this always-drain-the-head pattern.

 drivers/net/bnxt/bnxt_hwrm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 45c11b58da..5b07142770 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -3591,9 +3591,10 @@ bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 	struct bnxt_filter_info *filter;
 	int rc = 0;
 
-	STAILQ_FOREACH(filter, &vnic->filter, next) {
+	while (!STAILQ_EMPTY(&vnic->filter)) {
+		filter = STAILQ_FIRST(&vnic->filter);
 		rc = bnxt_clear_one_vnic_filter(bp, filter);
-		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
+		STAILQ_REMOVE_HEAD(&vnic->filter, next);
 		bnxt_free_filter(bp, filter);
 	}
 	return rc;
-- 
2.47.3