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

[Robert Sanford]

From: Robert Sanford <rsanford at 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 at dpdk.org

Signed-off-by: Robert Sanford <rsanford at akamai.com>
Reviewed-by: Morten Brørup <mb at smartsharesystems.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