* [PATCH net v3 1/6] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy
2026-08-03 23:43 [PATCH net v3 0/6] net: mana: HW channel reliability and hardening fixes Long Li
@ 2026-08-03 23:43 ` Long Li
2026-08-03 23:43 ` [PATCH net v3 2/6] net: mana: fix HWC RQ/SQ buffer size swap Long Li
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Long Li @ 2026-08-03 23:43 UTC (permalink / raw)
To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
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.
Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
drivers/infiniband/hw/mana/cq.c | 46 ++++++++++++++++---
.../net/ethernet/microsoft/mana/gdma_main.c | 25 ++++++++--
.../net/ethernet/microsoft/mana/hw_channel.c | 29 ++++++++----
drivers/net/ethernet/microsoft/mana/mana_en.c | 22 +++++++--
include/net/mana/gdma.h | 21 ++++++++-
5 files changed, 119 insertions(+), 24 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..fa6fcc2475c1 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;
@@ -1053,15 +1055,28 @@ static void mana_gd_create_cq(const struct gdma_queue_spec *spec,
static void mana_gd_destroy_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)
+ /* No rcu_read_lock() here: mana_gd_destroy_cq() 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;
- if (!gc->cq_table[id])
+ if (!rcu_access_pointer(cq_table[id]))
return;
- gc->cq_table[id] = NULL;
+ rcu_assign_pointer(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..65b025e8f211 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2496,6 +2496,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 +2597,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);
@@ -2821,6 +2828,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 +2913,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);
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 0c395917b214..0b48ded0aecd 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -418,7 +418,26 @@ 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" is justified by teardown ordering, not by
+ * a lock: the base table is replaced+freed only by
+ * mana_hwc_destroy_channel() (and the create-time reinit), and every
+ * teardown path first runs mana_remove() + mana_rdma_remove(), which
+ * synchronously drain the netdev and the IB device
+ * (unregister_netdevice / ib_unregister_device) that bound all
+ * install/remove callers; the reinit case runs before either
+ * consumer is probed. So no slow-path caller can run while the base
+ * table is being freed.
+ */
+ struct gdma_queue __rcu * __rcu *cq_table;
/* Protect eq_test_event and test_event_eq_id */
struct mutex eq_test_event_mutex;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v3 2/6] net: mana: fix HWC RQ/SQ buffer size swap
2026-08-03 23:43 [PATCH net v3 0/6] net: mana: HW channel reliability and hardening fixes Long Li
2026-08-03 23:43 ` [PATCH net v3 1/6] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy Long Li
@ 2026-08-03 23:43 ` Long Li
2026-08-03 23:43 ` [PATCH net v3 3/6] net: mana: free HWC comp_buf after destroying the EQ Long Li
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Long Li @ 2026-08-03 23:43 UTC (permalink / raw)
To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
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. A response larger than the undersized RQ
buffer could overflow it, and mana_hwc_rx_event_handler() recovered the
RX slot index by dividing by the wrong size (max_req_msg_size).
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.
Store the queue dimensions before creating the CQ, which registers the
RX completion handler: that handler divides by max_resp_msg_size and
range-checks num_inflight_msg, so both must be set before it can run.
Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
.../net/ethernet/microsoft/mana/hw_channel.c | 18 ++++++++++++------
include/net/mana/hw_channel.h | 1 +
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 409e20caeccd..cbb56c764787 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",
@@ -721,6 +721,15 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
if (err)
return err;
+ /* Set the queue dimensions before creating the CQ: doing so
+ * registers mana_hwc_rx_event_handler(), which divides by
+ * hwc->max_resp_msg_size and range-checks hwc->num_inflight_msg.
+ * They must be valid before any RX completion can be delivered.
+ */
+ hwc->num_inflight_msg = q_depth;
+ hwc->max_req_msg_size = max_req_msg_size;
+ hwc->max_resp_msg_size = max_resp_msg_size;
+
/* CQ is shared by SQ and RQ, so CQ's queue depth is the sum of SQ
* queue depth and RQ queue depth.
*/
@@ -733,23 +742,20 @@ 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);
goto out;
}
- hwc->num_inflight_msg = q_depth;
- hwc->max_req_msg_size = max_req_msg_size;
-
return 0;
out:
/* mana_hwc_create_channel() will do the cleanup.*/
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v3 3/6] net: mana: free HWC comp_buf after destroying the EQ
2026-08-03 23:43 [PATCH net v3 0/6] net: mana: HW channel reliability and hardening fixes Long Li
2026-08-03 23:43 ` [PATCH net v3 1/6] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy Long Li
2026-08-03 23:43 ` [PATCH net v3 2/6] net: mana: fix HWC RQ/SQ buffer size swap Long Li
@ 2026-08-03 23:43 ` Long Li
2026-08-03 23:43 ` [PATCH net v3 4/6] net: mana: validate hardware-supplied values in the HWC RX path Long Li
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Long Li @ 2026-08-03 23:43 UTC (permalink / raw)
To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
mana_hwc_destroy_cq() freed hwc_cq->comp_buf before destroying the CQ and
EQ. comp_buf is dereferenced by mana_hwc_comp_event(), which the EQ
interrupt handler invokes; freeing it while the EQ was still registered
let a late handler touch freed memory.
Destroy the CQ and EQ first -- the EQ teardown deregisters the IRQ and
fences in-flight handlers -- then free comp_buf and hwc_cq.
Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
drivers/net/ethernet/microsoft/mana/hw_channel.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index cbb56c764787..d701c427fe47 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -384,14 +384,20 @@ 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);
-
if (hwc_cq->gdma_cq)
mana_gd_destroy_queue(gc, hwc_cq->gdma_cq);
+ /* comp_buf is reached only by mana_hwc_comp_event(), which the
+ * EQ handler invokes via cq_table[id]. The CQ destroy above
+ * already cleared that slot and ran synchronize_rcu(), so no
+ * handler can reach comp_buf once it returns. Destroying the EQ
+ * here additionally tears down the IRQ (defense in depth) before
+ * comp_buf and hwc_cq are freed below.
+ */
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v3 4/6] net: mana: validate hardware-supplied values in the HWC RX path
2026-08-03 23:43 [PATCH net v3 0/6] net: mana: HW channel reliability and hardening fixes Long Li
` (2 preceding siblings ...)
2026-08-03 23:43 ` [PATCH net v3 3/6] net: mana: free HWC comp_buf after destroying the EQ Long Li
@ 2026-08-03 23:43 ` Long Li
2026-08-03 23:43 ` [PATCH net v3 5/6] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering Long Li
2026-08-03 23:43 ` [PATCH net v3 6/6] net: mana: fix stale HWC response after command timeout Long Li
5 siblings, 0 replies; 7+ messages in thread
From: Long Li @ 2026-08-03 23:43 UTC (permalink / raw)
To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
mana_hwc_rx_event_handler() and mana_hwc_handle_resp() 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:
- bounds-check the SGE pointer located via the device-supplied
inline_oob_size_div4 before dereferencing it, so a corrupted OOB
size cannot push the read past the RQ buffer;
- 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;
- require the response to cover a full gdma_resp_hdr before reading
hwc_msg_id, so a short response cannot complete a slot with stale
bytes left by the buffer's previous occupant;
- bounds-check hwc_msg_id in mana_hwc_handle_resp() before indexing the
inflight bitmap and caller_ctx;
- reject a resp_len larger than the RX buffer.
Repost the RX WQE on every validation early-return so a rejected response
does not permanently shrink the posted RQ depth. The two paths that
cannot identify the slot (an out-of-bounds SGE pointer, or an SGE address
matching no posted slot) intentionally leak a single WQE rather than risk
reposting the wrong one.
Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
.../net/ethernet/microsoft/mana/hw_channel.c | 82 ++++++++++++++++++-
1 file changed, 78 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index d701c427fe47..e378b8ec97c9 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -83,6 +83,17 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
struct hwc_caller_ctx *ctx;
int err;
+ /* Validate msg_id is in range before using it to index bitmap
+ * and caller_ctx array. Malicious firmware could send
+ * out-of-range msg_id causing out-of-bounds access.
+ */
+ 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 +101,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;
@@ -261,19 +284,69 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
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 is read from the WQE in device-accessible RQ
+ * memory, so a malicious host in a CVM (or buggy firmware) could set
+ * it to push the SGE past the RQ buffer. Bounds-check the SGE it
+ * locates before dereferencing sge->address below. A validly posted
+ * WQE keeps the SGE inside the ring (worst case ends exactly at the
+ * buffer boundary); reject anything that would read past it. The
+ * slot cannot be trusted here, so leak this RX WQE rather than repost
+ * the wrong one -- as in the SGE-address mismatch path below.
+ */
+ if ((u8 *)(sge + 1) > (u8 *)rq->queue_mem_ptr + rq->queue_size) {
+ dev_err(hwc->dev, "HWC RX: SGE past RQ buffer, oob_div4=%u\n",
+ dma_oob->inline_oob_size_div4);
+ return;
+ }
+
+ /* Recover the originating RX slot from the SGE address. Of the three
+ * terms here only sge->address lives in device-accessible RQ memory;
+ * rq_base_addr and max_resp_msg_size are driver-private constants. An
+ * in-range but wrong/unaligned SGE (corrupted WQE, or a malicious host
+ * in a CVM) 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 to be in range AND the address
+ * to exactly match the value the driver posted for that slot.
+ */
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);
+ if (rx_req_idx >= hwc_rxq->queue_depth) {
+ /* Cannot trust which WQE this is, so we cannot safely repost
+ * it; leak one RX WQE and bail. An out-of-range index means
+ * a corrupted SGE from hardware (or host tampering), an
+ * unrecoverable device error.
+ */
+ dev_err(hwc->dev, "HWC RX: SGE idx %llu out of range\n",
+ rx_req_idx);
return;
}
rx_req = &hwc_rxq->msg_buf->reqs[rx_req_idx];
+ if (sge->address != (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->address, rx_req_idx);
+ return;
+ }
+
resp = (struct gdma_resp_hdr *)rx_req->buf_va;
+ /* Validate resp_len covers the response header before reading
+ * hwc_msg_id. A short response leaves stale data from the
+ * previous buffer occupant, which could match a live slot and
+ * complete the wrong request.
+ */
+ if (rx_oob->tx_oob_data_size < sizeof(*resp)) {
+ dev_err(hwc->dev, "HWC RX: short resp_len=%u\n",
+ rx_oob->tx_oob_data_size);
+ mana_hwc_post_rx_wqe(hwc_rxq, rx_req);
+ return;
+ }
+
/* Read msg_id once from DMA buffer to prevent TOCTOU:
* DMA memory is shared/unencrypted in CVMs - host can
* modify it between reads.
@@ -281,6 +354,7 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
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;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v3 5/6] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering
2026-08-03 23:43 [PATCH net v3 0/6] net: mana: HW channel reliability and hardening fixes Long Li
` (3 preceding siblings ...)
2026-08-03 23:43 ` [PATCH net v3 4/6] net: mana: validate hardware-supplied values in the HWC RX path Long Li
@ 2026-08-03 23:43 ` Long Li
2026-08-03 23:43 ` [PATCH net v3 6/6] net: mana: fix stale HWC response after command timeout Long Li
5 siblings, 0 replies; 7+ messages in thread
From: Long Li @ 2026-08-03 23:43 UTC (permalink / raw)
To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
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.
establish_channel() now tears down on any later failure and clears
setup_active once teardown succeeds; destroy_channel() gates teardown on
setup_active. 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>
---
.../net/ethernet/microsoft/mana/hw_channel.c | 82 ++++++++++++++-----
include/net/mana/gdma.h | 8 +-
include/net/mana/hw_channel.h | 9 ++
3 files changed, 77 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index e378b8ec97c9..1603968d7989 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)
@@ -768,20 +769,33 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
if (err)
return err;
- if (!wait_for_completion_timeout(&hwc->hwc_init_eqe_comp, 60 * HZ))
- return -ETIMEDOUT;
+ /* setup_hwc activated MST entries — hardware can now DMA into
+ * our queue buffers. If anything below fails, we must tear
+ * down before returning so the caller doesn't need to track
+ * whether setup_hwc succeeded.
+ */
+ hwc->setup_active = true;
+
+ if (!wait_for_completion_timeout(&hwc->hwc_init_eqe_comp, 60 * HZ)) {
+ err = -ETIMEDOUT;
+ goto teardown;
+ }
*q_depth = hwc->hwc_init_q_depth_max;
*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))
- return -EPROTO;
+ if (WARN_ON(cq->id >= gc->max_num_cqs)) {
+ err = -EPROTO;
+ goto teardown;
+ }
cq_table = vcalloc(gc->max_num_cqs, sizeof(*cq_table));
- if (!cq_table)
- return -ENOMEM;
+ if (!cq_table) {
+ err = -ENOMEM;
+ goto teardown;
+ }
rcu_assign_pointer(cq_table[cq->id], cq);
/* Publish the fully-initialised table last; pairs with the
@@ -790,6 +804,16 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
rcu_assign_pointer(gc->cq_table, cq_table);
return 0;
+
+teardown:
+ {
+ int td_err = mana_smc_teardown_hwc(&gc->shm_channel, false);
+
+ if (!td_err)
+ hwc->setup_active = false;
+
+ return td_err ? td_err : err;
+ }
}
static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
@@ -907,11 +931,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 (gc->max_num_cqs > 0)
- mana_smc_teardown_hwc(&gc->shm_channel, false);
+ 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 (hwc->cq)
+ mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
if (hwc->txq)
mana_hwc_destroy_wq(hwc, hwc->txq);
@@ -919,17 +970,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 0b48ded0aecd..74eccc80cf6b 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 73671f479399..3d8543acb5cc 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -200,6 +200,15 @@ struct hw_channel_context {
u32 pf_dest_vrcq_id;
u32 hwc_timeout;
+ /* 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v3 6/6] net: mana: fix stale HWC response after command timeout
2026-08-03 23:43 [PATCH net v3 0/6] net: mana: HW channel reliability and hardening fixes Long Li
` (4 preceding siblings ...)
2026-08-03 23:43 ` [PATCH net v3 5/6] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering Long Li
@ 2026-08-03 23:43 ` Long Li
5 siblings, 0 replies; 7+ messages in thread
From: Long Li @ 2026-08-03 23:43 UTC (permalink / raw)
To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
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:
- 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.
- A duplicate or replayed response for the same msg_id is dropped
via a per-slot "responded" flag, so the response-side reference is
consumed exactly once and cannot over-release the slot.
- 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.
Replace the depth-1 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, ignore a zero HWC_DATA_CFG_HWC_TIMEOUT reported
by the device: msecs_to_jiffies(0) would time out every command at once
and latch the whole channel. 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>
---
.../net/ethernet/microsoft/mana/hw_channel.c | 201 +++++++++++++++---
include/net/mana/hw_channel.h | 25 ++-
2 files changed, 190 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 1603968d7989..d92032b466af 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -7,25 +7,49 @@
#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);
+ refcount_set(&ctx->refcnt, 1);
+ 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,
+ hwc->hwc_timed_out ||
+ !bitmap_full(r->map, r->size));
- spin_unlock_irqrestore(&r->lock, flags);
+ if (hwc->hwc_timed_out)
+ return -ETIMEDOUT;
+ }
*msg_id = index;
-
return 0;
}
@@ -35,10 +59,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,
@@ -114,22 +145,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);
- /* Must post rx wqe before complete(), otherwise the next rx may
- * hit no_wqe error.
+ 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) {
+ /* Only overwrite error if the sender hasn't timed out
+ * or been force-completed by destroy. When output_buf
+ * is NULL, a terminal error (-ENODEV or timeout) has
+ * already been set — preserve it so the sender doesn't
+ * see a spurious success.
+ */
+ ctx->error = err;
+ }
+
+ /* 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,
@@ -216,7 +269,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:
@@ -708,7 +766,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)
@@ -738,8 +796,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;
@@ -750,6 +810,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;
@@ -999,13 +1062,17 @@ 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;
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];
@@ -1017,8 +1084,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)
@@ -1034,8 +1104,14 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
dest_vrcq = hwc->pf_dest_vrcq_id;
}
+ /* Take handle_resp's ref before posting — hardware can respond
+ * immediately after the doorbell ring.
+ */
+ refcount_inc(&ctx->refcnt);
+
err = mana_hwc_post_tx_wqe(txq, tx_wr, dest_vrq, dest_vrcq, false);
if (err) {
+ refcount_dec(&ctx->refcnt);
dev_err(hwc->dev, "HWC: Failed to post send WQE: %d\n", err);
goto out;
}
@@ -1046,31 +1122,86 @@ 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);
+ 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: no WQE was submitted so handle_resp()
+ * cannot race here. refcount is 1 (no second ref taken).
+ */
+ ctx = hwc->caller_ctx + msg_id;
+ spin_lock_irqsave(&ctx->lock, flags);
+ ctx->output_buf = NULL;
+ spin_unlock_irqrestore(&ctx->lock, flags);
+ 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 3d8543acb5cc..5a55cedf0607 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -173,6 +173,23 @@ struct hwc_caller_ctx {
u32 error; /* Linux error code */
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,13 +210,19 @@ 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;
u32 hwc_timeout;
+ /* 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
^ permalink raw reply related [flat|nested] 7+ messages in thread