> From: Robert Sanford [mailto:[email protected]] > Sent: Tuesday, 3 November 2026 02.11 > To: [email protected] > Cc: [email protected]; Morten Brørup; > [email protected]; Robert Sanford; [email protected] > Subject: [PATCH v1 1/3] mempool: fix race between list walk and free > > From: Robert Sanford <[email protected]> > > rte_mempool_walk(), rte_mempool_lookup() and rte_mempool_list_dump() > traverse the mempool list holding only the mempool list lock. > rte_mempool_create_empty() inserts into the list holding both the > mempool list lock and the tailq lock, but rte_mempool_free() unlinks > and frees the list entry holding only the tailq lock. So a mempool can > be unlinked and freed, together with its list entry, while another > thread is walking the list. The walker then reads freed memory. Since > freed heap memory is zeroed, the walk callback is passed a NULL > mempool, or rte_mempool_lookup() compares the name of one, and the > process crashes. > > Take the mempool list lock in rte_mempool_free() as well, in the same > order as rte_mempool_create_empty(). As a result, a rte_mempool_walk() > callback must not free a mempool, as it already could not create one. > Document this. > > Fixes: a3791ff0a3f8 ("mempool: introduce a function to free a pool") > Cc: [email protected] > > Signed-off-by: Robert Sanford <[email protected]> > --- > lib/mempool/rte_mempool.c | 8 ++++++++ > lib/mempool/rte_mempool.h | 3 +++ > 2 files changed, 11 insertions(+) > > diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c > index 069aabaa70..d9f339a3c2 100644 > --- a/lib/mempool/rte_mempool.c > +++ b/lib/mempool/rte_mempool.c > @@ -729,6 +729,13 @@ rte_mempool_free(struct rte_mempool *mp) > return; > > mempool_list = RTE_TAILQ_CAST(rte_mempool_tailq.head, > rte_mempool_list); > + /* > + * Take the mempool list lock as well, like > rte_mempool_create_empty(), > + * so that the entry is not unlinked and freed while > + * rte_mempool_walk(), rte_mempool_lookup() or > rte_mempool_list_dump() > + * is traversing the list. > + */ > + rte_mcfg_mempool_write_lock(); > rte_mcfg_tailq_write_lock(); > /* find out tailq entry */ > TAILQ_FOREACH(te, mempool_list, next) { > @@ -741,6 +748,7 @@ rte_mempool_free(struct rte_mempool *mp) > rte_free(te); > } > rte_mcfg_tailq_write_unlock(); > + rte_mcfg_mempool_write_unlock(); > > mempool_event_callback_invoke(RTE_MEMPOOL_EVENT_DESTROY, mp); > rte_mempool_trace_free(mp); > diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h > index b34cd893b1..bd58d8a9d5 100644 > --- a/lib/mempool/rte_mempool.h > +++ b/lib/mempool/rte_mempool.h > @@ -1947,6 +1947,9 @@ uint32_t rte_mempool_calc_obj_size(uint32_t > elt_size, uint32_t flags, > /** > * Walk list of all memory pools > * > + * The mempool list is locked while the iterator function runs, so the > + * iterator function must not create or free a mempool. > + * > * @param func > * Iterator function > * @param arg > -- > 2.34.1
Reviewed-by: Morten Brørup <[email protected]> Verified that rte_mempool_create_empty() takes the mcfg_mempool and mcfg_tailq locks in the same order as this patch.
