|WARNING| mem: fix fbarray placement colliding with PCI BARs

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Fri Oct 23 08:35:46 CEST 2026


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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

**Resource leak on error path:**

The error-path cleanup destroys fbarrays but does not free their backing memory. The `rte_fbarray_destroy()` call releases the fbarray structure but the underlying memory allocated by `eal_memseg_list_init()` for the fbarray data itself is not freed.

In `lib/eal/common/eal_common_memalloc.c`, `eal_memseg_list_init()` calls `rte_fbarray_init()` which allocates memory for the array. The destroy function must be paired with cleanup that frees this memory.

Suggested fix:
```c
/* destroy the segment lists created so far */
for (i = 0; i < n_msl; i++) {
	msl = &mcfg->memsegs[i];
	if (msl->base_va != NULL) {
		eal_mem_free(msl->base_va, msl->len);
		msl->base_va = NULL;
	}
	rte_fbarray_destroy(&msl->memseg_arr);
	memset(msl, 0, sizeof(*msl));
}
```

However, verify in the actual `eal_memseg_list_init()` implementation whether it allocates VA space for the memseg list itself (distinct from the VA space reserved later) or only creates the fbarray metadata. If it only creates metadata, check whether `rte_fbarray_destroy()` fully cleans up or if there's a separate allocation.

---

## WARNINGS

**Missing release notes:**

This patch fixes a crash in secondary processes (intermittently on arm64 with 64K pages, about 1 in 10 starts). A bug fix that resolves a crash affecting secondary process startup should be documented in the release notes (`doc/guides/rel_notes/release_XX_YY.rst`).

Add an entry under the "Fixed Issues" section describing the fix:
```rst
* **mem: Fixed fbarray placement colliding with PCI BARs.**

  Fixed an issue where memseg list fbarrays could be mapped immediately
  after the memseg VA space, causing PCI BAR mappings to collide with
  secondary process libraries or stacks. This intermittently crashed
  secondary processes during EAL init, especially on arm64 with 64K pages.
```

---

## FINAL CHECK

### Pass 1: Correctness bugs
- Traced error path for resource leaks: found fbarray cleanup may be incomplete
- Checked for use-after-free: none identified
- Verified error codes are propagated: correct (returns -1 on failure)

### Pass 2: Style/process false positives
No false positives to remove.

### Pass 3: Evidence verification
- Error finding: Line `rte_fbarray_destroy(&msl->memseg_arr);` in the new error path. The concern is that memory allocated by the init may not be fully freed. The line is in the added code block starting at `+		/* destroy the segment lists created so far */`.
- Warning finding: No release notes added in the patch.

Review-Result: ERROR


More information about the test-report mailing list