|WARNING| net/bnxt: add bounds checks for FW-supplied IDs

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-06

# DPDK Patch Review

## Summary
This patch adds bounds checking for firmware-supplied IDs in the bnxt driver's port database update function. The changes prevent potential out-of-bounds array accesses when firmware provides invalid IDs.

---

## Errors

### 1. Missing bounds check on first use of `func->phy_port_id`

**Location:** Lines 162-180 (first use of `func` after assignment at line 167)

The code uses `func->phy_port_id` at line 180 without validation, but only checks it at line 219 (after the second assignment to `func`). Between lines 167-180, `func` points to `port_db->ulp_func_id_tbl[intf->drv_func_id]`, and `func->phy_port_id` is read from firmware data. This value could be out of bounds when used at line 213 (`bnxt_pmd_get_iface_mac`) and line 219 (`port_db->phy_port_list[func->phy_port_id]`).

**Fix:** Add the `phy_port_id` bounds check immediately after the first `func` assignment (after line 167, before line 180 where it's first used):

```c
func = &port_db->ulp_func_id_tbl[intf->drv_func_id];
if (func->phy_port_id >= port_db->phy_port_cnt) {
	BNXT_DRV_DBG(ERR, "Invalid phy_port_id %d", func->phy_port_id);
	return -EINVAL;
}
if (!func->func_valid) {
	/* existing code */
```

### 2. Unchecked `phy_port_id` when `intf->type` is VF_REP

**Location:** Lines 188-195 (VF_REP case)

When `intf->type` is `BNXT_ULP_INTF_TYPE_VF_REP`, the code reassigns `func` to point to `port_db->ulp_func_id_tbl[intf->vf_func_id]` at line 193. This new `func` has its own `phy_port_id` field (from firmware), which is later used at lines 213 and 219 without validation. Only the first `func->phy_port_id` (from `drv_func_id`) is checked at line 219.

**Fix:** Add a second `phy_port_id` bounds check after assigning `func` for the VF_REP case:

```c
func = &port_db->ulp_func_id_tbl[intf->vf_func_id];
if (func->phy_port_id >= port_db->phy_port_cnt) {
	BNXT_DRV_DBG(ERR, "Invalid VF phy_port_id %d", func->phy_port_id);
	return -EINVAL;
}
func->func_svif = /* existing code */
```

---

## Warnings

None.

---

## Info

### Code structure observation

The function updates the `func` pointer twice (once for `drv_func_id`, once conditionally for `vf_func_id`), but the final bounds check at line 219 only validates whichever `func->phy_port_id` is current at that point. This is error-prone. Consider validating `phy_port_id` immediately after each `func` assignment to make the validation logic local and obvious.

---

## Positive Findings

- The patch correctly identifies a real security issue (untrusted firmware data used as array index)
- The added bounds checks use the correct comparison (`>=` against array size)
- Error messages are clear and identify which field failed validation
- The checks occur before the values are used as array indices