diff mbox series

[2/3] eal/freebsd: Do not index out of bounds in memseg list

Message ID 20250506175010.1141585-3-jfree@FreeBSD.org (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series EAL memory fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jake Freeland May 6, 2025, 5:50 p.m. UTC 
It is possible for rte_fbarray_find_next_n_free() to misreport that there
are n contiguous open spots. If we need two contiguous entries for a
hole, make sure that we're not indexing out-of-bounds in the fbarray.

The `arr->len - arr->count < n` condition in fbarray_find_n() is meant to
safeguard against this, but we are not updating arr->count when inserting
holes, so an undesired index may be returned.

Signed-off-by: Jake Freeland <jfree@FreeBSD.org>
---
 lib/eal/freebsd/eal_memory.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Burakov, Anatoly May 8, 2025, 11:20 a.m. UTC | #1 
On 5/6/2025 7:50 PM, Jake Freeland wrote:
> It is possible for rte_fbarray_find_next_n_free() to misreport that there
> are n contiguous open spots. If we need two contiguous entries for a
> hole, make sure that we're not indexing out-of-bounds in the fbarray.
> 
> The `arr->len - arr->count < n` condition in fbarray_find_n() is meant to
> safeguard against this, but we are not updating arr->count when inserting
> holes, so an undesired index may be returned.
> 
> Signed-off-by: Jake Freeland <jfree@FreeBSD.org>

I don't quite get how this happens.

If we "need hole" that means the memseg list isn't empty, and previous 
segment is occupied. When we need hole, we request 2 free spots from the 
memseg. Let's assume there's only 2 free spots left in the memseg list, 
so we get ms_idx equal to (arr->len - 2).

We get ms_idx - that is, the index of where the free spot starts, and 
there's 2 spots guaranteed to be free at that point. We need a hole, so 
we increment ms_idx once to arrive at last free spot (arr->len - 1).

Where would the overflow come from?

(also, now that I think of it, I suspect we should not be starting our 
search from index 0 every time, because if we "don't need a hole" 
because the segment is adjacent to previous one, that means 
find_next_n(1) starting from 0 will find us a hole we left previously 
for some other segment - perhaps we should find first unoccupied spot 
from the end, and start the find_next_n search from there?)
diff mbox series

Patch

diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
index bcf5a6f986..bdbac0c3f3 100644
--- a/lib/eal/freebsd/eal_memory.c
+++ b/lib/eal/freebsd/eal_memory.c
@@ -171,6 +171,9 @@  rte_eal_hugepage_init(void)
 				    rte_fbarray_is_used(arr, ms_idx - 1))
 					ms_idx++;
 
+				if (ms_idx == (int)arr->len)
+					continue;
+
 				break;
 			}
 			if (msl_idx == RTE_MAX_MEMSEG_LISTS) {