[PATCH net v5 0/7] net: mana: HW channel reliability and hardening fixes

[PATCH net v5 0/7] net: mana: HW channel reliability and hardening fixes

[Long Li]

This series fixes a set of latent bugs and robustness gaps in the MANA
Hardware Channel (HWC), the control path the driver uses to talk to the
device.  The issues range from a use-after-free of completion queues
during teardown to buffer mis-sizing, unsafe teardown ordering, missing
validation of device-supplied RX metadata, and stale-response handling
after a command timeout.

Patch overview:

  1  RCU-protect gc->cq_table lookups against concurrent CQ destroy
     The EQ interrupt handler dereferences CQ pointers from gc->cq_table
     while teardown can free them.  Put the table under RCU and wait a
     grace period before freeing, closing the use-after-free.

  2  fix HWC RQ/SQ buffer size swap
     init_queues() sized the RQ with max_req_msg_size and the SQ with
     max_resp_msg_size -- backwards.  Correct the swap; both sizes are
     equal in practice, so this is a latent-correctness fix.

  3  free HWC comp_buf after destroying the EQ
     Reorder teardown so the EQ is destroyed (readers quiesced) before
     comp_buf and the CQ are freed, preventing a late EQ-handler access
     to freed memory.

  4  validate hardware-supplied values in the HWC RX path
     Bounds-check the SGE, verify the recovered slot index and SGE
     address, and validate response length and msg_id before use, so
     malformed or hostile DMA metadata cannot cause wrong-slot completion
     or out-of-bounds access.

  5  fix HWC teardown safety with setup_active flag and destroy ordering
     Track setup activation explicitly, tear the EQ/CQ down before the
     TXQ/RXQ, and on an unrecoverable teardown failure leak the HWC
     resources rather than free memory the device may still DMA into.

  6  fix stale HWC response after command timeout
     Replace the inflight-slot semaphore with a bitmap + waitqueue and
     per-slot refcount/lock; latch the channel on timeout so no new slots
     are handed out, drop duplicate/late responses, and ignore a zero
     firmware-supplied timeout.

  7  keep max_num_cqs immutable once cq_table is allocated
     gc->max_num_cqs is set once when cq_table is allocated and never
     reset, so a spoofed post-init HWC event cannot inflate the bound
     past the allocation and drive an out-of-bounds cq_table access.

Follow-up feature work (net-next, sent separately):

The original series also contained two patches that are improvements, not
fixes:

  net: mana: support concurrent HWC requests
  net: mana: add dynamic HWC queue depth with reinit path

Per the netdev tree rules, fixes go to 'net' and features/improvements go
to 'net-next', and the two must not be combined in a single submission.
Those two patches build on the locking and teardown groundwork in this
series, so they will be posted as a separate net-next series only after
these fixes have propagated from net into net-next through the usual
periodic merge.

Changes since v4:

 - No code changes.  Resending as a standalone thread; v4 was
   accidentally sent in-reply-to the v3 thread.
 - v4: https://lore.kernel.org/netdev/20260808023417.1746886-1-longli@microsoft.com/
 - v3: https://lore.kernel.org/netdev/20260803234355.636038-1-longli@microsoft.com/

Changes since v3:

Addressed the netdev-ai and sashiko.dev automated reviews of v3.

 - New patch 7 ("keep max_num_cqs immutable once cq_table is
   allocated"): gc->max_num_cqs is set once and never reset, so a
   spoofed post-init HWC event cannot inflate the bound past the
   allocation and cause an out-of-bounds cq_table access.
 - patch 1: replaced the per-CQ synchronize_rcu() in the netdev teardown
   paths with a two-pass quiesce/free that takes a single grace period
   per teardown; snapshot cq->id and max_num_cqs with READ_ONCE() so the
   same value sizes, bounds and indexes cq_table; corrected the
   gc->cq_table lifetime comment; rescoped the changelog to the
   use-after-free fix.
 - patch 2: reworded the changelog as a latent-correctness fix (both
   message sizes are 0x1000, so the swap has no observable overflow) and
   dropped the incorrect note about hoisting the queue dimensions.
 - patch 4: removed the short-response early return so a malformed
   response reaches verify_resp_msg() -> -EPROTO and completes the
   sender instead of hanging it; account leaked RX WQEs and trip
   hwc_timeout on RQ exhaustion; read the device-supplied
   inline_oob_size_div4 (through its u32 flags word, as it is a
   bit-field) and sge->address with READ_ONCE() and reject any value
   other than the one the driver programs; reframed the msg_id check as
   defense in depth.
 - patch 5: arm setup_active immediately after mana_smc_setup_hwc()
   succeeds; destroy the EQ (IRQ deregister + drain) before the CQ; drop
   the redundant teardown in mana_hwc_establish_channel() that caused a
   double hardware timeout and masked the original error.
 - patch 6: take both the sender and response-side references up front in
   mana_hwc_get_msg_index() (refcount initialised to 2, under the lock
   that publishes the slot) so an early/stale/forged response cannot free
   the slot before the sender posts; changed caller_ctx::error from u32
   to int; reject a zero firmware-supplied HWC timeout in the query path
   as well as the reconfig path; access hwc_timed_out with READ_ONCE()/
   WRITE_ONCE(); comment and changelog fixes.
 - Assorted comment and commit-message clarifications throughout.

Changes since v2:

 - Per maintainer feedback, split the original combined series: the
   fixes here target 'net'; the two feature patches now go to 'net-next'
   and are sent separately (see above).  Rebased the fixes onto net.
 - Dropped the pcie_flr()-based reset fallback from the teardown path.
   pcie_flr() resets device config without the save/restore that
   pci_reset_function() provides, and cannot be used as a drop-in here.
   On an unrecoverable teardown failure the driver now leaks the HWC
   resources instead of touching memory the device may still DMA into.
 - patch 2: store the HWC queue dimensions before creating the CQ so the
   RX completion handler can never observe a zero max_resp_msg_size
   divisor or a stale num_inflight_msg bound.
 - Assorted commit-message and comment clarifications.

Long Li (7):
  net: mana: RCU-protect gc->cq_table lookups against concurrent CQ
    destroy
  net: mana: fix HWC RQ/SQ buffer size swap
  net: mana: free HWC comp_buf after destroying the EQ
  net: mana: validate hardware-supplied values in the HWC RX path
  net: mana: fix HWC teardown safety with setup_active flag and destroy
    ordering
  net: mana: fix stale HWC response after command timeout
  net: mana: keep max_num_cqs immutable once cq_table is allocated

 drivers/infiniband/hw/mana/cq.c               |  46 +-
 .../net/ethernet/microsoft/mana/gdma_main.c   |  48 +-
 .../net/ethernet/microsoft/mana/hw_channel.c  | 452 +++++++++++++++---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 136 ++++--
 include/net/mana/gdma.h                       |  40 +-
 include/net/mana/hw_channel.h                 |  44 +-
 6 files changed, 652 insertions(+), 114 deletions(-)


base-commit: dd057113ac7ba5bdd2aed3d9405305911152f911
-- 
2.43.0

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

[Long Li]

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 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               |  46 +++++-
 .../net/ethernet/microsoft/mana/gdma_main.c   |  41 ++++--
 .../net/ethernet/microsoft/mana/hw_channel.c  |  29 ++--
 drivers/net/ethernet/microsoft/mana/mana_en.c | 136 ++++++++++++++----
 include/net/mana/gdma.h                       |  32 ++++-
 5 files changed, 234 insertions(+), 50 deletions(-)

diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
index f2547989f422..2bf4be21cede 100644
--- a/drivers/infiniband/hw/mana/cq.c
+++ b/drivers/infiniband/hw/mana/cq.c
@@ -131,12 +131,20 @@ static void mana_ib_cq_handler(void *ctx, struct gdma_queue *gdma_cq)
 int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
 {
 	struct gdma_context *gc = mdev_to_gc(mdev);
+	struct gdma_queue __rcu **cq_table;
 	struct gdma_queue *gdma_cq;
 
-	if (cq->queue.id >= gc->max_num_cqs)
+	/* No rcu_read_lock(): install/remove run within the IB device
+	 * lifetime, which mana_rdma_remove() (ib_unregister_device) drains
+	 * before the base cq_table can be freed.  See gdma_context::cq_table
+	 * in gdma.h for why "true" is sound.
+	 */
+	cq_table = rcu_dereference_protected(gc->cq_table, true);
+	if (!cq_table || cq->queue.id >= gc->max_num_cqs)
 		return -EINVAL;
+
 	/* Create CQ table entry, sharing a CQ between WQs is not supported */
-	if (gc->cq_table[cq->queue.id])
+	if (rcu_access_pointer(cq_table[cq->queue.id]))
 		return -EINVAL;
 	if (cq->queue.kmem)
 		gdma_cq = cq->queue.kmem;
@@ -149,23 +157,49 @@ int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
 	gdma_cq->type = GDMA_CQ;
 	gdma_cq->cq.callback = mana_ib_cq_handler;
 	gdma_cq->id = cq->queue.id;
-	gc->cq_table[cq->queue.id] = gdma_cq;
+	rcu_assign_pointer(cq_table[cq->queue.id], gdma_cq);
 	return 0;
 }
 
 void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
 {
 	struct gdma_context *gc = mdev_to_gc(mdev);
+	struct gdma_queue __rcu **cq_table;
+	struct gdma_queue *gdma_cq;
 
-	if (cq->queue.id >= gc->max_num_cqs || cq->queue.id == INVALID_QUEUE_ID)
+	if (cq->queue.id == INVALID_QUEUE_ID)
 		return;
 
 	if (cq->queue.kmem)
 	/* Then it will be cleaned and removed by the mana */
 		return;
 
-	kfree(gc->cq_table[cq->queue.id]);
-	gc->cq_table[cq->queue.id] = NULL;
+	/* No rcu_read_lock(): like mana_ib_install_cq_cb(), this runs within
+	 * the IB device lifetime that mana_rdma_remove() drains before the
+	 * base cq_table can be freed.  See gdma_context::cq_table in gdma.h.
+	 */
+	cq_table = rcu_dereference_protected(gc->cq_table, true);
+	if (!cq_table || cq->queue.id >= gc->max_num_cqs)
+		return;
+	/* Removers for a given CQ are serialized by the IB core, so the slot
+	 * is read and cleared without rcu_read_lock() or atomicity: a CQ is
+	 * never torn down while a live QP references it (cq->usecnt), nor
+	 * while the QP-create that installed the entry is still running (that
+	 * create holds a reference on the CQ uobject across its error path,
+	 * before usecnt is taken).  Any double-remove is therefore sequential
+	 * -- the later caller sees the NULL stored below and returns.
+	 */
+	gdma_cq = rcu_dereference_protected(cq_table[cq->queue.id], true);
+	if (!gdma_cq)
+		return;  /* already removed by a prior teardown path */
+
+	rcu_assign_pointer(cq_table[cq->queue.id], NULL);
+
+	/* Wait for in-flight EQ handlers that may have loaded the old
+	 * pointer via rcu_dereference() to finish before freeing.
+	 */
+	synchronize_rcu();
+	kfree(gdma_cq);
 }
 
 int mana_ib_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index e8b7ffb47eb9..7714040d1df4 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -732,6 +732,7 @@ static void mana_gd_process_eqe(struct gdma_queue *eq)
 	union gdma_eqe_info eqe_info;
 	enum gdma_eqe_type type;
 	struct gdma_event event;
+	struct gdma_queue __rcu **cq_table;
 	struct gdma_queue *cq;
 	struct gdma_eqe *eqe;
 	u32 cq_id;
@@ -743,10 +744,11 @@ static void mana_gd_process_eqe(struct gdma_queue *eq)
 	switch (type) {
 	case GDMA_EQE_COMPLETION:
 		cq_id = eqe->details[0] & 0xFFFFFF;
-		if (WARN_ON_ONCE(cq_id >= gc->max_num_cqs))
+		cq_table = rcu_dereference(gc->cq_table);
+		if (WARN_ON_ONCE(cq_id >= gc->max_num_cqs || !cq_table))
 			break;
 
-		cq = gc->cq_table[cq_id];
+		cq = rcu_dereference(cq_table[cq_id]);
 		if (WARN_ON_ONCE(!cq || cq->type != GDMA_CQ || cq->id != cq_id))
 			break;
 
@@ -1050,18 +1052,41 @@ static void mana_gd_create_cq(const struct gdma_queue_spec *spec,
 	queue->cq.callback = spec->cq.callback;
 }
 
-static void mana_gd_destroy_cq(struct gdma_context *gc,
-			       struct gdma_queue *queue)
+bool mana_gd_unpublish_cq(struct gdma_context *gc, struct gdma_queue *queue)
 {
+	struct gdma_queue __rcu **cq_table;
 	u32 id = queue->id;
 
-	if (id >= gc->max_num_cqs)
-		return;
+	/* No rcu_read_lock() here: unpublish runs only on the
+	 * CQ-destroy/teardown path, where the base cq_table is stable.  See
+	 * the lifecycle note on gdma_context::cq_table in gdma.h for why the
+	 * "true" predicate is sound.
+	 */
+	cq_table = rcu_dereference_protected(gc->cq_table, true);
+	if (!cq_table || id >= gc->max_num_cqs)
+		return false;
+
+	if (!rcu_access_pointer(cq_table[id]))
+		return false;
+
+	rcu_assign_pointer(cq_table[id], NULL);
+	return true;
+}
 
-	if (!gc->cq_table[id])
+static void mana_gd_destroy_cq(struct gdma_context *gc,
+			       struct gdma_queue *queue)
+{
+	/* A batched teardown may already have cleared the slot and taken the
+	 * grace period; then there is nothing left to wait for.
+	 */
+	if (!mana_gd_unpublish_cq(gc, queue))
 		return;
 
-	gc->cq_table[id] = NULL;
+	/* Wait for in-flight EQ handlers that may have loaded the old
+	 * pointer via rcu_dereference() to finish before the caller
+	 * frees the CQ memory.
+	 */
+	synchronize_rcu();
 }
 
 int mana_gd_create_hwc_queue(struct gdma_dev *gd,
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index e3c24d50dad0..409e20caeccd 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -674,6 +674,7 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	struct gdma_queue *sq = hwc->txq->gdma_wq;
 	struct gdma_queue *eq = hwc->cq->gdma_eq;
 	struct gdma_queue *cq = hwc->cq->gdma_cq;
+	struct gdma_queue __rcu **cq_table;
 	int err;
 
 	init_completion(&hwc->hwc_init_eqe_comp);
@@ -698,11 +699,15 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	if (WARN_ON(cq->id >= gc->max_num_cqs))
 		return -EPROTO;
 
-	gc->cq_table = vcalloc(gc->max_num_cqs, sizeof(struct gdma_queue *));
-	if (!gc->cq_table)
+	cq_table = vcalloc(gc->max_num_cqs, sizeof(*cq_table));
+	if (!cq_table)
 		return -ENOMEM;
 
-	gc->cq_table[cq->id] = cq;
+	rcu_assign_pointer(cq_table[cq->id], cq);
+	/* Publish the fully-initialised table last; pairs with the
+	 * rcu_dereference(gc->cq_table) in mana_gd_process_eqe().
+	 */
+	rcu_assign_pointer(gc->cq_table, cq_table);
 
 	return 0;
 }
@@ -811,6 +816,7 @@ int mana_hwc_create_channel(struct gdma_context *gc)
 void mana_hwc_destroy_channel(struct gdma_context *gc)
 {
 	struct hw_channel_context *hwc = gc->hwc.driver_data;
+	struct gdma_queue __rcu **old_cq_table;
 
 	if (!hwc)
 		return;
@@ -818,10 +824,8 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	/* gc->max_num_cqs is set in mana_hwc_init_event_handler(). If it's
 	 * non-zero, the HWC worked and we should tear down the HWC here.
 	 */
-	if (gc->max_num_cqs > 0) {
+	if (gc->max_num_cqs > 0)
 		mana_smc_teardown_hwc(&gc->shm_channel, false);
-		gc->max_num_cqs = 0;
-	}
 
 	if (hwc->txq)
 		mana_hwc_destroy_wq(hwc, hwc->txq);
@@ -832,6 +836,14 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	if (hwc->cq)
 		mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
 
+	/* Reset only after mana_hwc_destroy_cq() above has run with a valid
+	 * max_num_cqs so mana_gd_destroy_cq() clears the CQ table slot and
+	 * waits out in-flight EQ handlers (synchronize_rcu) before the CQ is
+	 * freed.  Clearing it earlier would make that path early-return and
+	 * skip the slot clear, leaving a dangling cq_table entry.
+	 */
+	gc->max_num_cqs = 0;
+
 	kfree(hwc->caller_ctx);
 	hwc->caller_ctx = NULL;
 
@@ -848,8 +860,9 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	gc->hwc.driver_data = NULL;
 	gc->hwc.gdma_context = NULL;
 
-	vfree(gc->cq_table);
-	gc->cq_table = NULL;
+	old_cq_table = rcu_replace_pointer(gc->cq_table, NULL, true);
+	synchronize_rcu();
+	vfree(old_cq_table);
 }
 
 int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 92bb55935c1c..5d215981bba8 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2427,12 +2427,18 @@ static void mana_deinit_txq(struct mana_port_context *apc, struct mana_txq *txq)
 
 static void mana_destroy_txq(struct mana_port_context *apc)
 {
+	struct gdma_context *gc = apc->ac->gdma_dev->gdma_context;
 	struct napi_struct *napi;
 	int i;
 
 	if (!apc->tx_qp)
 		return;
 
+	/* Pass 1: quiesce each CQ on the device and clear its cq_table slot.
+	 * Taking one grace period below for the whole port avoids up to
+	 * apc->num_queues serialized synchronize_rcu() calls (one per CQ in
+	 * mana_gd_destroy_cq()) under RTNL on every teardown.
+	 */
 	for (i = 0; i < apc->num_queues; i++) {
 		if (!apc->tx_qp[i])
 			continue;
@@ -2448,8 +2454,24 @@ static void mana_destroy_txq(struct mana_port_context *apc)
 			apc->tx_qp[i]->txq.napi_initialized = false;
 		}
 
-		if (apc->tx_qp[i]->tx_object != INVALID_MANA_HANDLE)
-			mana_destroy_wq_obj(apc, GDMA_SQ, apc->tx_qp[i]->tx_object);
+		if (apc->tx_qp[i]->tx_object != INVALID_MANA_HANDLE) {
+			mana_destroy_wq_obj(apc, GDMA_SQ,
+					    apc->tx_qp[i]->tx_object);
+			apc->tx_qp[i]->tx_object = INVALID_MANA_HANDLE;
+		}
+
+		if (apc->tx_qp[i]->tx_cq.gdma_cq)
+			mana_gd_unpublish_cq(gc, apc->tx_qp[i]->tx_cq.gdma_cq);
+	}
+
+	synchronize_rcu();
+
+	/* Pass 2: the slots are clear, so mana_gd_destroy_cq() skips its own
+	 * grace period; free the CQ, the TXQ and the queue pair.
+	 */
+	for (i = 0; i < apc->num_queues; i++) {
+		if (!apc->tx_qp[i])
+			continue;
 
 		mana_deinit_cq(apc, &apc->tx_qp[i]->tx_cq);
 
@@ -2496,6 +2518,7 @@ static int mana_create_txq(struct mana_port_context *apc,
 	struct mana_obj_spec cq_spec;
 	struct gdma_queue_spec spec;
 	struct gdma_context *gc;
+	struct gdma_queue __rcu **cq_table;
 	struct mana_txq *txq;
 	struct mana_cq *cq;
 	u32 txq_size;
@@ -2596,12 +2619,18 @@ static int mana_create_txq(struct mana_port_context *apc,
 
 		cq->gdma_id = cq->gdma_cq->id;
 
-		if (WARN_ON(cq->gdma_id >= gc->max_num_cqs)) {
+		/* No rcu_read_lock(): mana_create_txq runs under RTNL during
+		 * netdev bring-up, inside the netdev lifetime that
+		 * mana_remove() drains before the base cq_table can be freed.
+		 * See gdma_context::cq_table in gdma.h for why "true" is sound.
+		 */
+		cq_table = rcu_dereference_protected(gc->cq_table, true);
+		if (WARN_ON(!cq_table || cq->gdma_id >= gc->max_num_cqs)) {
 			err = -EINVAL;
 			goto out;
 		}
 
-		gc->cq_table[cq->gdma_id] = cq->gdma_cq;
+		rcu_assign_pointer(cq_table[cq->gdma_id], cq->gdma_cq);
 
 		mana_create_txq_debugfs(apc, i);
 
@@ -2621,25 +2650,20 @@ static int mana_create_txq(struct mana_port_context *apc,
 	return err;
 }
 
-static void mana_destroy_rxq(struct mana_port_context *apc,
+/* Quiesce an RXQ's CQ on the device and clear its cq_table slot, without
+ * waiting for a grace period.  Split out of mana_destroy_rxq() so a batch
+ * teardown (mana_destroy_rxqs()) can quiesce every RXQ and then take a
+ * single synchronize_rcu() instead of one per RXQ.
+ */
+static void mana_quiesce_rxq(struct mana_port_context *apc,
 			     struct mana_rxq *rxq, bool napi_initialized)
-
 {
 	struct gdma_context *gc = apc->ac->gdma_dev->gdma_context;
-	struct mana_recv_buf_oob *rx_oob;
-	struct device *dev = gc->dev;
-	struct napi_struct *napi;
-	struct page *page;
-	int i;
-
-	if (!rxq)
-		return;
+	struct napi_struct *napi = &rxq->rx_cq.napi;
 
 	debugfs_remove_recursive(rxq->mana_rx_debugfs);
 	rxq->mana_rx_debugfs = NULL;
 
-	napi = &rxq->rx_cq.napi;
-
 	if (napi_initialized) {
 		napi_synchronize(napi);
 
@@ -2650,8 +2674,27 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
 	if (xdp_rxq_info_is_reg(&rxq->xdp_rxq))
 		xdp_rxq_info_unreg(&rxq->xdp_rxq);
 
-	if (rxq->rxobj != INVALID_MANA_HANDLE)
+	if (rxq->rxobj != INVALID_MANA_HANDLE) {
 		mana_destroy_wq_obj(apc, GDMA_RQ, rxq->rxobj);
+		rxq->rxobj = INVALID_MANA_HANDLE;
+	}
+
+	if (rxq->rx_cq.gdma_cq)
+		mana_gd_unpublish_cq(gc, rxq->rx_cq.gdma_cq);
+}
+
+/* Free an RXQ once its cq_table slot has been cleared and a grace period
+ * has elapsed (see mana_quiesce_rxq()).  mana_deinit_cq() ->
+ * mana_gd_destroy_cq() finds the slot already NULL and skips its own
+ * synchronize_rcu().
+ */
+static void mana_free_rxq(struct mana_port_context *apc, struct mana_rxq *rxq)
+{
+	struct gdma_context *gc = apc->ac->gdma_dev->gdma_context;
+	struct mana_recv_buf_oob *rx_oob;
+	struct device *dev = gc->dev;
+	struct page *page;
+	int i;
 
 	mana_deinit_cq(apc, &rxq->rx_cq);
 
@@ -2685,6 +2728,23 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
 	kvfree(rxq);
 }
 
+static void mana_destroy_rxq(struct mana_port_context *apc,
+			     struct mana_rxq *rxq, bool napi_initialized)
+
+{
+	if (!rxq)
+		return;
+
+	mana_quiesce_rxq(apc, rxq, napi_initialized);
+
+	/* Wait for in-flight EQ handlers that may have loaded the old CQ
+	 * pointer via rcu_dereference() before freeing.
+	 */
+	synchronize_rcu();
+
+	mana_free_rxq(apc, rxq);
+}
+
 static int mana_fill_rx_oob(struct mana_recv_buf_oob *rx_oob, u32 mem_key,
 			    struct mana_rxq *rxq, struct device *dev)
 {
@@ -2821,6 +2881,7 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
 	struct gdma_queue_spec spec;
 	struct mana_cq *cq = NULL;
 	struct gdma_context *gc;
+	struct gdma_queue __rcu **cq_table;
 	u32 cq_size, rq_size;
 	struct mana_rxq *rxq;
 	int err;
@@ -2905,12 +2966,18 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
 	if (err)
 		goto out;
 
-	if (WARN_ON(cq->gdma_id >= gc->max_num_cqs)) {
+	/* No rcu_read_lock(): mana_create_rxq runs under RTNL during netdev
+	 * bring-up, inside the netdev lifetime that mana_remove() drains
+	 * before the base cq_table can be freed.  See gdma_context::cq_table
+	 * in gdma.h for why "true" is sound.
+	 */
+	cq_table = rcu_dereference_protected(gc->cq_table, true);
+	if (WARN_ON(!cq_table || cq->gdma_id >= gc->max_num_cqs)) {
 		err = -EINVAL;
 		goto out;
 	}
 
-	gc->cq_table[cq->gdma_id] = cq->gdma_cq;
+	rcu_assign_pointer(cq_table[cq->gdma_id], cq->gdma_cq);
 
 	netif_napi_add_weight_locked(ndev, &cq->napi, mana_poll, 1);
 
@@ -2987,16 +3054,31 @@ static void mana_destroy_rxqs(struct mana_port_context *apc)
 	struct mana_rxq *rxq;
 	u32 rxq_idx;
 
-	if (apc->rxqs) {
+	if (!apc->rxqs)
+		return;
 
-		for (rxq_idx = 0; rxq_idx < apc->num_queues; rxq_idx++) {
-			rxq = apc->rxqs[rxq_idx];
-			if (!rxq)
-				continue;
+	/* Pass 1: quiesce every RXQ's CQ and clear its cq_table slot. */
+	for (rxq_idx = 0; rxq_idx < apc->num_queues; rxq_idx++) {
+		rxq = apc->rxqs[rxq_idx];
+		if (!rxq)
+			continue;
 
-			mana_destroy_rxq(apc, rxq, true);
-			apc->rxqs[rxq_idx] = NULL;
-		}
+		mana_quiesce_rxq(apc, rxq, true);
+	}
+
+	/* One grace period for the whole port instead of one per RXQ. */
+	synchronize_rcu();
+
+	/* Pass 2: the slots are clear, so mana_gd_destroy_cq() skips its own
+	 * grace period; free each RXQ.
+	 */
+	for (rxq_idx = 0; rxq_idx < apc->num_queues; rxq_idx++) {
+		rxq = apc->rxqs[rxq_idx];
+		if (!rxq)
+			continue;
+
+		mana_free_rxq(apc, rxq);
+		apc->rxqs[rxq_idx] = NULL;
 	}
 }
 
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 0c395917b214..b8b1b23f3c36 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -418,7 +418,31 @@ struct gdma_context {
 
 	/* This maps a CQ index to the queue structure. */
 	unsigned int		max_num_cqs;
-	struct gdma_queue	**cq_table;
+	/* Both the base pointer and each entry are RCU-managed.  The fast
+	 * path (mana_gd_process_eqe) reads the base via rcu_dereference()
+	 * under rcu_read_lock(), so the table is freed with
+	 * rcu_assign_pointer(NULL) + synchronize_rcu() and an in-flight
+	 * reader can never observe freed memory.
+	 *
+	 * The slow paths -- mana_gd_destroy_cq() and the CQ install/remove
+	 * callers (mana_create_txq/_rxq, mana_ib_install/remove_cq_cb) --
+	 * instead read the base with rcu_dereference_protected(cq_table,
+	 * true).  The bare "true" asserts teardown/bring-up ordering, not a
+	 * lock: the base table is allocated in mana_hwc_establish_channel()
+	 * and replaced+freed only by mana_hwc_destroy_channel() (via
+	 * mana_gd_cleanup_device()) and the create-time reinit.  The reinit
+	 * runs before either consumer is probed, and cleanup_device() runs
+	 * after mana_remove() / mana_rdma_remove() have detached the ports
+	 * under RTNL and drained the IB device, so no install/remove caller
+	 * is running when the base is freed.  This is an ordering argument
+	 * about when cleanup_device() runs: suspend and shutdown keep the
+	 * netdev registered, so it does not rely on unregister_netdevice()
+	 * having run on every path.  mana_hwc_destroy_channel() itself reads
+	 * cq_table (mana_hwc_destroy_cq()) before it replaces and vfree()s
+	 * the base, so that access is ordered ahead of the free by program
+	 * order.
+	 */
+	struct gdma_queue	__rcu * __rcu *cq_table;
 
 	/* Protect eq_test_event and test_event_eq_id  */
 	struct mutex		eq_test_event_mutex;
@@ -496,6 +520,12 @@ int mana_gd_create_mana_wq_cq(struct gdma_dev *gd,
 
 void mana_gd_destroy_queue(struct gdma_context *gc, struct gdma_queue *queue);
 
+/* Clear a CQ's cq_table slot without waiting for a grace period.  Batched
+ * teardown paths clear several slots and then take a single synchronize_rcu();
+ * single-CQ callers use mana_gd_destroy_cq() instead, which also waits.
+ */
+bool mana_gd_unpublish_cq(struct gdma_context *gc, struct gdma_queue *queue);
+
 int mana_gd_poll_cq(struct gdma_queue *cq, struct gdma_comp *comp, int num_cqe);
 
 void mana_gd_ring_cq(struct gdma_queue *cq, u8 arm_bit);
-- 
2.43.0

[PATCH net v5 2/7] net: mana: fix HWC RQ/SQ buffer size swap

[Long Li]

The HWC RQ receives responses and the SQ sends requests, but
mana_hwc_init_queues() sized the RQ with max_req_msg_size and the SQ with
max_resp_msg_size -- backwards.  mana_hwc_rx_event_handler() also
recovered the RX slot index by dividing by max_req_msg_size instead of
the response size that strides the RQ buffer.

Both mistakes are latent today: the only caller passes
HW_CHANNEL_MAX_REQUEST_SIZE and HW_CHANNEL_MAX_RESPONSE_SIZE, which are
both 0x1000, and the queues are never re-created with the
hardware-reported sizes (those are only used by mana_hwc_test_channel()),
so max_req_msg_size == max_resp_msg_size on every path and there is no
observable overflow or mis-indexing.  The bug would only surface if the
two sizes ever diverged.

Size the RQ by max_resp_msg_size and the SQ by max_req_msg_size, store
max_resp_msg_size in hw_channel_context, and use it as the RX slot stride
so the code is correct regardless of the two sizes.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes in v5:
 - No code changes since v4 (resend as a standalone thread).

Changes in v4:
 - Reworded the changelog as a latent-correctness fix (both message
   sizes are 0x1000, so the swap has no observable overflow) and dropped
   the note about hoisting the queue dimensions above
   mana_hwc_create_cq().

 drivers/net/ethernet/microsoft/mana/hw_channel.c | 7 ++++---
 include/net/mana/hw_channel.h                    | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 409e20caeccd..3f011ebbe7b3 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -263,7 +263,7 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
 
 	/* Select the RX work request for virtual address and for reposting. */
 	rq_base_addr = hwc_rxq->msg_buf->mem_info.dma_handle;
-	rx_req_idx = (sge->address - rq_base_addr) / hwc->max_req_msg_size;
+	rx_req_idx = (sge->address - rq_base_addr) / hwc->max_resp_msg_size;
 
 	if (rx_req_idx >= hwc_rxq->msg_buf->num_reqs) {
 		dev_err(hwc->dev, "HWC RX: wrong rx_req_idx=%llu, num_reqs=%u\n",
@@ -733,14 +733,14 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
 		goto out;
 	}
 
-	err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth, max_req_msg_size,
+	err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth, max_resp_msg_size,
 				 hwc->cq, &hwc->rxq);
 	if (err) {
 		dev_err(hwc->dev, "Failed to create HWC RQ: %d\n", err);
 		goto out;
 	}
 
-	err = mana_hwc_create_wq(hwc, GDMA_SQ, q_depth, max_resp_msg_size,
+	err = mana_hwc_create_wq(hwc, GDMA_SQ, q_depth, max_req_msg_size,
 				 hwc->cq, &hwc->txq);
 	if (err) {
 		dev_err(hwc->dev, "Failed to create HWC SQ: %d\n", err);
@@ -749,6 +749,7 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
 
 	hwc->num_inflight_msg = q_depth;
 	hwc->max_req_msg_size = max_req_msg_size;
+	hwc->max_resp_msg_size = max_resp_msg_size;
 
 	return 0;
 out:
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 16feb39616c1..73671f479399 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -181,6 +181,7 @@ struct hw_channel_context {
 
 	u16 num_inflight_msg;
 	u32 max_req_msg_size;
+	u32 max_resp_msg_size;
 
 	u16 hwc_init_q_depth_max;
 	u32 hwc_init_max_req_msg_size;
-- 
2.43.0

[PATCH net v5 3/7] net: mana: free HWC comp_buf after destroying the EQ

[Long Li]

mana_hwc_destroy_cq() freed hwc_cq->comp_buf and destroyed the CQ before
the EQ.  That was unsafe while the EQ was still registered: the EQ
interrupt handler reaches comp_buf via mana_hwc_comp_event() and the CQ
object (hwc->cq->gdma_cq) via mana_hwc_init_event_handler(), so a late
EQE dispatched after the free could touch freed memory.

Destroy the EQ first.  mana_gd_destroy_queue() on the EQ deregisters its
IRQ and waits out in-flight handlers, fencing all EQE dispatch; only then
free the CQ and comp_buf.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes in v5:
 - No code changes since v4 (resend as a standalone thread).

Changes in v4:
 - No functional change since v3; the teardown-ordering guarantees this
   patch relies on are made explicit in patch 5.

 .../net/ethernet/microsoft/mana/hw_channel.c   | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 3f011ebbe7b3..19896bb5ce1a 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -384,14 +384,24 @@ static void mana_hwc_comp_event(void *ctx, struct gdma_queue *q_self)
 
 static void mana_hwc_destroy_cq(struct gdma_context *gc, struct hwc_cq *hwc_cq)
 {
-	kfree(hwc_cq->comp_buf);
+	/* Destroy the EQ before the CQ.  mana_gd_destroy_queue() on the EQ
+	 * deregisters its IRQ and waits out in-flight handlers, fencing all
+	 * EQE dispatch — both the completion path and HWC init/reconfig
+	 * events.  Freeing the CQ first would leave the EQ live and able to
+	 * dispatch an event that dereferences hwc->cq->gdma_cq (e.g.
+	 * mana_hwc_init_event_handler()) after it has been freed.
+	 */
+	if (hwc_cq->gdma_eq)
+		mana_gd_destroy_queue(gc, hwc_cq->gdma_eq);
 
+	/* comp_buf is reached only by mana_hwc_comp_event(), invoked from
+	 * the now-fenced EQ handler, so it is safe to free once the EQ and
+	 * CQ are gone.
+	 */
 	if (hwc_cq->gdma_cq)
 		mana_gd_destroy_queue(gc, hwc_cq->gdma_cq);
 
-	if (hwc_cq->gdma_eq)
-		mana_gd_destroy_queue(gc, hwc_cq->gdma_eq);
-
+	kfree(hwc_cq->comp_buf);
 	kfree(hwc_cq);
 }
 
-- 
2.43.0

[PATCH net v5 4/7] net: mana: validate hardware-supplied values in the HWC RX path

[Long Li]

mana_hwc_rx_event_handler() consumed lengths and indices taken straight
from device DMA without validation.  A buggy firmware or a malicious host
(in a confidential VM, where the DMA buffer is shared) could drive a
wrong or reused in-flight request to completion or index out of bounds.
Validate before use:

  - snapshot the device-supplied inline_oob_size_div4 (read once through
    its u32 flags word with READ_ONCE(), as it is a bit-field) and reject
    any value other than the one the driver programs
    (INLINE_OOB_SMALL_SIZE / 4), so a corrupted OOB size cannot move the
    SGE out of the WQE before it is dereferenced;
  - snapshot sge->address with READ_ONCE() and validate and use only the
    snapshot, so the value that is bounds-checked is the value that is
    used (the DMA buffer is host-writable in a confidential VM);
  - match the SGE address against the address the driver posted for that
    slot, not just an in-range index -- an in-range but wrong SGE would
    otherwise truncate onto a neighbouring slot and read a stale response;
  - reject a resp_len larger than the RX buffer.

As defence in depth, mana_hwc_handle_resp() also bounds-checks hwc_msg_id
before indexing the inflight bitmap and caller_ctx.  Its only caller
already rejects the same range with the value it passes by value, so this
is a guard at the indexing site, not a reachable out-of-bounds.

Repost the RX WQE on every validation early-return that can still
identify its slot.  The paths that cannot -- an unexpected OOB size, an
out-of-range index, or an SGE address matching no posted slot --
intentionally leak a single WQE rather than risk reposting the wrong one.
Because the HWC RQ depth is never replenished, count those leaks and,
once they exhaust the posted depth, log the terminal state and shorten
the command timeout so callers fail fast instead of draining silently.

A short response is no longer rejected in the handler: it reaches
mana_hwc_handle_resp(), whose mana_hwc_verify_resp_msg() fails it with
-EPROTO and completes the waiting sender, so a single malformed response
cannot convert into a channel-wide timeout.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes in v5:
 - No code changes since v4 (resend as a standalone thread).

Changes in v4:
 - Removed the short-response early return so a malformed response
   reaches verify_resp_msg() -> -EPROTO and completes the sender instead
   of hanging it.
 - Account leaked RX WQEs and trip hwc_timeout on RQ exhaustion.
 - Read inline_oob_size_div4 (through its u32 flags word, as it is a
   bit-field) and sge->address with READ_ONCE() and reject any value
   other than the one the driver programs.
 - Reframed the msg_id check as defense in depth in the changelog.

 .../net/ethernet/microsoft/mana/hw_channel.c  | 116 ++++++++++++++++--
 include/net/mana/hw_channel.h                 |   6 +
 2 files changed, 111 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 19896bb5ce1a..5db8cfe2d844 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -83,6 +83,19 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
 	struct hwc_caller_ctx *ctx;
 	int err;
 
+	/* Defence in depth: the sole caller, mana_hwc_rx_event_handler(),
+	 * already rejects msg_id >= hwc->num_inflight_msg with the value it
+	 * passes here by value, so this cannot be reached out of range.  Keep
+	 * the guard at the indexing site so the bitmap and caller_ctx array
+	 * are never indexed without a bound in view.
+	 */
+	if (msg_id >= hwc->num_inflight_msg) {
+		dev_err(hwc->dev, "hwc_rx: msg_id %u >= max %u\n",
+			msg_id, hwc->num_inflight_msg);
+		mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
+		return;
+	}
+
 	if (!test_bit(msg_id, hwc->inflight_msg_res.map)) {
 		dev_err(hwc->dev, "hwc_rx: invalid msg_id = %u\n", msg_id);
 		mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
@@ -90,6 +103,18 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
 	}
 
 	ctx = hwc->caller_ctx + msg_id;
+
+	/* Reject responses larger than the RX DMA buffer — the SGE
+	 * limits what hardware can DMA, so an oversized resp_len
+	 * indicates a firmware bug.  Fail rather than silently
+	 * truncating.
+	 */
+	if (resp_len > rx_req->buf_len) {
+		dev_err(hwc->dev, "HWC RX: resp_len %u > buf_len %u\n",
+			resp_len, rx_req->buf_len);
+		resp_len = 0;
+	}
+
 	err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
 	if (err)
 		goto out;
@@ -237,18 +262,39 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
 	}
 }
 
+/* An RX WQE whose SGE the handler cannot trust is deliberately not
+ * reposted: reposting a slot we may have mis-identified could double-post
+ * a buffer the device still owns.  Each such leak permanently lowers the
+ * RQ's posted depth, so once the whole depth is gone the channel can no
+ * longer receive responses.  Make that terminal state explicit -- log it
+ * once and shorten the command timeout so callers fail fast -- rather than
+ * letting every later command drain its full timeout against a dead RQ.
+ */
+static void mana_hwc_rx_leak_wqe(struct hw_channel_context *hwc)
+{
+	if (++hwc->rx_leaked_wqe == hwc->rxq->queue_depth) {
+		dev_err(hwc->dev,
+			"HWC RX: RQ exhausted after %u leaked WQEs; channel unusable\n",
+			hwc->rx_leaked_wqe);
+		hwc->hwc_timeout = 1;
+	}
+}
+
 static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
 				      const struct hwc_rx_oob *rx_oob)
 {
 	struct hw_channel_context *hwc = ctx;
 	struct hwc_wq *hwc_rxq = hwc->rxq;
 	struct hwc_work_request *rx_req;
+	struct gdma_wqe oob_snapshot;
 	struct gdma_resp_hdr *resp;
 	struct gdma_wqe *dma_oob;
 	struct gdma_queue *rq;
 	struct gdma_sge *sge;
 	u64 rq_base_addr;
 	u64 rx_req_idx;
+	u64 sge_addr;
+	u32 oob_div4;
 	u16 msg_id;
 	u8 *wqe;
 
@@ -259,28 +305,76 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
 	wqe = mana_gd_get_wqe_ptr(rq, rx_oob->wqe_offset / GDMA_WQE_BU_SIZE);
 	dma_oob = (struct gdma_wqe *)wqe;
 
-	sge = (struct gdma_sge *)(wqe + 8 + dma_oob->inline_oob_size_div4 * 4);
-
-	/* Select the RX work request for virtual address and for reposting. */
+	/* inline_oob_size_div4 lives in device-accessible RQ memory (shared
+	 * and host-writable in a confidential VM), so snapshot it once and
+	 * validate and use only the snapshot.  It is a bit-field, which
+	 * READ_ONCE() cannot take the size of, so read the u32 flags word it
+	 * shares through the union and extract the field from the local copy.
+	 * The driver programs INLINE_OOB_SMALL_SIZE for every HWC RQ WQE via
+	 * mana_gd_post_work_request(), so the only valid value is
+	 * INLINE_OOB_SMALL_SIZE / 4, which puts the SGE at wqe + 16 inside
+	 * this WQE's own BU.  Reject anything else -- the slot cannot be
+	 * trusted, so leak this RX WQE rather than repost the wrong one.
+	 */
+	oob_snapshot.flags = READ_ONCE(dma_oob->flags);
+	oob_div4 = oob_snapshot.inline_oob_size_div4;
+	if (oob_div4 != INLINE_OOB_SMALL_SIZE / 4) {
+		dev_err(hwc->dev, "HWC RX: unexpected inline_oob_size_div4=%u\n",
+			oob_div4);
+		mana_hwc_rx_leak_wqe(hwc);
+		return;
+	}
+	sge = (struct gdma_sge *)(wqe + 8 + oob_div4 * 4);
+
+	/* Recover the originating RX slot from the SGE address.  Snapshot it
+	 * once, for the same shared-memory reason: of the three terms only
+	 * sge_addr comes from device memory; rq_base_addr and
+	 * max_resp_msg_size are driver-private.  An in-range but wrong SGE
+	 * would otherwise truncate onto a neighbouring slot, letting us read
+	 * a stale response that could complete the wrong, reused in-flight
+	 * request.  Require the index in range AND the address to exactly
+	 * match the value the driver posted for that slot.
+	 */
+	sge_addr = READ_ONCE(sge->address);
 	rq_base_addr = hwc_rxq->msg_buf->mem_info.dma_handle;
-	rx_req_idx = (sge->address - rq_base_addr) / hwc->max_resp_msg_size;
-
-	if (rx_req_idx >= hwc_rxq->msg_buf->num_reqs) {
-		dev_err(hwc->dev, "HWC RX: wrong rx_req_idx=%llu, num_reqs=%u\n",
-			rx_req_idx, hwc_rxq->msg_buf->num_reqs);
+	rx_req_idx = (sge_addr - rq_base_addr) / hwc->max_resp_msg_size;
+
+	if (rx_req_idx >= hwc_rxq->queue_depth) {
+		/* Cannot identify the slot, so we cannot safely repost this
+		 * WQE; leak it.  An out-of-range index means a corrupted SGE
+		 * from hardware or host tampering.
+		 */
+		dev_err(hwc->dev, "HWC RX: SGE idx %llu out of range\n",
+			rx_req_idx);
+		mana_hwc_rx_leak_wqe(hwc);
 		return;
 	}
 
 	rx_req = &hwc_rxq->msg_buf->reqs[rx_req_idx];
+	if (sge_addr != (u64)rx_req->buf_sge_addr) {
+		/* In-range index but the address does not match what the
+		 * driver posted for that slot; the same unrecoverable case,
+		 * so leak this WQE rather than repost the wrong one.
+		 */
+		dev_err(hwc->dev, "HWC RX: invalid SGE address %llx (idx=%llu)\n",
+			sge_addr, rx_req_idx);
+		mana_hwc_rx_leak_wqe(hwc);
+		return;
+	}
+
 	resp = (struct gdma_resp_hdr *)rx_req->buf_va;
 
-	/* Read msg_id once from DMA buffer to prevent TOCTOU:
-	 * DMA memory is shared/unencrypted in CVMs - host can
-	 * modify it between reads.
+	/* Read msg_id once from the DMA buffer to prevent TOCTOU: DMA memory
+	 * is shared/unencrypted in CVMs, so the host can modify it between
+	 * reads.  A short response is not rejected here; it is handed to
+	 * mana_hwc_handle_resp() below, whose mana_hwc_verify_resp_msg()
+	 * fails it with -EPROTO and completes the waiting sender, so one
+	 * malformed response cannot stall the whole channel.
 	 */
 	msg_id = READ_ONCE(resp->response.hwc_msg_id);
 	if (msg_id >= hwc->num_inflight_msg) {
 		dev_err(hwc->dev, "HWC RX: wrong msg_id=%u\n", msg_id);
+		mana_hwc_post_rx_wqe(hwc_rxq, rx_req);
 		return;
 	}
 
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 73671f479399..787c6f96d5b5 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -200,6 +200,12 @@ struct hw_channel_context {
 	u32 pf_dest_vrcq_id;
 	u32 hwc_timeout;
 
+	/* Count of RX WQEs deliberately not reposted after an untrusted SGE
+	 * (see mana_hwc_rx_leak_wqe()); once it reaches the RQ depth the
+	 * channel can no longer receive responses.
+	 */
+	u32 rx_leaked_wqe;
+
 	struct hwc_caller_ctx *caller_ctx;
 };
 
-- 
2.43.0

[PATCH net v5 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering

[Long Li]

Three teardown hazards let the hardware touch memory the driver freed.

First, once mana_smc_setup_hwc() succeeds the device has active MST
entries and can DMA into the HWC queue buffers.  If a later step in
mana_hwc_establish_channel() fails, the caller had no reliable way to
know teardown was required and could free those buffers while the
mappings were still live -- a DMA-after-free.  max_num_cqs was used as a
"HWC is up" proxy, but it is only set when the init EQE arrives.

Add a setup_active flag, set the moment setup_hwc activates MST entries.
On a later failure establish_channel() just returns the error; the
caller's error path (mana_hwc_create_channel() -> destroy_channel())
performs the single teardown, gated on setup_active.  Tearing down inline
as well would run teardown twice -- doubling the 60s hardware timeout on
failure and masking the original error code.  max_num_cqs is no longer
reset: it is an immutable bound (see gdma.h) and cq_table == NULL is the
sole teardown signal.

Second, destroy_channel() freed the TXQ/RXQ buffers while the HWC EQ was
still on the interrupt dispatch list, so an in-flight interrupt could run
the handler against freed buffers:

  CPU A (mana_gd_intr, hard IRQ)        CPU B (destroy_channel)
  ----------------------------------    ------------------------------
                                        free TXQ/RXQ DMA buffers
  handler accesses RQ/TXQ buffers       (EQ still registered)

Destroy the CQ first: mana_hwc_destroy_cq() -> mana_gd_deregister_irq()
removes the EQ via list_del_rcu() + synchronize_rcu(), after which no
handler can reach the queues; only then free the TXQ and RXQ.

Third, if mana_smc_teardown_hwc() itself fails the MST entries stay
live, yet destroy_channel() went on to free the CQ/RQ/TXQ buffers the
device can still DMA into -- a DMA-after-free on systems without an
IOMMU to fault the stale access.  Leak the HWC resources on teardown
failure instead of freeing memory the hardware can still reach, and
keep setup_active set so the failure is not mistaken for a clean
teardown.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes in v5:
 - No code changes since v4 (resend as a standalone thread).

Changes in v4:
 - Arm setup_active immediately after mana_smc_setup_hwc() succeeds.
 - Destroy the EQ (IRQ deregister + drain) before the CQ.
 - Dropped the redundant teardown in mana_hwc_establish_channel() that
   caused a double hardware timeout and masked the original error code.

 .../net/ethernet/microsoft/mana/hw_channel.c  | 56 ++++++++++++++-----
 include/net/mana/gdma.h                       |  8 ++-
 include/net/mana/hw_channel.h                 |  9 +++
 3 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 5db8cfe2d844..adc7ad98ca8d 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -4,6 +4,7 @@
 #include <net/mana/gdma.h>
 #include <net/mana/mana.h>
 #include <net/mana/hw_channel.h>
+#include <linux/pci.h>
 #include <linux/vmalloc.h>
 
 static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, u16 *msg_id)
@@ -792,6 +793,15 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	if (err)
 		return err;
 
+	/* setup_hwc activated MST entries — hardware can now DMA into our
+	 * queue buffers.  Record that in setup_active so the caller's error
+	 * path (mana_hwc_create_channel() -> mana_hwc_destroy_channel())
+	 * tears the HWC down exactly once.  Do not also tear down here: a
+	 * second teardown would double the hardware timeout on failure and
+	 * mask the original error code.
+	 */
+	hwc->setup_active = true;
+
 	if (!wait_for_completion_timeout(&hwc->hwc_init_eqe_comp, 60 * HZ))
 		return -ETIMEDOUT;
 
@@ -926,11 +936,38 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	if (!hwc)
 		return;
 
-	/* gc->max_num_cqs is set in mana_hwc_init_event_handler(). If it's
-	 * non-zero, the HWC worked and we should tear down the HWC here.
+	/* Tear down the HWC if setup_hwc previously activated MST entries.
+	 * This is the definitive flag — unlike max_num_cqs which depends
+	 * on the init EQE arriving.
+	 *
+	 * If teardown fails the device may still have active MST entries
+	 * and can DMA into the HWC queue buffers.  Freeing them would risk
+	 * memory corruption on systems without an IOMMU to fault the stale
+	 * DMA, so leak the HWC resources instead of handing the pages back
+	 * to the allocator.  Keep setup_active set so the failure is not
+	 * mistaken for a clean teardown.
+	 */
+	if (hwc->setup_active) {
+		int td_err = mana_smc_teardown_hwc(&gc->shm_channel, false);
+
+		if (td_err) {
+			dev_err(gc->dev,
+				"HWC teardown failed: %d, leaking resources\n",
+				td_err);
+			return;
+		}
+
+		hwc->setup_active = false;
+	}
+
+	/* Tear down the HWC CQ object first — mana_hwc_destroy_cq()
+	 * both unpublishes the CQ from cq_table (+synchronize_rcu) and
+	 * deregisters the HWC EQ from the interrupt handler list (via
+	 * mana_gd_deregister_irq + synchronize_rcu), guaranteeing no
+	 * interrupt handler can access RQ/TXQ buffers after this point.
 	 */
-	if (gc->max_num_cqs > 0)
-		mana_smc_teardown_hwc(&gc->shm_channel, false);
+	if (hwc->cq)
+		mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
 
 	if (hwc->txq)
 		mana_hwc_destroy_wq(hwc, hwc->txq);
@@ -938,17 +975,6 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	if (hwc->rxq)
 		mana_hwc_destroy_wq(hwc, hwc->rxq);
 
-	if (hwc->cq)
-		mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
-
-	/* Reset only after mana_hwc_destroy_cq() above has run with a valid
-	 * max_num_cqs so mana_gd_destroy_cq() clears the CQ table slot and
-	 * waits out in-flight EQ handlers (synchronize_rcu) before the CQ is
-	 * freed.  Clearing it earlier would make that path early-return and
-	 * skip the slot clear, leaving a dangling cq_table entry.
-	 */
-	gc->max_num_cqs = 0;
-
 	kfree(hwc->caller_ctx);
 	hwc->caller_ctx = NULL;
 
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index b8b1b23f3c36..e59b8b31e834 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -416,7 +416,13 @@ struct gdma_context {
 	/* L2 MTU */
 	u16 adapter_mtu;
 
-	/* This maps a CQ index to the queue structure. */
+	/* Size of cq_table, i.e. the largest valid CQ index + 1.  Set once
+	 * when cq_table is allocated and treated as immutable for the
+	 * table's lifetime (a bound only) -- it is never reset on teardown.
+	 * cq_table == NULL is the sole "table torn down" signal, so every
+	 * cq_table[id] access must guard with both !cq_table (gone) and
+	 * id >= max_num_cqs (out of bounds).
+	 */
 	unsigned int		max_num_cqs;
 	/* Both the base pointer and each entry are RCU-managed.  The fast
 	 * path (mana_gd_process_eqe) reads the base via rcu_dereference()
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 787c6f96d5b5..8340abd36af6 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -206,6 +206,15 @@ struct hw_channel_context {
 	 */
 	u32 rx_leaked_wqe;
 
+	/* Set after mana_smc_setup_hwc() succeeds (hardware has active
+	 * MST entries).  Cleared only after mana_smc_teardown_hwc()
+	 * succeeds, on both the recoverable establish_channel path and the
+	 * terminal destroy_channel path.  If teardown fails it stays set:
+	 * establish_channel() skips its retry and destroy_channel() leaks
+	 * the HWC rather than free buffers the device may still DMA into.
+	 */
+	bool setup_active;
+
 	struct hwc_caller_ctx *caller_ctx;
 };
 
-- 
2.43.0

[PATCH net v5 6/7] net: mana: fix stale HWC response after command timeout

[Long Li]

The HWC freed a message slot (mana_hwc_put_msg_index) the instant
mana_hwc_send_request() timed out, while the hardware command was still
pending and caller_ctx.output_buf still pointed at the caller's response
buffer.  A late response then raced two ways:

  - handle_resp() runs in CQ interrupt context and memcpy()'d into
    output_buf after the sender had returned and its buffer was gone.

  - the freed slot was reused by the next request, so the stale
    response completed the wrong command with another request's data.

Give each caller_ctx a spinlock, a refcount and an -EINPROGRESS
sentinel (and change caller_ctx::error from u32 to int so it holds
the negative errno values, including the sentinel, without relying on
unsigned wraparound):

  - The sender publishes output_buf under the slot lock and NULLs it
    under the same lock on timeout/exit, so handle_resp() (also under
    the lock) skips the copy once the sender is gone.

  - The slot is released only when both the sender and handle_resp()
    have dropped their reference, so a msg_id whose response is still
    outstanding is never handed to a new request.

  - Both references are taken up front in mana_hwc_get_msg_index(),
    under the same lock that publishes the slot, so a stale, duplicate
    or early response that arrives before the sender posts drops only
    the response-side reference and cannot release the slot out from
    under the sender.  A per-slot "responded" flag drops the payload of
    any such extra response.

  - On a genuine timeout the channel is marked hwc_timed_out and further
    mana_hwc_get_msg_index() callers fail with -ETIMEDOUT instead of
    reusing a slot whose response may still arrive.  The flag is read
    with READ_ONCE() outside the bitmap lock and written with
    WRITE_ONCE() under it.

Replace the counting semaphore with a waitqueue + bitmap so a slot held
past a timeout does not deadlock admission and timed-out waiters can be
released.

Because the timeout latch keys off wait_for_completion_timeout()
returning immediately, a zero hwc_timeout would time out every command
at once and latch the whole channel.  Ignore a device-reported zero from
both sources that feed hwc_timeout -- the HWC_DATA_CFG_HWC_TIMEOUT
reconfig event and the GDMA_QUERY_HWC_TIMEOUT response -- and keep the
positive default instead.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes in v5:
 - No code changes since v4 (resend as a standalone thread).

Changes in v4:
 - Take both the sender and response-side references up front in
   mana_hwc_get_msg_index() (refcount initialised to 2, under the lock
   that publishes the slot) so an early/stale/forged response cannot
   free the slot before the sender posts; the pre-post error path
   latches ->responded to avoid a double drop.
 - Changed caller_ctx::error from u32 to int so it holds the negative
   -EINPROGRESS sentinel and errno values directly.
 - Reject a zero firmware-supplied HWC timeout in the query path as well
   as the reconfig path.
 - Access hwc_timed_out with READ_ONCE()/WRITE_ONCE(); comment and
   changelog fixes.

 .../net/ethernet/microsoft/mana/gdma_main.c   |   7 +-
 .../net/ethernet/microsoft/mana/hw_channel.c  | 218 +++++++++++++++---
 include/net/mana/hw_channel.h                 |  27 ++-
 3 files changed, 214 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 7714040d1df4..9c2f5e0cfcbc 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -310,7 +310,12 @@ static int mana_gd_query_hwc_timeout(struct pci_dev *pdev, u32 *timeout_val)
 	if (err || resp.hdr.status)
 		return err ? err : -EPROTO;
 
-	*timeout_val = resp.timeout_ms;
+	/* A zero timeout would make every HWC command time out immediately
+	 * and latch the channel (see the HWC_DATA_CFG_HWC_TIMEOUT handler).
+	 * Ignore a zero from the device and keep the caller's positive value.
+	 */
+	if (resp.timeout_ms)
+		*timeout_val = resp.timeout_ms;
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index adc7ad98ca8d..2f0dae353955 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -7,25 +7,58 @@
 #include <linux/pci.h>
 #include <linux/vmalloc.h>
 
+/* Acquire a free message slot from the inflight bitmap.  Returns
+ * -ETIMEDOUT if a prior HWC command has timed out (preserving the
+ * error code callers expect).
+ */
 static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, u16 *msg_id)
 {
 	struct gdma_resource *r = &hwc->inflight_msg_res;
 	unsigned long flags;
 	u32 index;
 
-	down(&hwc->sema);
+	for (;;) {
+		spin_lock_irqsave(&r->lock, flags);
 
-	spin_lock_irqsave(&r->lock, flags);
+		if (hwc->hwc_timed_out) {
+			spin_unlock_irqrestore(&r->lock, flags);
+			return -ETIMEDOUT;
+		}
 
-	index = find_first_zero_bit(hwc->inflight_msg_res.map,
-				    hwc->inflight_msg_res.size);
+		index = find_first_zero_bit(r->map, r->size);
+		if (index < r->size) {
+			struct hwc_caller_ctx *ctx;
+
+			bitmap_set(r->map, index, 1);
+			ctx = &hwc->caller_ctx[index];
+			reinit_completion(&ctx->comp_event);
+			/* Take the response-side reference here, under
+			 * r->lock and together with the slot bitmap bit,
+			 * so a stale or duplicate response that lands
+			 * before mana_hwc_send_request() posts the request
+			 * cannot drop the refcount to zero and free the
+			 * slot under the sender.  One reference is the
+			 * sender's; the other is released by
+			 * mana_hwc_handle_resp().
+			 */
+			refcount_set(&ctx->refcnt, 2);
+			ctx->responded = false;
+			ctx->msg_id = index;
+			ctx->error = -EINPROGRESS;
+			spin_unlock_irqrestore(&r->lock, flags);
+			break;
+		}
+		spin_unlock_irqrestore(&r->lock, flags);
 
-	bitmap_set(hwc->inflight_msg_res.map, index, 1);
+		wait_event(hwc->msg_waitq,
+			   READ_ONCE(hwc->hwc_timed_out) ||
+			   !bitmap_full(r->map, r->size));
 
-	spin_unlock_irqrestore(&r->lock, flags);
+		if (READ_ONCE(hwc->hwc_timed_out))
+			return -ETIMEDOUT;
+	}
 
 	*msg_id = index;
-
 	return 0;
 }
 
@@ -35,10 +68,17 @@ static void mana_hwc_put_msg_index(struct hw_channel_context *hwc, u16 msg_id)
 	unsigned long flags;
 
 	spin_lock_irqsave(&r->lock, flags);
-	bitmap_clear(hwc->inflight_msg_res.map, msg_id, 1);
+	bitmap_clear(r->map, msg_id, 1);
 	spin_unlock_irqrestore(&r->lock, flags);
 
-	up(&hwc->sema);
+	wake_up(&hwc->msg_waitq);
+}
+
+static void hwc_ctx_put(struct hw_channel_context *hwc,
+			struct hwc_caller_ctx *ctx)
+{
+	if (refcount_dec_and_test(&ctx->refcnt))
+		mana_hwc_put_msg_index(hwc, ctx->msg_id);
 }
 
 static int mana_hwc_verify_resp_msg(const struct hwc_caller_ctx *caller_ctx,
@@ -116,22 +156,44 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
 		resp_len = 0;
 	}
 
-	err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
-	if (err)
-		goto out;
+	spin_lock(&ctx->lock);
 
-	ctx->status_code = resp_msg->status;
+	if (ctx->responded) {
+		/* A response for this slot was already delivered; this is a
+		 * duplicate or replayed one.  Drop it so the hwc_ctx_put()
+		 * a first response performs is not done twice, which would
+		 * over-release the slot while the sender still owns it.
+		 */
+		spin_unlock(&ctx->lock);
+		mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
+		return;
+	}
+	ctx->responded = true;
 
-	memcpy(ctx->output_buf, resp_msg, resp_len);
-out:
-	ctx->error = err;
+	err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
+
+	if (!err && ctx->output_buf) {
+		ctx->status_code = resp_msg->status;
+		memcpy(ctx->output_buf, resp_msg, resp_len);
+		ctx->error = 0;
+	} else if (ctx->output_buf) {
+		/* Record the error only while the sender still owns the
+		 * request: a non-NULL output_buf means it is still waiting.
+		 * Once it has timed out (or been force-completed by destroy)
+		 * it clears output_buf and takes its own error, so a late
+		 * response must not write ctx->error or the buffer here.
+		 */
+		ctx->error = err;
+	}
 
-	/* Must post rx wqe before complete(), otherwise the next rx may
-	 * hit no_wqe error.
+	/* Post RX WQE before completing — the next response may arrive
+	 * immediately and needs a posted buffer.
 	 */
 	mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
-
 	complete(&ctx->comp_event);
+	spin_unlock(&ctx->lock);
+
+	hwc_ctx_put(hwc, ctx);
 }
 
 static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
@@ -218,7 +280,12 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
 
 		switch (type) {
 		case HWC_DATA_CFG_HWC_TIMEOUT:
-			hwc->hwc_timeout = val;
+			/* A zero timeout would make every command time out
+			 * immediately and latch hwc_timed_out, disabling the
+			 * channel.  Ignore it and keep the positive default.
+			 */
+			if (val)
+				hwc->hwc_timeout = val;
 			break;
 
 		case HWC_DATA_HW_LINK_CONNECT:
@@ -732,7 +799,7 @@ static int mana_hwc_init_inflight_msg(struct hw_channel_context *hwc,
 {
 	int err;
 
-	sema_init(&hwc->sema, num_msg);
+	init_waitqueue_head(&hwc->msg_waitq);
 
 	err = mana_gd_alloc_res_map(num_msg, &hwc->inflight_msg_res);
 	if (err)
@@ -762,8 +829,10 @@ static int mana_hwc_test_channel(struct hw_channel_context *hwc, u16 q_depth,
 	if (!ctx)
 		return -ENOMEM;
 
-	for (i = 0; i < q_depth; ++i)
+	for (i = 0; i < q_depth; ++i) {
+		spin_lock_init(&ctx[i].lock);
 		init_completion(&ctx[i].comp_event);
+	}
 
 	hwc->caller_ctx = ctx;
 
@@ -774,6 +843,9 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 				      u32 *max_req_msg_size,
 				      u32 *max_resp_msg_size)
 {
+	/* No RCU needed: called only from mana_hwc_create_channel
+	 * during init, before the channel is published to senders.
+	 */
 	struct hw_channel_context *hwc = gc->hwc.driver_data;
 	struct gdma_queue *rq = hwc->rxq->gdma_wq;
 	struct gdma_queue *sq = hwc->txq->gdma_wq;
@@ -1004,13 +1076,18 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 	struct hwc_wq *txq = hwc->txq;
 	struct gdma_req_hdr *req_msg;
 	struct hwc_caller_ctx *ctx;
+	unsigned long flags;
+	bool drop_resp_ref;
 	u32 dest_vrcq = 0;
 	u32 dest_vrq = 0;
 	u32 command;
+	u32 status;
 	u16 msg_id;
 	int err;
 
-	mana_hwc_get_msg_index(hwc, &msg_id);
+	err = mana_hwc_get_msg_index(hwc, &msg_id);
+	if (err)
+		return err;
 
 	tx_wr = &txq->msg_buf->reqs[msg_id];
 
@@ -1022,8 +1099,11 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 	}
 
 	ctx = hwc->caller_ctx + msg_id;
+
+	spin_lock_irqsave(&ctx->lock, flags);
 	ctx->output_buf = resp;
 	ctx->output_buflen = resp_len;
+	spin_unlock_irqrestore(&ctx->lock, flags);
 
 	req_msg = (struct gdma_req_hdr *)tx_wr->buf_va;
 	if (req)
@@ -1039,6 +1119,10 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 		dest_vrcq = hwc->pf_dest_vrcq_id;
 	}
 
+	/* handle_resp()'s reference was taken in mana_hwc_get_msg_index(),
+	 * so hardware responding immediately after the doorbell ring cannot
+	 * release the slot before this sender is done with it.
+	 */
 	err = mana_hwc_post_tx_wqe(txq, tx_wr, dest_vrq, dest_vrcq, false);
 	if (err) {
 		dev_err(hwc->dev, "HWC: Failed to post send WQE: %d\n", err);
@@ -1051,31 +1135,95 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 			dev_err(hwc->dev, "Command 0x%x timed out: %u ms\n",
 				command, hwc->hwc_timeout);
 
-		/* Reduce further waiting if HWC no response */
+		/* NULL out output_buf so a late handle_resp() won't write
+		 * into the caller's buffer after the sender returns, then
+		 * check whether handle_resp() already delivered a valid
+		 * response between the timeout firing and this lock
+		 * acquisition — ctx->error != -EINPROGRESS means it ran.
+		 */
+		spin_lock_irqsave(&ctx->lock, flags);
+		ctx->output_buf = NULL;
+		err = ctx->error;
+		status = ctx->status_code;
+		spin_unlock_irqrestore(&ctx->lock, flags);
+
+		if (err != -EINPROGRESS) {
+			/* handle_resp() delivered a valid response just after
+			 * the timeout fired.  The hardware is alive, so use
+			 * the response and leave the channel usable; do not
+			 * latch hwc_timed_out or degrade hwc_timeout for what
+			 * turned out to be a transient race.
+			 */
+			hwc_ctx_put(hwc, ctx);
+			goto check_status;
+		}
+
+		/* Genuine timeout: no response arrived.  Reduce further
+		 * waiting, and mark the channel timed out under the bitmap
+		 * lock so get_msg_index() cannot acquire new slots after this.
+		 */
 		if (hwc->hwc_timeout > 1)
 			hwc->hwc_timeout = 1;
 
+		spin_lock_irqsave(&hwc->inflight_msg_res.lock, flags);
+		WRITE_ONCE(hwc->hwc_timed_out, true);
+		spin_unlock_irqrestore(&hwc->inflight_msg_res.lock, flags);
+		wake_up_all(&hwc->msg_waitq);
+
 		err = -ETIMEDOUT;
-		goto out;
+		hwc_ctx_put(hwc, ctx);
+		goto done;
 	}
 
-	if (ctx->error) {
-		err = ctx->error;
-		goto out;
-	}
+	/* NULL output_buf so a late handle_resp() won't memcpy into
+	 * the caller's buffer after the sender exits.  Read error and
+	 * status_code under the same lock — after hwc_ctx_put the slot
+	 * may be reused and these fields overwritten.
+	 */
+	spin_lock_irqsave(&ctx->lock, flags);
+	ctx->output_buf = NULL;
+	err = ctx->error;
+	status = ctx->status_code;
+	spin_unlock_irqrestore(&ctx->lock, flags);
+	hwc_ctx_put(hwc, ctx);
+
+check_status:
+	if (err)
+		goto done;
 
-	if (ctx->status_code && ctx->status_code != GDMA_STATUS_MORE_ENTRIES) {
-		if (ctx->status_code == GDMA_STATUS_CMD_UNSUPPORTED) {
+	if (status && status != GDMA_STATUS_MORE_ENTRIES) {
+		if (status == GDMA_STATUS_CMD_UNSUPPORTED) {
 			err = -EOPNOTSUPP;
-			goto out;
+			goto done;
 		}
+
 		if (command != MANA_QUERY_PHY_STAT)
 			dev_err(hwc->dev, "Command 0x%x failed with status: 0x%x\n",
-				command, ctx->status_code);
+				command, status);
 		err = -EPROTO;
-		goto out;
+		goto done;
 	}
+
+	err = 0;
+	goto done;
 out:
-	mana_hwc_put_msg_index(hwc, msg_id);
+	/* Pre-post error paths: the request was never submitted, so in the
+	 * common case mana_hwc_handle_resp() will not run for this slot and
+	 * the sender must drop both the response-side reference taken in
+	 * mana_hwc_get_msg_index() and its own.  Guard against a stale or
+	 * forged response that raced in first: latch ->responded under the
+	 * lock so any later handle_resp() is a no-op, and drop the response-
+	 * side reference here only if handle_resp() has not already done so.
+	 */
+	ctx = hwc->caller_ctx + msg_id;
+	spin_lock_irqsave(&ctx->lock, flags);
+	ctx->output_buf = NULL;
+	drop_resp_ref = !ctx->responded;
+	ctx->responded = true;
+	spin_unlock_irqrestore(&ctx->lock, flags);
+	if (drop_resp_ref)
+		refcount_dec(&ctx->refcnt);
+	hwc_ctx_put(hwc, ctx);
+done:
 	return err;
 }
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 8340abd36af6..23bf83e2a3ec 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -171,8 +171,25 @@ struct hwc_caller_ctx {
 	void *output_buf;
 	u32 output_buflen;
 
-	u32 error; /* Linux error code */
+	int error; /* Linux error code (negative errno or 0) */
 	u32 status_code;
+
+	/* Protects output_buf against concurrent access from
+	 * handle_resp() (CQ interrupt) and the sender timeout path.
+	 */
+	spinlock_t lock;
+
+	/* Tracks sender + handle_resp ownership.  The last put
+	 * (refcount reaches 0) releases the bitmap slot.
+	 */
+	refcount_t refcnt;
+	u16 msg_id;
+
+	/* Set under lock by the first handle_resp() for this slot so a
+	 * duplicate or replayed response is dropped instead of consuming
+	 * the response-side reference a second time.
+	 */
+	bool responded;
 };
 
 struct hw_channel_context {
@@ -193,8 +210,9 @@ struct hw_channel_context {
 	struct hwc_wq *txq;
 	struct hwc_cq *cq;
 
-	struct semaphore sema;
 	struct gdma_resource inflight_msg_res;
+	/* Waitqueue for senders blocked on a full inflight bitmap. */
+	wait_queue_head_t msg_waitq;
 
 	u32 pf_dest_vrq_id;
 	u32 pf_dest_vrcq_id;
@@ -206,6 +224,11 @@ struct hw_channel_context {
 	 */
 	u32 rx_leaked_wqe;
 
+	/* Set on first HWC timeout.  Causes get_msg_index() to return
+	 * -ETIMEDOUT instead of waiting, draining all queued senders.
+	 */
+	bool hwc_timed_out;
+
 	/* Set after mana_smc_setup_hwc() succeeds (hardware has active
 	 * MST entries).  Cleared only after mana_smc_teardown_hwc()
 	 * succeeds, on both the recoverable establish_channel path and the
-- 
2.43.0

[PATCH net v5 7/7] net: mana: keep max_num_cqs immutable once cq_table is allocated

[Long Li]

mana_hwc_init_event_handler() applied every HWC_INIT_DATA_MAX_NUM_CQS
event straight to gc->max_num_cqs.  That handler stays live for the whole
channel lifetime -- it also services runtime reconfig and link events --
so it is not confined to the initial bootstrap.

gc->cq_table is allocated once, sized to the max_num_cqs seen at bootstrap,
and every reader (mana_gd_process_eqe(), mana_create_rxq() and
mana_create_txq()) bounds-checks a CQ index against gc->max_num_cqs before
indexing gc->cq_table.  A device -- or a malicious host in a confidential
VM -- that sends a later HWC_INIT_DATA_MAX_NUM_CQS with a larger value
inflates the bound past the allocation.  This includes an event timed to
land while mana_hwc_establish_channel() is between reading the count and
publishing cq_table.  A subsequent out-of-range CQ id then passes the
bounds check and indexes cq_table out of bounds: an out-of-bounds read in
the EQ fast path, or an out-of-bounds pointer write in
mana_create_rxq()/mana_create_txq(), corrupting guest kernel memory.

Stop writing gc->max_num_cqs from the event handler.  Store the reported
value in hwc_init_max_num_cqs, and let mana_hwc_establish_channel() commit
it to gc->max_num_cqs once, from the same snapshot that sizes cq_table.
The handler store uses WRITE_ONCE() and the establish-time read uses
READ_ONCE(), since the two run concurrently (EQ interrupt vs process
context); the single, non-reloadable read is what guarantees the value
that sizes cq_table is the same one published as the bound, even across
the sleeping vcalloc().  gc->max_num_cqs then always matches the
allocation and no later event can change the bound after the table is
published, so the existing bounds checks are sufficient.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes in v5:
 - No code changes since v4 (resend as a standalone thread).

Changes in v4:
 - New patch in v4, split out of the v3 teardown-safety work in response
   to review: gc->max_num_cqs is set once when cq_table is allocated and
   never reset, so a spoofed post-init HWC event cannot inflate the
   bound past the allocation.

 .../net/ethernet/microsoft/mana/hw_channel.c  | 34 ++++++++++++++++---
 include/net/mana/hw_channel.h                 |  1 +
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 2f0dae353955..03b5e2f02e35 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -246,7 +246,15 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
 			break;
 
 		case HWC_INIT_DATA_MAX_NUM_CQS:
-			gd->gdma_context->max_num_cqs = val;
+			/* Store, don't apply: mana_hwc_establish_channel()
+			 * commits this to gc->max_num_cqs once, together
+			 * with sizing cq_table, so a spoofed post-init event
+			 * cannot inflate the bound past the allocation.
+			 * WRITE_ONCE() pairs with the READ_ONCE() there:
+			 * this store runs in EQ interrupt context,
+			 * concurrently with that process-context read.
+			 */
+			WRITE_ONCE(hwc->hwc_init_max_num_cqs, val);
 			break;
 
 		case HWC_INIT_DATA_PDID:
@@ -852,6 +860,8 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	struct gdma_queue *eq = hwc->cq->gdma_eq;
 	struct gdma_queue *cq = hwc->cq->gdma_cq;
 	struct gdma_queue __rcu **cq_table;
+	u32 num_cqs;
+	u32 cq_id;
 	int err;
 
 	init_completion(&hwc->hwc_init_eqe_comp);
@@ -881,15 +891,29 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	*max_req_msg_size = hwc->hwc_init_max_req_msg_size;
 	*max_resp_msg_size = hwc->hwc_init_max_resp_msg_size;
 
-	/* Both were set in mana_hwc_init_event_handler(). */
-	if (WARN_ON(cq->id >= gc->max_num_cqs))
+	/* Snapshot the device-reported CQ count and CQ id into locals and
+	 * use only the locals below, so the same value that sizes cq_table
+	 * also bounds and indexes it -- even across the sleeping vcalloc().
+	 * Both fields are written by mana_hwc_init_event_handler() from EQ
+	 * interrupt context: hwc_init_max_num_cqs under WRITE_ONCE() (paired
+	 * here), and cq->id as an ordinary store.  READ_ONCE() keeps each
+	 * read tear-free and, crucially, non-reloadable, so a spoofed
+	 * post-init event cannot make the WARN_ON() pass against one value
+	 * while the allocation or the index uses another.
+	 */
+	num_cqs = READ_ONCE(hwc->hwc_init_max_num_cqs);
+	cq_id = READ_ONCE(cq->id);
+
+	if (WARN_ON(cq_id >= num_cqs))
 		return -EPROTO;
 
-	cq_table = vcalloc(gc->max_num_cqs, sizeof(*cq_table));
+	cq_table = vcalloc(num_cqs, sizeof(*cq_table));
 	if (!cq_table)
 		return -ENOMEM;
 
-	rcu_assign_pointer(cq_table[cq->id], cq);
+	gc->max_num_cqs = num_cqs;
+
+	rcu_assign_pointer(cq_table[cq_id], cq);
 	/* Publish the fully-initialised table last; pairs with the
 	 * rcu_dereference(gc->cq_table) in mana_gd_process_eqe().
 	 */
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 23bf83e2a3ec..c275150baf49 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -203,6 +203,7 @@ struct hw_channel_context {
 	u16 hwc_init_q_depth_max;
 	u32 hwc_init_max_req_msg_size;
 	u32 hwc_init_max_resp_msg_size;
+	u32 hwc_init_max_num_cqs;
 
 	struct completion hwc_init_eqe_comp;
 
-- 
2.43.0