|WARNING| [5/5] net/bnxt: fix VF info alloc error path memory leak

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/169791

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-18

# DPDK Patch Review

## PATCH 1/5: net/bnxt: add VF ID boundary check before usage

### Errors

None.

### Warnings

None.

### Info

**Purpose and correctness:**
This patch fixes a **correctness bug** (out-of-bounds array access) by moving the VF ID range check before the index computation and `bp->pf->vf_info[vf_id]` dereference. The fix is sound: firmware-controlled `source_id` is now validated before being used as an array index.

**Error path handling:**
The patch correctly guards `fwd_cmd->req_type` dereference in the error log by checking `fwd_cmd != NULL`, and guards `bnxt_hwrm_reject_fwd_resp()`'s `memcpy()` against a NULL `encaped` pointer. Both are appropriate since the early reject path now sets `fwd_cmd = NULL` and `req_len = 0` before reaching those uses.

**Release notes:**
This is a backported fix for a potential memory corruption bug. The `Fixes:` and `Cc: stable@` tags are present, which is correct for this type of change. No release notes entry is required for a stable backport.

---

## PATCH 2/5: net/bnxt: fix bounds on firmware-reported resource counts

### Errors

None.

### Warnings

None.

### Info

**Purpose and correctness:**
This patch addresses **integer overflow** risks when firmware-supplied counts are used directly. The clamping of `max_ring_grps` at 65535 is correct (it is cast to `uint16_t` elsewhere in the code). The `max_l2_ctx` calculation widens the addition to `uint32_t` before clamping, which prevents the 16-bit overflow on the `bp->max_l2_ctx + bp->max_rx_em_flows` addition.

**Implementation:**
The use of `RTE_MIN()` with explicit casts is correct and matches DPDK idioms. The constants `BNXT_MAX_RING_GRPS` and `BNXT_MAX_L2_CTX` are appropriately defined in the header.

**Release notes:**
This is a backported fix (stable branch). The `Fixes:` and `Cc: stable@` tags are present. No release notes entry is required.

---

## PATCH 3/5: net/bnxt: fix use-after-free in VNIC filter cleanup

### Errors

None.

### Warnings

None.

### Info

**Purpose and correctness:**
This patch fixes a **use-after-free bug**. The original code used `STAILQ_FOREACH()`, which internally dereferences `filter->next` *after* the loop body runs. Since the loop body calls `bnxt_free_filter(filter)`, the `next` field is accessed after the memory has been freed.

The fix correctly replaces `STAILQ_FOREACH()` with a while-loop that uses `STAILQ_FIRST()` and `STAILQ_REMOVE_HEAD()`, removing each element from the list before freeing it. This is the standard DPDK pattern for freeing list elements during traversal.

**Release notes:**
This is a backported fix (stable branch). The `Fixes:` and `Cc: stable@` tags are present. No release notes entry is required.

---

## PATCH 4/5: net/bnxt: fix memory leak in VF VNIC query error path

### Errors

None.

### Warnings

None.

### Info

**Purpose and correctness:**
This patch fixes a **memory leak** on an error path. The function allocates `vnic_ids` via `rte_malloc()`, then calls `bnxt_hwrm_func_vf_vnic_query()`. If that call fails (returns negative), the original code returned immediately without freeing `vnic_ids`.

The fix adds `rte_free(vnic_ids)` on the error path before the early return. This is correct.

**Release notes:**
This is a backported fix (stable branch). The `Fixes:` and `Cc: stable@` tags are present. No release notes entry is required.

---

## PATCH 5/5: net/bnxt: fix VF info alloc error path memory leak

### Errors

None.

### Warnings

None.

### Info

**Purpose and correctness:**
This patch fixes an **error path resource leak** by moving the assignment `bp->pf->vf_info = vf_info` earlier in the allocation sequence. The issue is that if a nested allocation (e.g., `vf_info[i].vlan_table`) fails, the cleanup function `bnxt_free_vf_info()` needs `bp->pf->vf_info` to be set in order to walk the partially allocated array and free what was allocated so far.

The fix is correct: assigning `bp->pf->vf_info` immediately after allocating the `vf_info` array ensures that the cleanup path can find and free any per-VF allocations that succeeded before the failure.

**Release notes:**
This is a backported fix (stable branch). The `Fixes:` and `Cc: stable@` tags are present. No release notes entry is required.

---

## Summary

All five patches address **correctness bugs** (out-of-bounds access, integer overflow, use-after-free, memory leaks on error paths). The fixes are appropriate and follow DPDK error-handling patterns. No style, API, or documentation issues were found. All patches correctly target the stable branch with `Fixes:` and `Cc: stable@` tags.