|SUCCESS| net/af_packet: move size check to after vlan insert
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Thu Oct 29 00:22:23 CET 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172235
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-28
# DPDK Patch Review
## ERRORS
None identified.
## WARNINGS
None identified.
## INFO
**Vlan insertion error path leaks modified mbuf**
```c
if (mbuf->ol_flags & RTE_MBUF_F_TX_VLAN) {
if (rte_vlan_insert(&mbuf)) {
rte_pktmbuf_free(mbuf);
continue;
}
}
```
`rte_vlan_insert()` may modify `mbuf` (reallocate with headroom) before failing.
If it returns non-zero, the `mbuf` pointer may now point to a new mbuf,
and the original mbuf (if it was reallocated and the realloc failed partway)
could be leaked, or the new mbuf (if realloc succeeded but a later step failed)
is what gets freed.
This is **pre-existing code** -- the patch does not introduce or change this logic.
The patch only moves an unrelated size check.
Worth verifying `rte_vlan_insert()` semantics:
does it guarantee the `mbuf` pointer is valid (either unchanged or successfully reallocated)
when it returns an error, or can it leave `mbuf` pointing to freed memory?
If the latter, the `rte_pktmbuf_free(mbuf)` on error is a use-after-free.
Checking `lib/net/rte_vlan.h` implementation would confirm.
This note is for whoever works on af_packet Tx error handling next.
---
**Patch correctness**
The moved size check is now applied **after** vlan insertion,
which is correct: the check must validate the final packet size,
not the pre-insertion size.
A packet at exactly `frame_data_size` before insertion
would exceed the limit after adding 4 bytes of VLAN tag and would be silently transmitted truncated
or cause ring corruption.
The fix is sound.
Review-Result: CLEAN
More information about the test-report
mailing list