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

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

[Long Li]

This series fixes a set of reliability and safety bugs in the MANA HW
communication channel (HWC) and hardens the paths that consume
device-supplied values.  The HWC bootstrap and RX data come from the PF,
which is untrusted from a confidential-VM guest, so several of these are
also hardening fixes against a malicious or buggy host.

Patch 1 reference-counts the CQs looked up from the EQ interrupt handler so
a completion cannot race a concurrent CQ destroy.  Patches 2-3 fix an
RQ/SQ size swap and a use-after-free of the HWC comp_buf during teardown.
Patch 4 validates the lengths and indices taken from device DMA in the HWC
RX path.  Patch 5 makes HWC teardown safe when a device stops responding.
Patch 6 stops a stale/late HWC response from completing the wrong command
after a timeout.  Patch 7 keeps max_num_cqs immutable once cq_table is
allocated, so a later device event cannot inflate the bound past the
allocation.

The series has been build- and sparse-tested (C=2), and checkpatch is
clean on every patch.

Changes since v6:
- Patch 1: reworked from the lock-based scheme to lockless reference
  counting per review feedback.  Dropped gc->cq_table_lock entirely;
  lookups now take a reference under RCU (refcount_inc_not_zero) and the
  CQ is freed with kfree_rcu(), publish/unpublish are lockless.  Reordered
  mana_ib_destroy_cq() to detach the software callback before destroying
  the hardware CQ, closing a CQ-id recycle window.  Retitled accordingly
  ("reference-count CQs looked up from the EQ handler").
- Patch 4: bound the RX slot index by msg_buf->num_reqs (the __counted_by
  array bound) rather than the queue depth, and rate-limit the
  device-triggered RX error messages.
- Patch 5: the leak-on-teardown-failure branch now deregisters the HWC EQ
  IRQ and unpublishes the CQ before returning, so no late EQE can reach the
  leaked buffers.  Corrected the setup_active kerneldoc comment.
- Patch 6: dropped the terminal "timed out" latch, which could stop
  teardown commands from ever being posted to a slow device; the core
  stale-response fix (per-slot lock/refcount/responded flag) and the
  wait-queue admission change are retained, and the timeout still shortens
  later waits so teardown is posted.
- Patch 7: reject an out-of-range CQ id with a rate-limited error and
  -EPROTO instead of WARN_ON(), since both operands are device-controlled
  and WARN_ON() could panic a panic_on_warn guest.
- Patches 2, 3: commit-message wording only; no code change.

The v6 posting is at:
https://lore.kernel.org/netdev/20260811023823.2391255-1-longli@microsoft.com/

Long Li (7):
  net: mana: reference-count CQs looked up from the EQ handler
  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               |  41 +-
 .../net/ethernet/microsoft/mana/gdma_main.c   | 111 ++++-
 .../net/ethernet/microsoft/mana/hw_channel.c  | 402 ++++++++++++++----
 drivers/net/ethernet/microsoft/mana/mana_en.c |   8 +-
 include/net/mana/gdma.h                       |  29 +-
 include/net/mana/hw_channel.h                 |  34 +-
 6 files changed, 515 insertions(+), 110 deletions(-)


base-commit: f1b3416ceaf7ca4cb5cbd986ee8fe3ffaeda2d48
-- 
2.43.0

[PATCH net v7 1/7] net: mana: reference-count CQs looked up from the EQ handler

[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.  cq_table was a plain array,
read without RCU and freed without a grace period, so a concurrent CQ
teardown races the lookup 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

Reference-count the CQ, like the driver's existing QP get/put.  Mark
cq_table __rcu and look it up under the handler's rcu_read_lock():
mana_gd_get_cq() takes a reference with refcount_inc_not_zero() and
mana_gd_put_cq() drops it after the callback.  Teardown clears the slot,
drops the publish reference, waits for any in-flight handler, then frees
the CQ with kfree_rcu().

On the RDMA destroy path (mana_ib_destroy_cq) clear the dispatch entry
before destroying the HW CQ.  A late completion then finds an empty slot
and is dropped, and a cq_id the device recycles cannot alias the
outgoing entry.

The cq_id bound is hardened in a later patch.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes since v6:
Reworked from the v6 lock-based RCU scheme to lockless reference
counting per review feedback (Leon Romanovsky):
- Dropped gc->cq_table_lock; lookups take a reference under RCU with
  refcount_inc_not_zero() and the CQ is freed via kfree_rcu().
- publish/unpublish are lockless; the EQ-handler lookup uses
  smp_load_acquire() paired with the release-store that publishes the
  table.
- Reordered mana_ib_destroy_cq() to detach the software callback before
  destroying the hardware CQ, closing a CQ-id recycle window.
- Retitled (was "RCU-protect gc->cq_table lookups against concurrent CQ
  destroy").
 drivers/infiniband/hw/mana/cq.c               |  41 ++++---
 .../net/ethernet/microsoft/mana/gdma_main.c   | 100 +++++++++++++++---
 .../net/ethernet/microsoft/mana/hw_channel.c  |  29 +++--
 drivers/net/ethernet/microsoft/mana/mana_en.c |   8 +-
 include/net/mana/gdma.h                       |  23 +++-
 5 files changed, 160 insertions(+), 41 deletions(-)

diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
index f2547989f422901075fa19a1ba48daf3e9a1ec96..022c82479ef6c47d78bc64b638f18aa25bbd3308 100644
--- a/drivers/infiniband/hw/mana/cq.c
+++ b/drivers/infiniband/hw/mana/cq.c
@@ -108,11 +108,12 @@ int mana_ib_destroy_cq(struct ib_cq *ibcq, struct ib_udata *udata)
 
 	mdev = container_of(ibdev, struct mana_ib_dev, ib_dev);
 
+	/* Detach the dispatch entry first, then stop the HW CQ and free the
+	 * queue.  A completion racing teardown then finds an empty slot, and
+	 * a recycled cq_id cannot alias this CQ.  Errors are logged inside.
+	 */
 	mana_ib_remove_cq_cb(mdev, cq);
 
-	/* Ignore return code as there is not much we can do about it.
-	 * The error message is printed inside.
-	 */
 	mana_ib_gd_destroy_cq(mdev, cq);
 
 	mana_ib_destroy_queue(mdev, &cq->queue);
@@ -132,12 +133,8 @@ 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 *gdma_cq;
+	int err;
 
-	if (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])
-		return -EINVAL;
 	if (cq->queue.kmem)
 		gdma_cq = cq->queue.kmem;
 	else
@@ -149,23 +146,41 @@ 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;
-	return 0;
+
+	err = mana_gd_publish_cq(gc, gdma_cq);
+	if (err && !cq->queue.kmem)
+		kfree(gdma_cq);
+
+	return err;
 }
 
 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 || cq->queue.id >= gc->max_num_cqs)
 		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;
+	rcu_read_lock();
+	cq_table = READ_ONCE(gc->cq_table);
+	gdma_cq = cq_table ? rcu_dereference(cq_table[cq->queue.id]) : NULL;
+	/* Match the CQ under RCU so the slot cannot be freed mid-check. */
+	if (gdma_cq && gdma_cq->cq.context != cq)
+		gdma_cq = NULL;
+	rcu_read_unlock();
+
+	if (!gdma_cq)
+		return;
+
+	/* Remove from the table, then free after a grace period. */
+	mana_gd_unpublish_cq(gc, gdma_cq);
+	kfree_rcu(gdma_cq, rcu);
 }
 
 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 e8b7ffb47eb982d139b80bc4fb4bbb0ad5307962..b29e078b419b3c16326ad890c8e97401e1d3f3a9 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -724,6 +724,9 @@ int mana_schedule_serv_work(struct gdma_context *gc, enum gdma_eqe_type type)
 	return 0;
 }
 
+static struct gdma_queue *mana_gd_get_cq(struct gdma_context *gc, u32 cq_id);
+static void mana_gd_put_cq(struct gdma_queue *cq);
+
 static void mana_gd_process_eqe(struct gdma_queue *eq)
 {
 	u32 head = eq->head % (eq->queue_size / GDMA_EQE_SIZE);
@@ -743,16 +746,16 @@ 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))
-			break;
-
-		cq = gc->cq_table[cq_id];
-		if (WARN_ON_ONCE(!cq || cq->type != GDMA_CQ || cq->id != cq_id))
+		cq = mana_gd_get_cq(gc, cq_id);
+		/* CQ already torn down: stale completion, drop it. */
+		if (!cq)
 			break;
 
-		if (cq->cq.callback)
+		if (!WARN_ON_ONCE(cq->type != GDMA_CQ || cq->id != cq_id) &&
+		    cq->cq.callback)
 			cq->cq.callback(cq->cq.context, cq);
 
+		mana_gd_put_cq(cq);
 		break;
 
 	case GDMA_EQE_TEST_EVENT:
@@ -1050,18 +1053,81 @@ 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)
+static struct gdma_queue *mana_gd_get_cq(struct gdma_context *gc, u32 cq_id)
 {
-	u32 id = queue->id;
+	struct gdma_queue __rcu **cq_table;
+	struct gdma_queue *cq = NULL;
 
-	if (id >= gc->max_num_cqs)
-		return;
+	/* IRQ reader: a stray completion can race the table publish in
+	 * mana_hwc_establish_channel(), so the acquire pairs with its
+	 * smp_store_release() to see a consistent table and bound.
+	 */
+	cq_table = smp_load_acquire(&gc->cq_table);
+	if (cq_table && cq_id < gc->max_num_cqs) {
+		cq = rcu_dereference(cq_table[cq_id]);
+		/* Fails if the CQ is being torn down. */
+		if (cq && !refcount_inc_not_zero(&cq->cq.refcount))
+			cq = NULL;
+	}
+
+	return cq;
+}
 
-	if (!gc->cq_table[id])
+static void mana_gd_put_cq(struct gdma_queue *cq)
+{
+	if (cq && refcount_dec_and_test(&cq->cq.refcount))
+		complete(&cq->cq.free);
+}
+
+int mana_gd_publish_cq(struct gdma_context *gc, struct gdma_queue *queue)
+{
+	struct gdma_queue __rcu **cq_table;
+
+	/* Only mana_gd_get_cq() (IRQ) races the table publish and needs the
+	 * acquire; this control path does not.
+	 */
+	cq_table = READ_ONCE(gc->cq_table);
+	if (!cq_table || queue->id >= gc->max_num_cqs)
+		return -EINVAL;
+
+	/* Sharing a CQ between WQs is not supported. */
+	if (rcu_access_pointer(cq_table[queue->id]))
+		return -EINVAL;
+
+	refcount_set(&queue->cq.refcount, 1);
+	init_completion(&queue->cq.free);
+	rcu_assign_pointer(cq_table[queue->id], queue);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS(mana_gd_publish_cq, "NET_MANA");
+
+void mana_gd_unpublish_cq(struct gdma_context *gc, struct gdma_queue *queue)
+{
+	struct gdma_queue __rcu **cq_table;
+
+	/* Only mana_gd_get_cq() (IRQ) races the table publish and needs the
+	 * acquire; this control path does not.
+	 */
+	cq_table = READ_ONCE(gc->cq_table);
+	if (!cq_table || queue->id >= gc->max_num_cqs ||
+	    rcu_access_pointer(cq_table[queue->id]) != queue)
 		return;
 
-	gc->cq_table[id] = NULL;
+	rcu_assign_pointer(cq_table[queue->id], NULL);
+
+	/* Drop the publish reference and wait for any handler that already
+	 * took one, so the caller can free the CQ.
+	 */
+	mana_gd_put_cq(queue);
+	wait_for_completion(&queue->cq.free);
+}
+EXPORT_SYMBOL_NS(mana_gd_unpublish_cq, "NET_MANA");
+
+static void mana_gd_destroy_cq(struct gdma_context *gc,
+			       struct gdma_queue *queue)
+{
+	mana_gd_unpublish_cq(gc, queue);
 }
 
 int mana_gd_create_hwc_queue(struct gdma_dev *gd,
@@ -1333,7 +1399,13 @@ void mana_gd_destroy_queue(struct gdma_context *gc, struct gdma_queue *queue)
 
 	mana_gd_destroy_dma_region(gc, gmi->dma_region_handle);
 	mana_gd_free_memory(gmi);
-	kfree(queue);
+	/* The EQ handler may still be looking this CQ up; free it after a
+	 * grace period.
+	 */
+	if (queue->type == GDMA_CQ)
+		kfree_rcu(queue, rcu);
+	else
+		kfree(queue);
 }
 EXPORT_SYMBOL_NS(mana_gd_destroy_queue, "NET_MANA");
 
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index e3c24d50dad07c65be9e94129dc09af9264f9f8d..b5ed2dbce6ceb7f7a5196dfe5ba3534eb4c5d330 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,19 @@ 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;
+	/* Publish the initialised table; pairs with smp_load_acquire()
+	 * in mana_gd_get_cq().
+	 */
+	smp_store_release(&gc->cq_table, cq_table);
+
+	/* Publish the HWC CQ now that the table is in place. */
+	err = mana_gd_publish_cq(gc, cq);
+	if (WARN_ON(err))
+		return err;
 
 	return 0;
 }
@@ -811,6 +820,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 +828,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 +840,11 @@ 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() has cleared the CQ table
+	 * slot, so it is not left dangling.
+	 */
+	gc->max_num_cqs = 0;
+
 	kfree(hwc->caller_ctx);
 	hwc->caller_ctx = NULL;
 
@@ -848,8 +861,10 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	gc->hwc.driver_data = NULL;
 	gc->hwc.gdma_context = NULL;
 
-	vfree(gc->cq_table);
+	old_cq_table = gc->cq_table;
 	gc->cq_table = NULL;
+	/* All EQs are gone, so no EQ handler can be using the table. */
+	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 92bb55935c1c4e76e3912794eb3c4483fb331821..515b39f085c0d6519cc90a53b8d879c07e33264e 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2596,13 +2596,11 @@ 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)) {
+		if (WARN_ON(mana_gd_publish_cq(gc, cq->gdma_cq))) {
 			err = -EINVAL;
 			goto out;
 		}
 
-		gc->cq_table[cq->gdma_id] = cq->gdma_cq;
-
 		mana_create_txq_debugfs(apc, i);
 
 		set_bit(NAPI_STATE_NO_BUSY_POLL, &cq->napi.state);
@@ -2905,13 +2903,11 @@ 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)) {
+	if (WARN_ON(mana_gd_publish_cq(gc, cq->gdma_cq))) {
 		err = -EINVAL;
 		goto out;
 	}
 
-	gc->cq_table[cq->gdma_id] = cq->gdma_cq;
-
 	netif_napi_add_weight_locked(ndev, &cq->napi, mana_poll, 1);
 
 	WARN_ON(xdp_rxq_info_reg(&rxq->xdp_rxq, ndev, rxq_idx,
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 0c395917b2144ec4c2faafa5d6c7de7a452f1ebf..abf243358bf82e2428478cb3cf2f387d9cd9ea28 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -333,6 +333,9 @@ struct gdma_queue {
 	u32 tail;
 	struct list_head entry;
 
+	/* For kfree_rcu(): CQs are looked up locklessly from the EQ handler. */
+	struct rcu_head rcu;
+
 	/* Extra fields specific to EQ/CQ. */
 	union {
 		struct {
@@ -352,6 +355,12 @@ struct gdma_queue {
 			void *context;
 
 			struct gdma_queue *parent; /* For CQ/EQ relationship */
+
+			/* Keep the CQ alive while the EQ handler runs its
+			 * callback; teardown waits on @free.
+			 */
+			refcount_t refcount;
+			struct completion free;
 		} cq;
 	};
 };
@@ -418,7 +427,11 @@ struct gdma_context {
 
 	/* This maps a CQ index to the queue structure. */
 	unsigned int		max_num_cqs;
-	struct gdma_queue	**cq_table;
+	/* Entries are published/cleared by CQ create/destroy and read
+	 * locklessly by the EQ handler under RCU.  max_num_cqs is the table
+	 * size; NULL means the table is torn down.
+	 */
+	struct gdma_queue __rcu	**cq_table;
 
 	/* Protect eq_test_event and test_event_eq_id  */
 	struct mutex		eq_test_event_mutex;
@@ -496,6 +509,14 @@ int mana_gd_create_mana_wq_cq(struct gdma_dev *gd,
 
 void mana_gd_destroy_queue(struct gdma_context *gc, struct gdma_queue *queue);
 
+/* Add a CQ to cq_table so the EQ handler can dispatch to it.  Returns
+ * -EINVAL if the id is out of range or already in use.
+ */
+int mana_gd_publish_cq(struct gdma_context *gc, struct gdma_queue *queue);
+
+/* Remove a CQ from cq_table and wait for the EQ handler to stop using it. */
+void 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 v7 2/7] net: mana: fix HWC RQ/SQ buffer size swap

[Long Li]

mana_hwc_init_queues() sized the RQ (which receives responses) with
max_req_msg_size and the SQ (which sends requests) with max_resp_msg_size
-- backwards -- and mana_hwc_rx_event_handler() strided the RQ by
max_req_msg_size when recovering the RX slot index.

This is latent today: the only caller passes equal sizes (both 0x1000)
and the queues are never re-created with the hardware-reported sizes, so
nothing overflows.  It would only surface if the two sizes diverged.

Size the RQ by max_resp_msg_size and the SQ by max_req_msg_size, and use
max_resp_msg_size as the RX slot stride.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes since v6:
Commit-message wording only; no code change.
 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 b5ed2dbce6ceb7f7a5196dfe5ba3534eb4c5d330..ccef9bf9c6bfde754c28f86103f0b05489091f02 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",
@@ -737,14 +737,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);
@@ -753,6 +753,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 16feb39616c1bead1a043b3fadc2e18a90651516..73671f479399ac296cf472ec6449e5e6b00a8515 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 v7 3/7] net: mana: free HWC comp_buf after destroying the EQ

[Long Li]

mana_hwc_destroy_cq() freed comp_buf and the CQ before the EQ.  While the
EQ was still registered its handler could reach comp_buf (via
mana_hwc_comp_event()) and the CQ (via mana_hwc_init_event_handler()), so
a late EQE could touch freed memory.

Destroy the EQ first: mana_gd_destroy_queue() deregisters its IRQ and
waits out in-flight handlers, so no EQE can 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 since v6:
Commit-message and comment wording only; no code change.
 drivers/net/ethernet/microsoft/mana/hw_channel.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index ccef9bf9c6bfde754c28f86103f0b05489091f02..7e01596df11b639b1801bef7bdb09c91dfeb0543 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -384,14 +384,17 @@ 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 first: it deregisters the IRQ and drains in-flight
+	 * handlers, so none can touch the CQ after it is freed.
+	 */
+	if (hwc_cq->gdma_eq)
+		mana_gd_destroy_queue(gc, hwc_cq->gdma_eq);
 
+	/* Safe to free now that the EQ handler is fenced. */
 	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 v7 4/7] net: mana: validate hardware-supplied values in the HWC RX path

[Long Li]

mana_hwc_rx_event_handler() used 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 complete a wrong
or reused request or index out of bounds.  Validate before use:

  - read inline_oob_size_div4 once (through its u32 flags word) and reject
    any value but the one the driver programs, so a corrupted OOB size
    cannot move the SGE out of the WQE;
  - read sge->address once and require it to match the address the driver
    posted for that slot, not just an in-range index, so a wrong SGE
    cannot complete a neighbouring slot's request;
  - reject a resp_len larger than the RX buffer;
  - bounds-check hwc_msg_id before indexing the inflight bitmap and
    caller_ctx.

Repost the RX WQE on every early-return that can still identify its slot.
The cases that cannot -- a bad OOB size, an out-of-range index, or an
address matching no slot -- leak one WQE rather than repost the wrong one.
The RQ depth is never replenished, so count the leaks and, once they
exhaust it, log it and shorten the command timeout so callers fail fast.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes since v6:
- Bound the RX slot index by msg_buf->num_reqs (the __counted_by array
  bound) instead of the queue depth.
- Rate-limit the device-triggered RX rejection messages.
- Tightened the commit message.
 .../net/ethernet/microsoft/mana/hw_channel.c  | 85 ++++++++++++++++---
 include/net/mana/hw_channel.h                 |  5 ++
 2 files changed, 80 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 7e01596df11b639b1801bef7bdb09c91dfeb0543..2691d609459122bdbafb144a8b77ac3785c0eddb 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -83,13 +83,29 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
 	struct hwc_caller_ctx *ctx;
 	int err;
 
+	/* The caller already bounds msg_id; re-check at the indexing site. */
+	if (msg_id >= hwc->num_inflight_msg) {
+		dev_err_ratelimited(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);
+		dev_err_ratelimited(hwc->dev, "hwc_rx: invalid msg_id = %u\n", msg_id);
 		mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
 		return;
 	}
 
 	ctx = hwc->caller_ctx + msg_id;
+
+	/* An oversized resp_len cannot fit the RX buffer: reject it. */
+	if (resp_len > rx_req->buf_len) {
+		dev_err_ratelimited(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 +253,37 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
 	}
 }
 
+/* Drop an RX WQE with an untrusted SGE rather than repost it, which could
+ * double-post a buffer the device still owns.  This lowers the RQ depth;
+ * once it is exhausted the channel can no longer receive, so log it and
+ * shorten the timeout to fail callers fast.
+ */
+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);
+		if (hwc->hwc_timeout > 1)
+			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 +294,58 @@ 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);
+	/* inline_oob_size_div4 comes from device memory (host-writable in a
+	 * CVM), so read it once from the shared flags word.  The driver only
+	 * ever programs INLINE_OOB_SMALL_SIZE, so reject any other value.
+	 */
+	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_ratelimited(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);
 
-	/* Select the RX work request for virtual address and for reposting. */
+	/* Recover the RX slot from the SGE address (read once, it is device
+	 * memory).  Require both an in-range index and an exact address
+	 * match, so a wrong SGE cannot complete an unrelated request.
+	 */
+	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;
+	rx_req_idx = (sge_addr - 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);
+		/* Out-of-range index: corrupted SGE, leak the WQE. */
+		dev_err_ratelimited(hwc->dev,
+				    "HWC RX: SGE idx %llu >= num_reqs %u\n",
+				    rx_req_idx, hwc_rxq->msg_buf->num_reqs);
+		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) {
+		/* Address does not match the posted slot: leak the WQE. */
+		dev_err_ratelimited(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: it is host-writable DMA memory.  A short response
+	 * is left for mana_hwc_handle_resp() to reject, so it cannot stall
+	 * the 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);
+		dev_err_ratelimited(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 73671f479399ac296cf472ec6449e5e6b00a8515..58ea72f32135674d41fb1469e4cce4b0d8e87560 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -200,6 +200,11 @@ struct hw_channel_context {
 	u32 pf_dest_vrcq_id;
 	u32 hwc_timeout;
 
+	/* RX WQEs dropped after an untrusted SGE; at RQ depth the channel
+	 * can no longer receive.
+	 */
+	u32 rx_leaked_wqe;
+
 	struct hwc_caller_ctx *caller_ctx;
 };
 
-- 
2.43.0

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

[Long Li]

Three teardown hazards let the hardware touch freed memory.

First, once mana_smc_setup_hwc() succeeds the device can DMA into the HWC
queue buffers.  If a later step in mana_hwc_establish_channel() failed,
the caller had no reliable signal that teardown was needed and could free
those buffers while the mappings were live.  Add a setup_active flag, set
the moment setup_hwc() activates the device; on failure establish_channel()
just returns and the caller's error path does the single teardown, gated
on setup_active.  (max_num_cqs was the old proxy, but it is only set when
the init EQE arrives.)

Second, destroy_channel() freed the TXQ/RXQ while the HWC EQ was still on
the interrupt dispatch list, so an in-flight interrupt could run the
handler against freed buffers.  Destroy the CQ first --
mana_hwc_destroy_cq() deregisters the EQ (list_del_rcu() +
synchronize_rcu()) -- then free the TXQ/RXQ.

Third, if mana_smc_teardown_hwc() itself fails the MST entries stay live,
yet destroy_channel() went on to free buffers the device can still DMA
into.  Leak the HWC resources on teardown failure instead, but still
deregister the EQ IRQ and unpublish the CQ so the leaked buffers are
unreachable from the interrupt handler, and keep setup_active set so it 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 since v6:
- The leak-on-teardown-failure branch now fences the interrupt path
  before returning: it deregisters the HWC EQ IRQ (calling the existing
  mana_gd_destroy_eq(), now made non-static) and unpublishes the CQ, so
  no late EQE can reach the leaked buffers.
- Corrected the setup_active kerneldoc comment to match the code.
 .../net/ethernet/microsoft/mana/gdma_main.c   |  4 +-
 .../net/ethernet/microsoft/mana/hw_channel.c  | 63 ++++++++++++++++---
 include/net/mana/gdma.h                       |  6 ++
 include/net/mana/hw_channel.h                 |  6 ++
 4 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index b29e078b419b3c16326ad890c8e97401e1d3f3a9..02f901b2cb1faf4a34e6e3e53deebeeb056f528f 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -976,8 +976,8 @@ int mana_gd_test_eq(struct gdma_context *gc, struct gdma_queue *eq)
 	return err;
 }
 
-static void mana_gd_destroy_eq(struct gdma_context *gc, bool flush_evenets,
-			       struct gdma_queue *queue)
+void mana_gd_destroy_eq(struct gdma_context *gc, bool flush_evenets,
+			struct gdma_queue *queue)
 {
 	int err;
 
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 2691d609459122bdbafb144a8b77ac3785c0eddb..88188523dcd4863b254451e77cb569c12c150033 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -747,6 +747,13 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 
 	init_completion(&hwc->hwc_init_eqe_comp);
 
+	/* Set before setup_hwc() activates the device's DMA into our buffers,
+	 * so a later failure still tears the HWC down instead of freeing
+	 * buffers the device may write to.  Do not tear down here: that would
+	 * double the timeout and mask the error.
+	 */
+	hwc->setup_active = true;
+
 	err = mana_smc_setup_hwc(&gc->shm_channel, false,
 				 eq->mem_info.dma_handle,
 				 cq->mem_info.dma_handle,
@@ -837,6 +844,16 @@ int mana_hwc_create_channel(struct gdma_context *gc)
 	u16 q_depth_max;
 	int err;
 
+	/* A previous teardown may have failed and left the old context
+	 * reachable.  Retry it before building a new channel; if it still
+	 * fails, return an error so mana_serv_reset() does a full PCI rescan.
+	 */
+	if (gd->driver_data) {
+		mana_hwc_destroy_channel(gc);
+		if (gd->driver_data)
+			return -ETIMEDOUT;
+	}
+
 	hwc = kzalloc_obj(*hwc);
 	if (!hwc)
 		return -ENOMEM;
@@ -894,18 +911,40 @@ 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.
+	/* Only tear down if setup_hwc() activated the device.  If teardown
+	 * fails the device may still DMA into these buffers, so leak them
+	 * rather than free, and keep setup_active set.
 	 */
-	if (gc->max_num_cqs > 0)
-		mana_smc_teardown_hwc(&gc->shm_channel, false);
-
-	if (hwc->txq)
-		mana_hwc_destroy_wq(hwc, hwc->txq);
+	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);
+			/* The device may still DMA into these buffers, so
+			 * leak them.  Still fence the interrupt path: drop
+			 * the EQ from the handler list and unpublish the CQ,
+			 * and NULL them so a later retry does not touch the
+			 * leaked queues again.
+			 */
+			if (hwc->cq && hwc->cq->gdma_eq) {
+				mana_gd_destroy_eq(gc, false, hwc->cq->gdma_eq);
+				hwc->cq->gdma_eq = NULL;
+			}
+			if (hwc->cq && hwc->cq->gdma_cq) {
+				mana_gd_unpublish_cq(gc, hwc->cq->gdma_cq);
+				hwc->cq->gdma_cq = NULL;
+			}
+			return;
+		}
 
-	if (hwc->rxq)
-		mana_hwc_destroy_wq(hwc, hwc->rxq);
+		hwc->setup_active = false;
+	}
 
+	/* Tear down the CQ/EQ first so no interrupt handler can touch the
+	 * RQ/TXQ buffers after this point.
+	 */
 	if (hwc->cq)
 		mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
 
@@ -914,6 +953,12 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	 */
 	gc->max_num_cqs = 0;
 
+	if (hwc->txq)
+		mana_hwc_destroy_wq(hwc, hwc->txq);
+
+	if (hwc->rxq)
+		mana_hwc_destroy_wq(hwc, hwc->rxq);
+
 	kfree(hwc->caller_ctx);
 	hwc->caller_ctx = NULL;
 
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index abf243358bf82e2428478cb3cf2f387d9cd9ea28..1642d5413897c1dea49d1d4c5d80684163234e76 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -509,6 +509,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);
 
+/* Flush (optional), deregister the IRQ for, and disable an EQ, without
+ * freeing its queue memory.
+ */
+void mana_gd_destroy_eq(struct gdma_context *gc, bool flush_events,
+			struct gdma_queue *queue);
+
 /* Add a CQ to cq_table so the EQ handler can dispatch to it.  Returns
  * -EINVAL if the id is out of range or already in use.
  */
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 58ea72f32135674d41fb1469e4cce4b0d8e87560..6e77163a06d8f2622430281647268c54939c4dc0 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -205,6 +205,12 @@ struct hw_channel_context {
 	 */
 	u32 rx_leaked_wqe;
 
+	/* True once setup_hwc() may have activated the device's DMA into the
+	 * HWC buffers: set before the setup command, cleared after teardown
+	 * succeeds.
+	 */
+	bool setup_active;
+
 	struct hwc_caller_ctx *caller_ctx;
 };
 
-- 
2.43.0

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

[Long Li]

mana_hwc_send_request() freed the message slot (mana_hwc_put_msg_index)
as soon as it timed out, while the command was still pending and
caller_ctx.output_buf still pointed at the caller's buffer.  A late
response then either memcpy()'d into that buffer from CQ interrupt context
after the sender had returned, or completed the next request that reused
the slot with stale data.

Give each caller_ctx a spinlock, a refcount and an -EINPROGRESS sentinel
(caller_ctx::error becomes int to hold the negative value):

  - The sender publishes and clears output_buf under the slot lock;
    handle_resp() takes the same lock and skips the copy once it is NULL,
    so a late response can no longer write into a sender's buffer after
    it has returned.
  - Both references (sender and handle_resp) are taken up front in
    mana_hwc_get_msg_index(), so an early response cannot drop the slot
    under the sender, and a per-slot "responded" flag drops a duplicate
    response.

Responses are still correlated only by the reusable hwc_msg_id, so a
response that arrives after its slot has been released and reused can
still be delivered to the new owner; closing that fully needs a
per-request identity and is left to a separate change.

Replace the counting semaphore with a waitqueue + bitmap so a slot held
past a timeout cannot deadlock admission.

Ignore a device-reported zero hwc_timeout from both sources that feed it
and keep the positive default.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes since v6:
- Dropped the terminal "timed out" latch: it could stop teardown
  commands from ever being posted to a slow device.  The core
  stale-response fix (per-slot lock, refcount and "responded" flag) and
  the wait-queue admission change are retained, and a timeout still
  shortens later waits so teardown is still posted.
- Softened the changelog; the msg_id reuse correlation gap is pre-existing
  and noted as out of scope.
 .../net/ethernet/microsoft/mana/gdma_main.c   |   7 +-
 .../net/ethernet/microsoft/mana/hw_channel.c  | 186 ++++++++++++++----
 include/net/mana/hw_channel.h                 |  21 +-
 3 files changed, 173 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 02f901b2cb1faf4a34e6e3e53deebeeb056f528f..418e55aa033c3141f922fc7b38c9a44696dc9f0f 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 88188523dcd4863b254451e77cb569c12c150033..b1269f7da0563a22c3cbe599df55572e36908139 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -6,25 +6,41 @@
 #include <net/mana/hw_channel.h>
 #include <linux/vmalloc.h>
 
+/* Acquire a free inflight message slot, waiting for one if all are in use. */
 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);
-
-	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(hwc->inflight_msg_res.map, index, 1);
+			ctx = &hwc->caller_ctx[index];
+			reinit_completion(&ctx->comp_event);
+			/* Take both references (sender + handle_resp) before
+			 * publishing the slot, so an early response cannot free
+			 * it under the sender.
+			 */
+			refcount_set(&ctx->refcnt, 2);
+			ctx->responded = false;
+			ctx->msg_id = index;
+			ctx->error = -EINPROGRESS;
+			/* Publish the slot last, after it is fully initialised. */
+			bitmap_set(r->map, index, 1);
+			spin_unlock_irqrestore(&r->lock, flags);
+			break;
+		}
+		spin_unlock_irqrestore(&r->lock, flags);
 
-	spin_unlock_irqrestore(&r->lock, flags);
+		wait_event(hwc->msg_waitq, !bitmap_full(r->map, r->size));
+	}
 
 	*msg_id = index;
-
 	return 0;
 }
 
@@ -34,10 +50,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,
@@ -106,22 +129,34 @@ 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;
+	/* Honour a response only while the sender owns the slot (output_buf
+	 * published) and has not already been answered; otherwise drop it as
+	 * premature, stale or duplicate without touching the refcount.
+	 */
+	if (!ctx->output_buf || ctx->responded) {
+		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:
+	err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
+	if (!err) {
+		ctx->status_code = resp_msg->status;
+		memcpy(ctx->output_buf, resp_msg, resp_len);
+	}
 	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,
@@ -208,7 +243,9 @@ 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;
+			/* Ignore a zero timeout; keep the positive default. */
+			if (val)
+				hwc->hwc_timeout = val;
 			break;
 
 		case HWC_DATA_HW_LINK_CONNECT:
@@ -695,7 +732,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)
@@ -725,8 +762,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;
 
@@ -737,6 +776,7 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 				      u32 *max_req_msg_size,
 				      u32 *max_resp_msg_size)
 {
+	/* Runs at init before the channel is used, so no locking is needed. */
 	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;
@@ -989,13 +1029,19 @@ 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;
+	u32 wait_ms;
 	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];
 
@@ -1007,8 +1053,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)
@@ -1024,43 +1073,104 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 		dest_vrcq = hwc->pf_dest_vrcq_id;
 	}
 
+	/* The response-side reference (from get_msg_index) keeps the slot
+	 * alive if hardware responds right after the doorbell.
+	 */
 	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);
 		goto out;
 	}
 
+	wait_ms = hwc->hwc_timeout;
 	if (!wait_for_completion_timeout(&ctx->comp_event,
-					 (msecs_to_jiffies(hwc->hwc_timeout)))) {
-		if (hwc->hwc_timeout != 0)
+					 msecs_to_jiffies(wait_ms))) {
+		if (wait_ms != 0)
 			dev_err(hwc->dev, "Command 0x%x timed out: %u ms\n",
-				command, hwc->hwc_timeout);
+				command, wait_ms);
+
+		/* Clear output_buf so a late response cannot write the caller's
+		 * buffer, then check whether one already arrived
+		 * (error != -EINPROGRESS).
+		 */
+		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) {
+			/* A valid response raced in just after the timeout;
+			 * the hardware is alive, so use it and keep the channel.
+			 */
+			hwc_ctx_put(hwc, ctx);
+			goto check_status;
+		}
+
+		err = -ETIMEDOUT;
+
+		/* No-wait teardown (hwc_timeout == 0) is expected to expire;
+		 * just release the slot so the next teardown command can reuse
+		 * it.
+		 */
+		if (wait_ms == 0)
+			goto out;
 
-		/* Reduce further waiting if HWC no response */
+		/* Genuine timeout: shorten later waits so subsequent commands
+		 * fail fast instead of each draining the full timeout.
+		 */
 		if (hwc->hwc_timeout > 1)
 			hwc->hwc_timeout = 1;
 
-		err = -ETIMEDOUT;
+		/* Release the slot via out:; a late response no longer touches
+		 * it, so the sender must drop the reference here.
+		 */
 		goto out;
 	}
 
-	if (ctx->error) {
-		err = ctx->error;
-		goto out;
-	}
+	/* Clear output_buf and read the result under the lock; the slot may
+	 * be reused after hwc_ctx_put().
+	 */
+	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);
+	/* Error, no-wait teardown, or timeout: drop the sender's and the
+	 * response-side references.  Latch ->responded so a racing response
+	 * is a no-op, and only drop the response-side ref if it has not.
+	 */
+	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 6e77163a06d8f2622430281647268c54939c4dc0..ceabdc6242573f13d1284a967ea73bc01698e37d 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -171,8 +171,24 @@ 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 by the first handle_resp(), or by the sender's timeout path,
+	 * so a later or duplicate response is dropped.
+	 */
+	bool responded;
 };
 
 struct hw_channel_context {
@@ -193,8 +209,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;
-- 
2.43.0

[PATCH net v7 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, but that handler stays live for the
whole channel lifetime, not just bootstrap.

cq_table is allocated once, sized to the bootstrap max_num_cqs, and every
reader bounds-checks a CQ index against gc->max_num_cqs before indexing
it.  A later event with a larger value -- from the device or a malicious
host -- inflates the bound past the allocation, so an out-of-range CQ id
then passes the check and indexes cq_table out of bounds (an OOB read in
the EQ path, or an OOB pointer write in mana_create_rxq()/txq()).

Stop writing gc->max_num_cqs from the handler.  Store the reported value
in hwc_init_max_num_cqs (WRITE_ONCE()) and let
mana_hwc_establish_channel() commit it to gc->max_num_cqs once
(READ_ONCE()), from the same snapshot that sizes cq_table.  The bound then
always matches the allocation and no later event can change it.

Reject an out-of-range CQ id with a rate-limited error and -EPROTO rather
than WARN_ON(): both operands are device-controlled -- a host that omits
MAX_NUM_CQS leaves the bound at 0 -- so WARN_ON() would let a malformed
response panic a panic_on_warn guest.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes since v6:
- Reject an out-of-range CQ id with a rate-limited dev_err() and -EPROTO
  instead of WARN_ON(): both operands are device-controlled, so WARN_ON()
  could panic a panic_on_warn guest.
- Tightened the commit message.
 .../net/ethernet/microsoft/mana/hw_channel.c  | 33 +++++++++++++++----
 include/net/mana/hw_channel.h                 |  1 +
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index b1269f7da0563a22c3cbe599df55572e36908139..d9bff4634dc35be772eaeea2c56bfe2562c9d6aa 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -209,7 +209,11 @@ 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 only; establish_channel() commits it to
+			 * max_num_cqs once, so a later event cannot grow the
+			 * bound past the allocation.  Pairs with its READ_ONCE().
+			 */
+			WRITE_ONCE(hwc->hwc_init_max_num_cqs, val);
 			break;
 
 		case HWC_INIT_DATA_PDID:
@@ -783,6 +787,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);
@@ -810,17 +816,32 @@ 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 count and id once, so the same value
+	 * sizes, bounds and indexes cq_table even across the sleeping
+	 * vcalloc() and a concurrent init event.
+	 */
+	num_cqs = READ_ONCE(hwc->hwc_init_max_num_cqs);
+	cq_id = READ_ONCE(cq->id);
+
+	/* Both operands come from untrusted HWC bootstrap events; a missing
+	 * MAX_NUM_CQS leaves num_cqs at 0.  Reject rather than WARN_ON() so a
+	 * malformed device response cannot panic a panic_on_warn guest.
+	 */
+	if (cq_id >= num_cqs) {
+		dev_err_ratelimited(hwc->dev,
+				    "HWC: bad CQ id %u >= max %u\n",
+				    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;
 
-	/* Publish the initialised table; pairs with smp_load_acquire()
-	 * in mana_gd_get_cq().
+	/* Publish the bound and the initialised table together; the release
+	 * pairs with smp_load_acquire() in mana_gd_get_cq().
 	 */
+	gc->max_num_cqs = num_cqs;
 	smp_store_release(&gc->cq_table, cq_table);
 
 	/* Publish the HWC CQ now that the table is in place. */
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index ceabdc6242573f13d1284a967ea73bc01698e37d..f6cac0b0e44c5abd72c308ddc9278e4d85ed41ab 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -202,6 +202,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