Re: [PATCH net v6 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy

[Leon Romanovsky <leon@kernel.org>]

On Mon, Aug 10, 2026 at 07:38:15PM -0700, Long Li wrote:
> The EQ interrupt handler (mana_gd_process_eqe) looks up the completing CQ
> in gc->cq_table[cq_id] and runs its callback, concurrently with CQ
> teardown on another CPU that clears the slot and frees the CQ.  cq_table
> was a plain pointer array freed with no grace period, so the two race
> into a use-after-free:
> 
>   CPU A (mana_gd_intr, hard IRQ)        CPU B (CQ destroy)
>   ----------------------------------    ------------------------------
>   cq = gc->cq_table[cq_id];  // valid
>                                         gc->cq_table[id] = NULL;
>                                         kfree(cq);          // freed
>   cq->cq.callback(ctx, cq);  // use-after-free
> 
> The handler's existing rcu_read_lock() only guards the per-IRQ EQ list
> traversal; cq_table was never under any RCU contract, and a read-side
> lock is inert unless the freer also defers the free past a grace period.
> 
> Put cq_table under RCU: annotate the base pointer and entries __rcu, read
> with rcu_dereference() in the handler, publish with rcu_assign_pointer(),
> and on teardown clear the slot then synchronize_rcu() before freeing the
> CQ.  The grace period blocks until every in-flight handler has dropped
> the old pointer, so the kfree() can no longer race the callback.
> 
> This fixes only the CQ lifetime (the use-after-free); it does not make
> the cq_id bound trustworthy.  gc->max_num_cqs is still range-checked
> outside the published table, and hardening that field against a spoofed
> device value is a separate change.
> 
> netdev teardown destroys a CQ per TX and per RX queue, so one grace
> period each in mana_gd_destroy_cq() would serialize up to
> 2 * MANA_MAX_NUM_QUEUES synchronize_rcu() calls under RTNL on every
> ifdown, MTU change or ring/channel reconfigure.  Clear all of a port's
> CQ slots first and take a single grace period per teardown instead:
> mana_gd_unpublish_cq() clears a slot without waiting, and
> mana_gd_destroy_cq() -- which still serves the single-CQ callers --
> finds the slot already cleared and skips its own synchronize_rcu().
> 
> Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> Changes in v6:
>  - mana_gd_unpublish_cq() and mana_ib_remove_cq_cb() clear a cq_table
>    slot only when it still points at the CQ being torn down, so the
>    two-pass teardown cannot wipe an entry a concurrent RDMA CQ create
>    recycled during the grace period.
>  - mana_gd_process_eqe() drops an already-unpublished (NULL) slot quietly
>    instead of a WARN_ON_ONCE() splat during a normal ifdown/MTU change,
>    and reads gc->cq_table before gc->max_num_cqs with an smp_rmb()
>    between them so a shrinking re-establish cannot pair a stale bound
>    with a newly published, smaller table.
>  - Documented the cq_table/max_num_cqs contract on the cq_table field
>    instead of rewording the comment above max_num_cqs.
> 
> Changes in v5:
>  - No code changes since v4 (resend as a standalone thread).
> 
> Changes in v4:
>  - Replaced the per-CQ synchronize_rcu() in the netdev teardown paths
>    with a two-pass quiesce/free that takes one grace period per
>    teardown; mana_gd_unpublish_cq() splits the slot-clear from the grace
>    period.
>  - Snapshot cq->id and max_num_cqs with READ_ONCE() in
>    mana_hwc_establish_channel() so one value sizes, bounds and indexes
>    cq_table.
>  - Corrected the gc->cq_table lifetime comment in gdma.h; rescoped the
>    changelog to the use-after-free fix (the bound is patch 7).
> 
>  drivers/infiniband/hw/mana/cq.c               |  51 ++++++-
>  .../net/ethernet/microsoft/mana/gdma_main.c   |  65 +++++++--
>  .../net/ethernet/microsoft/mana/hw_channel.c  |  29 ++--
>  drivers/net/ethernet/microsoft/mana/mana_en.c | 136 ++++++++++++++----
>  include/net/mana/gdma.h                       |  37 ++++-
>  5 files changed, 268 insertions(+), 50 deletions(-)

This patch is so bloated with AI that it is hard to read and difficult to justify
such a large diff for a simple change, which all drivers experience that
flow.

As a bare minimum. you need to reorder mana_ib_gd_destroy_cq(), mana_ib_destroy_queue(),
and mana_ib_remove_cq_cb() so that HW objects are stopped before SW state is torn down.

And probably introduce get/put CQ primitives.

Thanks