From: Robert Sanford <[email protected]>

rte_mempool_create_empty() adds a mempool to the mempool list before
the mempool has a store. The store is allocated, and
RTE_MEMPOOL_F_POOL_CREATED is set, by the first populate call. In
between, rte_mempool_walk() passes the mempool to its callback, and
rte_mempool_list_dump() calls rte_mempool_dump() on it, which reads
the number of objects in the store. With the ring driver this
dereferences a NULL ring, and the process crashes.

Skip mempools that do not have RTE_MEMPOOL_F_POOL_CREATED set in
rte_mempool_walk() and rte_mempool_list_dump(), and document it. The
populating thread sets the flag without holding a lock, so add a
release fence before setting it and an acquire fence after checking
it. A walker that sees the flag then also sees the store.

rte_mempool_lookup() is unchanged. It does not access the store, and
callers may look up a mempool that is not populated yet.

Fixes: 449c49b93a6b ("mempool: support handler operations")
Cc: [email protected]

Signed-off-by: Robert Sanford <[email protected]>
---
 lib/mempool/rte_mempool.c | 20 +++++++++++++++++++-
 lib/mempool/rte_mempool.h |  3 +++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index d9f339a3c2..7c4d4697c7 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -304,6 +304,12 @@ mempool_ops_alloc_once(struct rte_mempool *mp)
                ret = rte_mempool_ops_alloc(mp);
                if (ret != 0)
                        return ret;
+               /*
+                * rte_mempool_walk() and rte_mempool_list_dump() access the
+                * store of pools with this flag set. Make the store visible
+                * before the flag.
+                */
+               rte_atomic_thread_fence(rte_memory_order_release);
                mp->flags |= RTE_MEMPOOL_F_POOL_CREATED;
        }
        return 0;
@@ -1412,6 +1418,11 @@ rte_mempool_list_dump(FILE *f)
 
        TAILQ_FOREACH(te, mempool_list, next) {
                mp = (struct rte_mempool *) te->data;
+               /* skip mempools that are not populated yet */
+               if ((mp->flags & RTE_MEMPOOL_F_POOL_CREATED) == 0)
+                       continue;
+               /* pairs with the release fence in mempool_ops_alloc_once() */
+               rte_atomic_thread_fence(rte_memory_order_acquire);
                rte_mempool_dump(f, mp);
        }
 
@@ -1460,7 +1471,14 @@ void rte_mempool_walk(void (*func)(struct rte_mempool *, 
void *),
        rte_mcfg_mempool_read_lock();
 
        RTE_TAILQ_FOREACH_SAFE(te, mempool_list, next, tmp_te) {
-               (*func)((struct rte_mempool *) te->data, arg);
+               struct rte_mempool *mp = (struct rte_mempool *) te->data;
+
+               /* skip mempools that are not populated yet */
+               if ((mp->flags & RTE_MEMPOOL_F_POOL_CREATED) == 0)
+                       continue;
+               /* pairs with the release fence in mempool_ops_alloc_once() */
+               rte_atomic_thread_fence(rte_memory_order_acquire);
+               (*func)(mp, arg);
        }
 
        rte_mcfg_mempool_read_unlock();
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index bd58d8a9d5..7782768a2d 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -1907,6 +1907,8 @@ static inline void *rte_mempool_get_priv(struct 
rte_mempool *mp)
 /**
  * Dump the status of all mempools on the console
  *
+ * Mempools that are not populated yet are skipped.
+ *
  * @param f
  *   A pointer to a file for output
  */
@@ -1949,6 +1951,7 @@ uint32_t rte_mempool_calc_obj_size(uint32_t elt_size, 
uint32_t flags,
  *
  * The mempool list is locked while the iterator function runs, so the
  * iterator function must not create or free a mempool.
+ * Mempools that are not populated yet are skipped.
  *
  * @param func
  *   Iterator function
-- 
2.34.1

Reply via email to