|WARNING| [v2, 6/6] net/iavf: fix leak of IPsec crypto capabilities array

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-07-03

# DPDK Patch Review

Reviewing patch series 'series.patch' containing 6 patches fixing memory leaks in the Intel iavf and ice network drivers.

---

## PATCH 1/6: net/iavf: fix local memory leaks in TM hierarchy commit

### Errors

**Memory leak on qtc_map allocation failure (line 894-897)**

When `qtc_map` allocation fails (line 894), the code correctly sets `ret_val = IAVF_ERR_NO_MEMORY` and jumps to `fail_clear` (line 896). However, at this point `q_bw` and `q_tc_mapping` have already been successfully allocated (lines 837, 845) but are not freed on this error path. The `fail_clear` label (line 918) only calls `iavf_tm_conf_uninit(dev)` and does not free these temporary buffers. The `free()` calls for `q_bw` and `q_tc_mapping` at line 925-926 are only reached via the `err:` label, not from `fail_clear`.

Suggested fix:
```c
fail_clear:
	/* if we have error before we commit, we do not need to clear */
	if (clear_on_fail) {
		iavf_tm_conf_uninit(dev);
		iavf_tm_conf_init(dev);
	}
	/* fall through to err label to free temporary allocations */
err:
	free(q_bw);
	free(q_tc_mapping);
	free(qtc_map);
	return ret_val;
```
Or move the `free()` calls before the `if (clear_on_fail)` block in `fail_clear:`.

---

## PATCH 2/6: net/iavf: fix leak of queue to traffic class mapping data

### Warnings

**Unnecessary `free(NULL)` in iavf_tm_conf_uninit (line 100-101)**

The added free calls at lines 100-101 are executed regardless of whether `vf->qtc_map` was allocated. While `free(NULL)` is safe per POSIX, the driver already handles this cleanup at other uninit sites (lines 2758-2759 in patch 1), making this second cleanup redundant. Consider verifying the intended lifecycle: if `qtc_map` is always freed at `iavf_uninit_vf()`, the `iavf_tm_conf_uninit()` call may be unnecessary or indicate double-free risk.

---

## PATCH 3/6: net/iavf: fix memory leak on error when adding flow parser

### No issues found

The fix correctly adds `rte_free(parser_node)` on the error path where `parser_node` is allocated but never added to a TAILQ.

---

## PATCH 4/6: net/ice: fix buffer leak in config of Tx queue TM node

### No issues found

The patch correctly moves the `free(buf)` call outside the error check so it executes unconditionally after the adminq call. The replacement of `ice_malloc(hw, buf_size)` with `calloc(1, buf_size)` is appropriate since this is a temporary buffer that does not need hugepage backing.

---

## PATCH 5/6: net/iavf: fix leak of flex metadata extraction field

### No issues found

The fix correctly adds `rte_free(vf->proto_xtr)` and NULL assignment in `iavf_uninit_vf()` to match the allocation in `iavf_init_proto_xtr()`.

---

## PATCH 6/6: net/iavf: fix leak of IPsec crypto capabilities array

### No issues found

The patch correctly frees `iavf_sctx->crypto_capabilities` before freeing the containing `iavf_sctx` structure in the security context destroy path.

---

## Summary

- **Patch 1** has a correctness bug: the `qtc_map` allocation failure path does not free `q_bw` and `q_tc_mapping`.
- **Patch 2** has a minor warning about potential redundant cleanup, but is not a correctness issue.
- **Patches 3-6** are correct.

All patches correctly use `free()` for memory allocated with `calloc()` and `rte_free()` for memory allocated with `rte_malloc()`/`rte_zmalloc()`, following DPDK conventions.