|WARNING| [v2,2/2] net/bnxt: remove VLA from EM key block swap
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Tue Oct 13 20:27:06 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171206
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-13
# DPDK Patch Review
## Patch 1/2: net/bnxt: cap memcpy at field size
### Errors
**Unreachable code after clamping:**
```c
if (unlikely(size > RTE_PARSER_FLOW_HDR_FIELD_SIZE)) {
BNXT_DRV_DBG(ERR, "Field size %u exceeds max of %u, clamping",
size, RTE_PARSER_FLOW_HDR_FIELD_SIZE);
size = RTE_PARSER_FLOW_HDR_FIELD_SIZE;
}
field->size = size;
memcpy(field->spec, buffer, field->size);
```
After clamping `size` to 16 bytes, the `memcpy()` at line 75 still executes with the clamped size. If the caller passed a buffer smaller than 16 bytes and `size > 16`, this reads past the end of `buffer`. The commit message says "Clamp the size to the field storage and log an error" but does not say the function succeeds -- an error was logged, yet the function continues and may corrupt memory. This should return an error code (`-EINVAL`) instead of clamping and continuing.
The same issue occurs in `ulp_rte_prsr_fld_mask()` at lines 114-120: size is clamped, an error is logged, then the function continues to use the clamped size without returning an error or verifying the caller's buffer is large enough.
## Patch 2/2: net/bnxt: remove VLA from EM key block swap
### CLEAN
The VLA replacement with an in-place swap is correct. The nested loop at lines 411-417 swaps corresponding bytes between the low and high blocks without requiring a temporary buffer. This is a valid improvement.
---
**Summary:**
Patch 1/2 introduces a correctness bug: after detecting an out-of-bounds size, it clamps the value and continues, which may cause `memcpy()` to read past the end of the caller's buffer if the buffer is smaller than the clamped size. The function should return an error when the size exceeds the maximum, not silently clamp and proceed.
Patch 2/2 is correct and introduces no issues.
Review-Result: ERROR
More information about the test-report
mailing list