|WARNING| [7/7] net/ice: fix ACL profile segment leak
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Tue Oct 27 12:49:32 CET 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172158
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-27
# DPDK Patch Review
## PATCH 1/7: net/ice: fix DDP package double free on init failure
### Errors
**Double-free on error path:**
When `ice_init_hw_tbls()` fails, the patch changes `ice_free(hw, hw->pkg_copy)` to `ice_free_seg(hw)`.
However, the code does not return after this error -- it falls through to the next check.
If safe mode is disabled (`!ad->devargs.safe_mode_support`),
the error path at `err_init_fw:` calls `ice_deinit_hw(hw)`, which will call `ice_free_seg()` again
through `ice_free_hw_tbls()` (which calls `ice_deinit_hw_tbls()`, which calls `ice_free_seg()`).
The patch converts a single-free into a potential double-free unless safe mode is enabled.
The fix needs either an early return after `ice_free_seg(hw)`,
or to skip `ice_free_seg()` in the `err_init_fw` path when it has already been called.
Line 2788 (added):
```c
ice_free_seg(hw);
```
This is freed again at the `err_init_fw:` label through `ice_deinit_hw()`.
---
## PATCH 2/7: net/ice: fix memory leaks on port close
### Errors
None.
### Warnings
None.
### Info
The patch adds cleanup of filter management structures, DDP package, and VSI contexts on close.
The added calls match the corresponding allocations in `ice_dev_init()` and `ice_pf_setup()`.
Setting `hw->switch_info = NULL` after `ice_cleanup_fltr_mgmt_struct(hw)` is defensive
against a potential double-free if `ice_dev_close()` were called twice,
though the ethdev API does not permit this.
---
## PATCH 3/7: net/ice: fix DDP filename leak
### Errors
None.
### Warnings
None.
### Info
The patch correctly frees the previous `ddp_filename` before storing a new one
(handling the "devarg given more than once" case),
and frees it on all error paths after devargs parsing.
The `NULL` assignment after free is defensive and correct.
---
## PATCH 4/7: net/ice: fix main VSI leak on init failure
### Errors
**Resource leak remains on earlier error paths:**
The patch adds `ice_release_vsi(pf->main_vsi)` at a new `err_release_vsi:` label,
but this label is only reached from one error path (after `ice_send_driver_ver()` fails).
There are earlier error paths after `ice_pf_setup()` that also need to release the main VSI:
- `ice_load_pkg()` failure (line 2802)
- `ice_init_hw_tbls()` failure (line 2789, after the fix in patch 1)
These paths goto `err_pf_setup`, which calls `ice_res_pool_destroy(&pf->msix_pool)`
but does not release the VSI created by `ice_pf_setup()` at line 2821.
Lines 2802 and 2789 (pre-existing after patch 1):
```c
goto err_pf_setup;
```
The VSI is allocated at line 2821 by `ice_pf_setup()` but not freed on these error paths.
---
## PATCH 5/7: net/ice: fix MAC and VLAN filter leak on VSI release
### Errors
None.
### Warnings
None.
### Info
The patch adds cleanup of MAC and VLAN filter entries that `ice_remove_all_mac_vlan_filters()`
left behind due to early returns or HW removal failures.
The use of `RTE_TAILQ_FOREACH_SAFE` is correct for freeing list entries during traversal.
---
## PATCH 6/7: net/ice: fix FDIR resource leak on setup failure
### Errors
None.
### Warnings
None.
### Info
The patch adds cleanup of the filter list and counter pools on all error paths in `ice_fdir_setup()`.
This matches the corresponding allocations and ensures `ice_fdir_teardown()` can be called safely
even after a failed setup (since `fdir_vsi` is reset to NULL on failure).
---
## PATCH 7/7: net/ice: fix ACL profile segment leak
### Errors
None.
### Warnings
None.
### Info
The patch frees the flow segment buffer after `ice_flow_add_prof()` succeeds,
matching the existing free on the error path.
`ice_flow_add_prof()` copies the segment data, so the buffer is no longer needed.
---
## Summary
### PATCH 1/7
The fix introduces a potential double-free.
After calling `ice_free_seg(hw)` on line 2788, the code must either return immediately
or ensure the `err_init_fw:` path does not call `ice_deinit_hw(hw)`
(which will free the segment again through `ice_free_hw_tbls()`).
### PATCH 4/7
The patch fixes one VSI leak but leaves two others:
the error paths at lines 2789 and 2802 (after `ice_init_hw_tbls()` and `ice_load_pkg()` fail)
also need to goto `err_release_vsi` instead of `err_pf_setup`.
---
Review-Result: ERROR
More information about the test-report
mailing list