RE: [PATCH v1 1/3] mempool: fix race between list walk and free

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Robert Sanford [mailto:rsanford2@gmail.com]
> Sent: Tuesday, 3 November 2026 02.11
> To: dev@dpdk.org
> Cc: andrew.rybchenko@oktetlabs.ru; Morten Brørup;
> olivier.matz@6wind.com; Robert Sanford; stable@dpdk.org
> Subject: [PATCH v1 1/3] mempool: fix race between list walk and free
> 
> From: Robert Sanford <rsanford@akamai.com>
> 
> 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: stable@dpdk.org
> 
> Signed-off-by: Robert Sanford <rsanford@akamai.com>
> ---
>  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 <mb@smartsharesystems.com>

Verified that rte_mempool_create_empty() takes the mcfg_mempool and mcfg_tailq locks in the same order as this patch.