|WARNING| [v3, 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/166674

_AI Code Review_

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

# DPDK Patch Review

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

### Errors

None.

### Warnings

None.

### Info

The patch correctly identifies and fixes resource leaks on multiple error paths by:
- Initializing pointers to NULL to enable safe cleanup
- Using `calloc()` instead of `rte_zmalloc()` (appropriate for control path data not needing hugepage memory)
- Freeing `q_tc_mapping` and `q_bw` on both success and error paths
- Adding missing error propagation for `qtc_map` allocation failure (previously returned `IAVF_ERR_NO_MEMORY` without cleanup)

The changes are correct and follow good practice for error path resource management.

---

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

### Errors

None.

### Warnings

None.

### Info

The patch correctly fixes a resource leak by:
- Using `calloc()` instead of `rte_zmalloc()` (appropriate for control path data)
- Freeing `qtc_map` on error path
- Freeing old `qtc_map` when replacing with new one (the `old_qtc_map` pointer prevents use-after-free during the replacement)
- Adding cleanup in `iavf_tm_conf_uninit()` and `iavf_uninit_vf()`

The pattern of saving the old pointer before assignment and freeing it after successful assignment is correct.

---

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

### Errors

None.

### Warnings

None.

### Info

The patch correctly fixes a resource leak by freeing `parser_node` in the error path where it was allocated but never added to any TAILQ.

---

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

### Errors

None.

### Warnings

None.

### Info

The patch correctly fixes a resource leak by:
- Moving `free(buf)` to execute unconditionally after the `ice_aq_move_recfg_lan_txq()` call
- Using `calloc()` instead of `ice_malloc()` (appropriate since this is not performance-critical and doesn't need hugepage memory)

The error path still returns correctly after the free.

---

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

### Errors

None.

### Warnings

None.

### Info

The patch correctly fixes a resource leak by:
- Adding `rte_free(vf->proto_xtr)` in both `iavf_uninit_vf()` (error path) and `iavf_dev_close()` (normal shutdown path)
- Setting pointer to NULL after free (defensive coding)

The use of `rte_free()` is appropriate here since `iavf_init_proto_xtr()` uses `rte_zmalloc()`.

---

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

### Errors

None.

### Warnings

None.

### Info

The patch correctly fixes a resource leak by freeing `iavf_sctx->crypto_capabilities` before freeing the security context itself in `iavf_security_ctx_destroy()`.

---

## Summary

All six patches correctly identify and fix resource leaks on error paths and/or normal shutdown paths. The changes demonstrate proper error path tracing and resource cleanup discipline:

- Each allocated resource is freed on all paths (success, error, shutdown)
- Pointers are set to NULL after free (defensive)
- Appropriate memory allocator used for each context (hugepage vs standard malloc)
- No use-after-free, double-free, or missing cleanup issues introduced

**Overall assessment: No issues found. All patches are correct.**