* [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth
@ 2026-07-21 23:43 Long Li
2026-07-21 23:43 ` [PATCH net-next v2 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy Long Li
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Long Li @ 2026-07-21 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
This series hardens the MANA Hardware Channel (HWC) control-plane path
and then builds on that to support a dynamic HWC queue depth.
The HWC is the command channel the driver uses to talk to the device.
Today it is created at a fixed depth of one outstanding request, and
several of its lookup and teardown paths predate the RCU and DMA-lifetime
rules they now need to follow. Raising the queue depth and allowing
concurrent commands makes those latent races reachable, so the fixes come
first and the feature builds on them.
Patches 1-5 are fixes for pre-existing HWC bugs, each with a Fixes: tag:
1: cq_table was a plain pointer array freed with no grace period while
the EQ interrupt handler dereferenced it; put it under RCU.
2: the HWC RQ and SQ were sized with each other's message size, so a
response could overflow the RQ buffer and the RX slot stride was
computed with the wrong size.
3: comp_buf was freed before the EQ was destroyed, so a late completion
handler could touch freed memory.
4: the RX path consumed device-supplied lengths and indices without
validation; validate them before use (this matters for confidential
VMs, where the DMA buffer is shared with the host).
5: a failed mana_hwc_establish_channel() could leave live MST entries
while the driver freed the queue buffers, and destroy_channel() freed
the TXQ/RXQ before the EQ was quiesced; add a setup_active teardown
gate and destroy the CQ first.
Patches 6-7 add the feature:
6: replace the depth-1 semaphore with a slot bitmap and per-slot
synchronization so several management commands can be in flight,
with teardown that drains in-flight senders before freeing the HWC.
7: bootstrap the HWC at depth 1, query the device maximum and, if it is
larger, tear down and rebuild the queues at that depth. The reported
dimensions are validated before they size DMA allocations, and the
capability is advertised so firmware enables it only when the driver
supports it.
Changes since v1:
- patch 4: bounds-check the SGE pointer derived from the device-supplied
inline_oob_size_div4 before dereferencing it, so a corrupted OOB size
cannot push the read past the RQ buffer (found in review).
- patch 6: fix a use-after-free of gdma_context during teardown -- track
in-flight senders with a gc->hwc_lock-protected counter and drain it
with wait_event_lock_irq(), so the last sender's wake_up() cannot touch
gc after it is freed (found in review).
- patch 6: move the in-flight sender force-complete and drain ahead of
the teardown/FLR block, so the FLR-failure early return can no longer
skip the drain and strand a sender (found in review).
- Rebased on current net-next; kept all patches within 80 columns.
- v1: https://lore.kernel.org/netdev/20260715032942.3945317-1-longli@microsoft.com/
Long Li (7):
net: mana: RCU-protect gc->cq_table lookups against concurrent CQ
destroy
net: mana: fix HWC RQ/SQ buffer size swap
net: mana: free HWC comp_buf after destroying the EQ
net: mana: validate hardware-supplied values in the HWC RX path
net: mana: fix HWC teardown safety with setup_active flag and destroy
ordering
net: mana: support concurrent HWC requests with proper synchronization
net: mana: add dynamic HWC queue depth with reinit path
drivers/infiniband/hw/mana/cq.c | 46 +-
.../net/ethernet/microsoft/mana/gdma_main.c | 80 +-
.../net/ethernet/microsoft/mana/hw_channel.c | 742 ++++++++++++++++--
drivers/net/ethernet/microsoft/mana/mana_en.c | 22 +-
include/net/mana/gdma.h | 48 +-
include/net/mana/hw_channel.h | 44 +-
6 files changed, 890 insertions(+), 92 deletions(-)
base-commit: e354f7d60f14a3eacd5ec7b607346a5612f655ec
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v2 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy
2026-07-21 23:43 [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth Long Li
@ 2026-07-21 23:43 ` Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 2/7] net: mana: fix HWC RQ/SQ buffer size swap Long Li
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Long Li @ 2026-07-21 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 a38d4bb74621..9edebf7c097c 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -761,6 +761,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;
@@ -772,10 +773,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;
@@ -1082,15 +1084,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 a8c329bdbacf..5508746590e7 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2648,6 +2648,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;
@@ -2753,12 +2754,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);
@@ -2986,6 +2993,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;
@@ -3075,12 +3083,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 8529cef0d7c4..da52701e7816 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -430,7 +430,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] 13+ messages in thread
* [PATCH net-next v2 2/7] net: mana: fix HWC RQ/SQ buffer size swap
2026-07-21 23:43 [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth Long Li
2026-07-21 23:43 ` [PATCH net-next v2 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy Long Li
@ 2026-07-21 23:43 ` Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 3/7] net: mana: free HWC comp_buf after destroying the EQ Long Li
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Long Li @ 2026-07-21 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.
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 | 7 ++++---
include/net/mana/hw_channel.h | 1 +
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 409e20caeccd..3f011ebbe7b3 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -263,7 +263,7 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
/* Select the RX work request for virtual address and for reposting. */
rq_base_addr = hwc_rxq->msg_buf->mem_info.dma_handle;
- rx_req_idx = (sge->address - rq_base_addr) / hwc->max_req_msg_size;
+ rx_req_idx = (sge->address - rq_base_addr) / hwc->max_resp_msg_size;
if (rx_req_idx >= hwc_rxq->msg_buf->num_reqs) {
dev_err(hwc->dev, "HWC RX: wrong rx_req_idx=%llu, num_reqs=%u\n",
@@ -733,14 +733,14 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
goto out;
}
- err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth, max_req_msg_size,
+ err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth, max_resp_msg_size,
hwc->cq, &hwc->rxq);
if (err) {
dev_err(hwc->dev, "Failed to create HWC RQ: %d\n", err);
goto out;
}
- err = mana_hwc_create_wq(hwc, GDMA_SQ, q_depth, max_resp_msg_size,
+ err = mana_hwc_create_wq(hwc, GDMA_SQ, q_depth, max_req_msg_size,
hwc->cq, &hwc->txq);
if (err) {
dev_err(hwc->dev, "Failed to create HWC SQ: %d\n", err);
@@ -749,6 +749,7 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
hwc->num_inflight_msg = q_depth;
hwc->max_req_msg_size = max_req_msg_size;
+ hwc->max_resp_msg_size = max_resp_msg_size;
return 0;
out:
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 16feb39616c1..73671f479399 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -181,6 +181,7 @@ struct hw_channel_context {
u16 num_inflight_msg;
u32 max_req_msg_size;
+ u32 max_resp_msg_size;
u16 hwc_init_q_depth_max;
u32 hwc_init_max_req_msg_size;
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next v2 3/7] net: mana: free HWC comp_buf after destroying the EQ
2026-07-21 23:43 [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth Long Li
2026-07-21 23:43 ` [PATCH net-next v2 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy Long Li
2026-07-21 23:43 ` [PATCH net-next v2 2/7] net: mana: fix HWC RQ/SQ buffer size swap Long Li
@ 2026-07-21 23:43 ` Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 4/7] net: mana: validate hardware-supplied values in the HWC RX path Long Li
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Long Li @ 2026-07-21 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 3f011ebbe7b3..2239fdeda57c 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] 13+ messages in thread
* [PATCH net-next v2 4/7] net: mana: validate hardware-supplied values in the HWC RX path
2026-07-21 23:43 [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth Long Li
` (2 preceding siblings ...)
2026-07-21 23:43 ` [PATCH net-next v2 3/7] net: mana: free HWC comp_buf after destroying the EQ Long Li
@ 2026-07-21 23:43 ` Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering Long Li
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Long Li @ 2026-07-21 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 2239fdeda57c..515c45c4a890 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] 13+ messages in thread
* [PATCH net-next v2 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering
2026-07-21 23:43 [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth Long Li
` (3 preceding siblings ...)
2026-07-21 23:43 ` [PATCH net-next v2 4/7] net: mana: validate hardware-supplied values in the HWC RX path Long Li
@ 2026-07-21 23:43 ` Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 6/7] net: mana: support concurrent HWC requests with proper synchronization Long Li
2026-07-21 23:43 ` [PATCH net-next v2 7/7] net: mana: add dynamic HWC queue depth with reinit path Long Li
6 siblings, 1 reply; 13+ messages in thread
From: Long Li @ 2026-07-21 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
Two 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.
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 | 102 ++++++++++++++----
include/net/mana/gdma.h | 8 +-
include/net/mana/hw_channel.h | 8 ++
3 files changed, 96 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 515c45c4a890..5bc873d548d5 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,
@@ -902,11 +926,58 @@ 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 (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, issuing FLR\n",
+ td_err);
+
+ /* On systems without IOMMU, freeing DMA memory with
+ * active hardware MST mappings risks memory corruption.
+ * Issue FLR to force-reset the device and invalidate
+ * all hardware state including MST entries.
+ */
+ td_err = pcie_flr(to_pci_dev(gc->dev));
+ if (td_err) {
+ /* Device wedged: teardown and FLR
+ * both failed, so hardware may still
+ * DMA into our queue buffers. Leak
+ * all HWC DMA buffers instead of
+ * freeing memory the device can still
+ * reach; without an IOMMU the freed
+ * pages could be reused under live
+ * DMA.
+ */
+
+ dev_warn(gc->dev,
+ "Leaked HWC DMA buffers (CQ/RQ/TXQ) to prevent memory corruption. Device is no longer usable.\n");
+
+ /* Do not call destroy_cq/wq below;
+ * they would dma_free_coherent().
+ * Leave hwc/cq/rxq/txq allocated.
+ */
+ return;
+ }
+
+ dev_info(gc->dev, "FLR succeeded, hardware state cleared\n");
+ }
+
+ 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);
@@ -914,17 +985,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 da52701e7816..9ca7cf523366 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -428,7 +428,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..684dcec8e612 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -200,6 +200,14 @@ struct hw_channel_context {
u32 pf_dest_vrcq_id;
u32 hwc_timeout;
+ /* Set after mana_smc_setup_hwc() succeeds (hardware has active
+ * MST entries). On recoverable paths (establish_channel)
+ * cleared only after successful teardown so a retry remains
+ * possible. On the terminal destroy_channel path, cleared
+ * unconditionally since hwc is about to be freed.
+ */
+ bool setup_active;
+
struct hwc_caller_ctx *caller_ctx;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next v2 6/7] net: mana: support concurrent HWC requests with proper synchronization
2026-07-21 23:43 [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth Long Li
` (4 preceding siblings ...)
2026-07-21 23:43 ` [PATCH net-next v2 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering Long Li
@ 2026-07-21 23:43 ` Long Li
2026-07-21 23:43 ` [PATCH net-next v2 7/7] net: mana: add dynamic HWC queue depth with reinit path Long Li
6 siblings, 0 replies; 13+ messages in thread
From: Long Li @ 2026-07-21 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 serialized all management commands behind a depth-1 semaphore, a
bottleneck when many commands must be issued concurrently. Allowing
multiple in-flight requests exposes several shared-state races that this
patch addresses together.
- Replace the semaphore with a waitqueue + bitmap scheme so senders
acquire message slots in parallel and sleep only when all slots are
busy.
- A response is delivered by handle_resp() in CQ interrupt context
while the issuing sender may concurrently time out and return; both
touch the same caller_ctx slot (output_buf, output_buflen, error):
CPU A (sender timeout) CPU B (handle_resp, IRQ)
------------------------------ ------------------------------
read/clear output_buf write response into output_buf,
complete the slot
Add a per-slot spinlock to make these mutually exclusive, a per-slot
refcount so the last of {sender, handle_resp} releases the bitmap
slot (no double-release, no stuck slot), and an embedded completion
the sender waits on. The sender always NULLs output_buf after
waking, so a late handle_resp() skips the copy.
- Add a per-queue lock to hwc_wq; mana_gd_post_and_ring() is not safe
to call concurrently on the same queue.
- Add channel_up (cleared under the bitmap lock in destroy_channel) and
hwc_timed_out flags so new slot acquisitions are rejected during
teardown and after a timeout.
- destroy_channel() must not free the HWC while senders are still in
flight. Count active senders in a gc->hwc_lock-protected counter;
destroy_channel() force-completes the in-flight slots (-ENODEV), then
drains the counter to zero with wait_event_lock_irq() before freeing.
The counter changes only under hwc_lock and the last sender's
wake_up() runs under that lock, so evaluating the drain condition
under hwc_lock guarantees the waking sender has already dropped the
lock -- finished touching gc -- before the drain returns; it cannot
race the later free of gc. The waitqueue itself lives on
gdma_context, not hwc, so the wake never dereferences freed hwc.
- The sender looks up the channel via gc->hwc.driver_data, which
destroy_channel() clears and then frees. Without serialization the
lookup and the reference can straddle the free:
CPU A (mana_gd_send_request) CPU B (destroy_channel)
------------------------------ ------------------------------
hwc = gc->hwc.driver_data; // ok
driver_data = NULL;
wait active_senders == 0; // 0!
kfree(hwc);
hwc->active_senders++; // use-after-free
Guard driver_data with a new gc->hwc_lock spinlock, taken by the
readers (mana_gd_send_request, mana_need_log, mana_serv_reset) and by
the publish/clear, so "load the pointer + take a sender reference" is
atomic against the clear. After the clear a sender either already
holds a reference (and is waited for) or observes NULL and returns
-ENODEV. These are all control-plane paths (HWC commands sleep,
reset runs on a workqueue), so a plain spinlock -- not RCU -- is
sufficient.
This adds the concurrency infrastructure at the bootstrap queue depth of
1; the next patch raises it to the device-reported maximum.
Signed-off-by: Long Li <longli@microsoft.com>
---
.../net/ethernet/microsoft/mana/gdma_main.c | 55 ++-
.../net/ethernet/microsoft/mana/hw_channel.c | 352 +++++++++++++++---
include/net/mana/gdma.h | 15 +
include/net/mana/hw_channel.h | 33 +-
4 files changed, 408 insertions(+), 47 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 9edebf7c097c..17711ee37c8b 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -161,6 +161,8 @@ static int mana_gd_init_registers(struct pci_dev *pdev)
bool mana_need_log(struct gdma_context *gc, int err)
{
struct hw_channel_context *hwc;
+ bool need_log = true;
+ unsigned long flags;
if (err != -ETIMEDOUT)
return true;
@@ -168,11 +170,13 @@ bool mana_need_log(struct gdma_context *gc, int err)
if (!gc)
return true;
+ spin_lock_irqsave(&gc->hwc_lock, flags);
hwc = gc->hwc.driver_data;
if (hwc && hwc->hwc_timeout == 0)
- return false;
+ need_log = false;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
- return true;
+ return need_log;
}
static int mana_gd_query_max_resources(struct pci_dev *pdev)
@@ -367,9 +371,27 @@ static int mana_gd_detect_devices(struct pci_dev *pdev)
int mana_gd_send_request(struct gdma_context *gc, u32 req_len, const void *req,
u32 resp_len, void *resp)
{
- struct hw_channel_context *hwc = gc->hwc.driver_data;
+ struct hw_channel_context *hwc;
+ unsigned long flags;
+ int err;
+
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ hwc = gc->hwc.driver_data;
+ if (!hwc) {
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+ return -ENODEV;
+ }
+ hwc->active_senders++;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+ err = mana_hwc_send_request(hwc, req_len, req, resp_len, resp);
- return mana_hwc_send_request(hwc, req_len, req, resp_len, resp);
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ if (--hwc->active_senders == 0)
+ wake_up(&gc->hwc_drain_waitq);
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+ return err;
}
EXPORT_SYMBOL_NS(mana_gd_send_request, "NET_MANA");
@@ -622,6 +644,7 @@ static void mana_serv_reset(struct pci_dev *pdev)
{
struct gdma_context *gc = pci_get_drvdata(pdev);
struct hw_channel_context *hwc;
+ unsigned long flags;
int ret;
if (!gc) {
@@ -631,14 +654,17 @@ static void mana_serv_reset(struct pci_dev *pdev)
return;
}
+ spin_lock_irqsave(&gc->hwc_lock, flags);
hwc = gc->hwc.driver_data;
if (!hwc) {
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
dev_err(&pdev->dev, "MANA service: no HWC\n");
goto out;
}
/* HWC is not responding in this case, so don't wait */
hwc->hwc_timeout = 0;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
dev_info(&pdev->dev, "MANA reset cycle start\n");
@@ -1200,6 +1226,16 @@ static int mana_gd_create_dma_region(struct gdma_dev *gd,
if (!MANA_PAGE_ALIGNED(gmi->virt_addr))
return -EINVAL;
+ /* No RCU needed: this runs only on the data-path queue-creation
+ * path (mana_gd_create_mana_eq/mana_gd_create_mana_wq_cq, called
+ * by mana_en under RTNL and by mana_ib RDMA verbs, or during
+ * init). Every teardown path — mana_gd_remove, mana_gd_suspend,
+ * and the HWC reset/service path (which goes through
+ * mana_gd_suspend) — drains those consumers via mana_rdma_remove()
+ * + mana_remove() before mana_hwc_destroy_channel() clears
+ * gc->hwc.driver_data, so no concurrent destroy can race with
+ * this dereference.
+ */
hwc = gc->hwc.driver_data;
req_msg_size = struct_size(req, page_addr_list, num_page);
if (req_msg_size > hwc->max_req_msg_size)
@@ -1389,7 +1425,17 @@ int mana_gd_verify_vf_version(struct pci_dev *pdev)
struct hw_channel_context *hwc;
int err;
+ /* No RCU needed: this runs only inside mana_gd_setup, on the
+ * probe and resume paths. The PCI/PM core holds device_lock
+ * across .probe/.resume and .remove/.suspend, so setup cannot
+ * overlap teardown of the same device. The HWC reset/service
+ * path is additionally serialized by GC_IN_SERVICE and runs
+ * suspend (destroy) then resume (this) sequentially in one work
+ * item. driver_data was just set by mana_hwc_create_channel
+ * earlier in this same setup call, so it is live here.
+ */
hwc = gc->hwc.driver_data;
+
mana_gd_init_req_hdr(&req.hdr, GDMA_VERIFY_VF_DRIVER_VERSION,
sizeof(req), sizeof(resp));
@@ -2379,6 +2425,7 @@ static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
mutex_init(&gc->eq_test_event_mutex);
mutex_init(&gc->gic_mutex);
+ spin_lock_init(&gc->hwc_lock);
pci_set_drvdata(pdev, gc);
gc->bar0_pa = pci_resource_start(pdev, 0);
gc->bar0_size = pci_resource_len(pdev, 0);
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 5bc873d548d5..616fb088c559 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -7,25 +7,52 @@
#include <linux/pci.h>
#include <linux/vmalloc.h>
+/* Acquire a free message slot from the inflight bitmap. Returns
+ * -ENODEV if the channel is torn down, or -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->channel_up || hwc->hwc_timed_out) {
+ spin_unlock_irqrestore(&r->lock, flags);
+ return hwc->channel_up ? -ETIMEDOUT : -ENODEV;
+ }
- 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->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->channel_up ||
+ hwc->hwc_timed_out ||
+ !bitmap_full(r->map, r->size));
- spin_unlock_irqrestore(&r->lock, flags);
+ if (!hwc->channel_up)
+ return -ENODEV;
- *msg_id = index;
+ if (hwc->hwc_timed_out)
+ return -ETIMEDOUT;
+ }
+ *msg_id = index;
return 0;
}
@@ -35,10 +62,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 +148,32 @@ 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;
+ err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
- memcpy(ctx->output_buf, resp_msg, resp_len);
-out:
- ctx->error = err;
+ 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;
+ }
- /* 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,
@@ -641,6 +685,7 @@ static int mana_hwc_create_wq(struct hw_channel_context *hwc,
hwc_wq->gdma_wq = queue;
hwc_wq->queue_depth = q_depth;
hwc_wq->hwc_cq = hwc_cq;
+ spin_lock_init(&hwc_wq->lock);
err = mana_hwc_alloc_dma_buf(hwc, q_depth, max_msg_size,
&hwc_wq->msg_buf);
@@ -658,7 +703,7 @@ static int mana_hwc_create_wq(struct hw_channel_context *hwc,
return err;
}
-static int mana_hwc_post_tx_wqe(const struct hwc_wq *hwc_txq,
+static int mana_hwc_post_tx_wqe(struct hwc_wq *hwc_txq,
struct hwc_work_request *req,
u32 dest_virt_rq_id, u32 dest_virt_rcq_id,
bool dest_pf)
@@ -697,7 +742,11 @@ static int mana_hwc_post_tx_wqe(const struct hwc_wq *hwc_txq,
req->wqe_req.inline_oob_data = tx_oob;
req->wqe_req.client_data_unit = 0;
+ /* Serialize WQE posting — multiple senders may call concurrently. */
+ spin_lock(&hwc_txq->lock);
err = mana_gd_post_and_ring(hwc_txq->gdma_wq, &req->wqe_req, NULL);
+ spin_unlock(&hwc_txq->lock);
+
if (err)
dev_err(dev, "Failed to post WQE on HWC SQ: %d\n", err);
return err;
@@ -708,7 +757,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,18 +787,34 @@ 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;
- return mana_gd_test_eq(gc, hwc->cq->gdma_eq);
+ /* channel_up must be set before the test EQ request, because
+ * the request goes through mana_hwc_get_msg_index() which
+ * checks channel_up. caller_ctx is allocated above, so
+ * concurrent access to a NULL caller_ctx is not possible.
+ */
+ hwc->channel_up = true;
+
+ err = mana_gd_test_eq(gc, hwc->cq->gdma_eq);
+ if (err)
+ hwc->channel_up = false;
+
+ return err;
}
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;
@@ -866,6 +931,7 @@ int mana_hwc_create_channel(struct gdma_context *gc)
u32 max_req_msg_size, max_resp_msg_size;
struct gdma_dev *gd = &gc->hwc;
struct hw_channel_context *hwc;
+ unsigned long flags;
u16 q_depth_max;
int err;
@@ -874,10 +940,11 @@ int mana_hwc_create_channel(struct gdma_context *gc)
return -ENOMEM;
gd->gdma_context = gc;
- gd->driver_data = hwc;
hwc->gdma_dev = gd;
hwc->dev = gc->dev;
hwc->hwc_timeout = HW_CHANNEL_WAIT_RESOURCE_TIMEOUT_MS;
+ hwc->active_senders = 0;
+ init_waitqueue_head(&gc->hwc_drain_waitq);
/* HWC's instance number is always 0. */
gd->dev_id.as_uint32 = 0;
@@ -886,6 +953,15 @@ int mana_hwc_create_channel(struct gdma_context *gc)
gd->pdid = INVALID_PDID;
gd->doorbell = INVALID_DOORBELL;
+ /* Publish driver_data last, under hwc_lock: the lock orders the hwc
+ * initialisation above before the pointer becomes visible and
+ * serialises the publish against the control-plane readers in
+ * mana_gd_send_request(), mana_need_log() and mana_serv_reset().
+ */
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ gc->hwc.driver_data = hwc;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
/* mana_hwc_init_queues() only creates the required data structures,
* and doesn't touch the HWC device.
*/
@@ -920,15 +996,118 @@ int mana_hwc_create_channel(struct gdma_context *gc)
void mana_hwc_destroy_channel(struct gdma_context *gc)
{
+ /* This is the only destroy entry point. driver_data is read
+ * plainly here (teardown is serialised against other teardown);
+ * it is cleared under hwc_lock below before hwc is freed.
+ */
struct hw_channel_context *hwc = gc->hwc.driver_data;
struct gdma_queue __rcu **old_cq_table;
+ unsigned long flags;
if (!hwc)
return;
+ /* Prevent new requests from starting and wake any
+ * threads waiting for a free msg slot. Set channel_up under
+ * the bitmap lock so get_msg_index() cannot acquire a slot
+ * and increment active_senders after this point.
+ *
+ * If channel_up is already false (e.g. init failed before
+ * the channel was established), skip the lock — it may not
+ * have been initialized yet, and no senders can be active.
+ */
+ if (hwc->channel_up) {
+ spin_lock_irqsave(&hwc->inflight_msg_res.lock, flags);
+ hwc->channel_up = false;
+ spin_unlock_irqrestore(&hwc->inflight_msg_res.lock, flags);
+ wake_up_all(&hwc->msg_waitq);
+ }
+
+ /* Clear the pointer under hwc_lock so new callers in
+ * mana_gd_send_request() see NULL and return -ENODEV. The lock
+ * makes the readers' "load driver_data + active_senders++"
+ * atomic against this store, so once it returns no new sender can
+ * take a reference; the active_senders drain below waits out those
+ * that already did, before their hwc is freed.
+ */
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ gc->hwc.driver_data = NULL;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+ /* Force-complete any in-flight senders so they observe -ENODEV,
+ * return, and drop their references. This runs before the HWC
+ * hardware teardown below, so a live interrupt may still deliver
+ * a real response via handle_resp() concurrently — that is safe
+ * because the per-slot refcount model tolerates a concurrent
+ * complete() and both paths (handle_resp and this loop) drop
+ * their refs without double-releasing the slot.
+ */
+ if (hwc->caller_ctx) {
+ struct hwc_caller_ctx *ctx;
+ int i;
+
+ for (i = 0; i < hwc->num_inflight_msg; i++) {
+ if (!test_bit(i, hwc->inflight_msg_res.map))
+ continue;
+
+ ctx = &hwc->caller_ctx[i];
+
+ /* Wake senders blocked on wait_for_completion.
+ * Set error under lock to avoid racing with
+ * handle_resp() which writes error under the
+ * same lock. The sender NULLs output_buf
+ * after waking — doing it here would race
+ * with a sender that hasn't set output_buf yet.
+ */
+ spin_lock_irqsave(&ctx->lock, flags);
+ ctx->error = -ENODEV;
+ complete(&ctx->comp_event);
+ spin_unlock_irqrestore(&ctx->lock, flags);
+ }
+ }
+
+ /* Wait for all sender threads to finish and drop their refs
+ * before touching the hardware or freeing anything. Draining
+ * here -- ahead of the teardown/FLR block below -- guarantees no
+ * in-flight sender is still running when this function returns on
+ * any path, including the FLR-failure early return; otherwise a
+ * stranded sender would dereference gc->hwc_lock /
+ * gc->hwc_drain_waitq after the caller frees gc.
+ * After this, only slots held by timed-out senders whose
+ * handle_resp() never ran remain in the bitmap.
+ *
+ * active_senders is only ever modified under hwc_lock, and the
+ * last sender's wake_up() runs under that lock before it is
+ * released. Evaluating the condition under hwc_lock therefore
+ * guarantees that once we observe 0 the waking sender has
+ * already dropped the lock -- i.e. finished touching gc -- so it
+ * cannot race the caller freeing gc after this returns.
+ */
+ spin_lock_irq(&gc->hwc_lock);
+ wait_event_lock_irq(gc->hwc_drain_waitq,
+ hwc->active_senders == 0, gc->hwc_lock);
+ spin_unlock_irq(&gc->hwc_lock);
+
/* 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.
+ *
+ * The return value is intentionally not checked. This is the
+ * terminal cleanup path (device removal, suspend, or init
+ * failure) — resources must be freed regardless. If teardown
+ * fails, hardware may still have active MST entries, but:
+ *
+ * - Interrupts: mana_hwc_destroy_cq() below calls
+ * mana_gd_deregister_irq() which removes the HWC EQ from
+ * the interrupt dispatch list via list_del_rcu() +
+ * synchronize_rcu(). After that, no interrupt handler can
+ * invoke handle_resp() or access CQ/RQ buffers — even if
+ * the IRQ is shared with data path queues.
+ *
+ * - DMA: mana_hwc_destroy_wq() frees DMA buffers via
+ * dma_free_coherent() which unmaps the IOVA from the
+ * IOMMU. Any stale hardware DMA to the old address
+ * faults at the IOMMU, not in kernel memory.
*/
if (hwc->setup_active) {
int td_err = mana_smc_teardown_hwc(&gc->shm_channel, false);
@@ -947,7 +1126,9 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
/* Device wedged: teardown and FLR
* both failed, so hardware may still
* DMA into our queue buffers. Leak
- * all HWC DMA buffers instead of
+ * all HWC DMA buffers -- and the EQ
+ * IRQ registration, whose safe removal
+ * needs those queues -- instead of
* freeing memory the device can still
* reach; without an IOMMU the freed
* pages could be reused under live
@@ -972,9 +1153,12 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
/* 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.
+ * deregisters the HWC EQ from the interrupt handler's RCU list
+ * (via mana_gd_deregister_irq + synchronize_rcu), guaranteeing
+ * no interrupt handler can access RQ/TXQ buffers after this
+ * point. The active_senders drain above ensures no sender is
+ * accessing the CQ via txq->hwc_cq when it is destroyed. Then
+ * destroy TXQ and RQ safely.
*/
if (hwc->cq)
mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
@@ -985,6 +1169,23 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
if (hwc->rxq)
mana_hwc_destroy_wq(hwc, hwc->rxq);
+ /* Release any slots still held — these belong to timed-out
+ * senders where handle_resp() never ran (refcount = 1 with
+ * handle_resp's ref still outstanding).
+ */
+ if (hwc->caller_ctx) {
+ struct hwc_caller_ctx *ctx;
+ int i;
+
+ for (i = 0; i < hwc->num_inflight_msg; i++) {
+ if (!test_bit(i, hwc->inflight_msg_res.map))
+ continue;
+
+ ctx = &hwc->caller_ctx[i];
+ hwc_ctx_put(hwc, ctx);
+ }
+ }
+
kfree(hwc->caller_ctx);
hwc->caller_ctx = NULL;
@@ -998,7 +1199,6 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
hwc->hwc_timeout = 0;
kfree(hwc);
- gc->hwc.driver_data = NULL;
gc->hwc.gdma_context = NULL;
old_cq_table = rcu_replace_pointer(gc->cq_table, NULL, true);
@@ -1014,13 +1214,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];
@@ -1032,8 +1236,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)
@@ -1049,8 +1256,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;
}
@@ -1061,31 +1274,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/gdma.h b/include/net/mana/gdma.h
index 9ca7cf523366..01e845237b6a 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -481,6 +481,21 @@ struct gdma_context {
/* Hardware communication channel (HWC) */
struct gdma_dev hwc;
+ /* destroy_channel() waits here for all HWC senders to exit.
+ * Lives on gc (not hwc) so wake_up() after the last sender's
+ * atomic_dec doesn't dereference freed hwc memory.
+ */
+ wait_queue_head_t hwc_drain_waitq;
+
+ /* Serializes hwc.driver_data (the hw_channel_context pointer)
+ * between the control-plane readers in mana_gd_send_request(),
+ * mana_need_log() and mana_serv_reset() and the publish/clear in
+ * mana_hwc_create_channel()/mana_hwc_destroy_channel(). All users
+ * are control-plane (HWC commands sleep; reset runs on a workqueue),
+ * so a plain spinlock -- not RCU -- is sufficient.
+ */
+ spinlock_t hwc_lock;
+
/* Azure network adapter */
struct gdma_dev mana;
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 684dcec8e612..30bc236286dc 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -164,6 +164,9 @@ struct hwc_wq {
u16 queue_depth;
struct hwc_cq *hwc_cq;
+
+ /* Serializes concurrent mana_gd_post_and_ring() calls. */
+ spinlock_t lock;
};
struct hwc_caller_ctx {
@@ -173,6 +176,17 @@ 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;
};
struct hw_channel_context {
@@ -193,13 +207,24 @@ 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 after channel is fully established; cleared on teardown to
+ * abort waiters in mana_hwc_get_msg_index() and reject new sends.
+ */
+ bool channel_up;
+
+ /* 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). On recoverable paths (establish_channel)
* cleared only after successful teardown so a retry remains
@@ -208,6 +233,12 @@ struct hw_channel_context {
*/
bool setup_active;
+ /* Count of in-flight mana_gd_send_request() callers. Protected
+ * by gc->hwc_lock; the last sender to drop it to zero wakes
+ * gc->hwc_drain_waitq for the mana_hwc_destroy_channel() drain.
+ */
+ unsigned int active_senders;
+
struct hwc_caller_ctx *caller_ctx;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next v2 7/7] net: mana: add dynamic HWC queue depth with reinit path
2026-07-21 23:43 [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth Long Li
` (5 preceding siblings ...)
2026-07-21 23:43 ` [PATCH net-next v2 6/7] net: mana: support concurrent HWC requests with proper synchronization Long Li
@ 2026-07-21 23:43 ` Long Li
6 siblings, 0 replies; 13+ messages in thread
From: Long Li @ 2026-07-21 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 is first established at a bootstrap queue depth of 1. Query the
device's maximum supported depth and, if larger, tear down and rebuild
the HWC queues at that depth before re-establishing the channel, so more
management commands can be in flight. Advertise
GDMA_DRV_CAP_FLAG_1_DYN_HWC_QUEUE_DEPTH so the firmware enables this only
when the driver supports it.
mana_hwc_destroy_queues() tears down the CQ first, which deregisters the
EQ IRQ (mana_gd_deregister_irq() + synchronize_rcu()) so no interrupt
handler can touch the queues, then the TXQ, RXQ and inflight resources.
Validate the device-reported dimensions before they size DMA
allocations: enforce the request/response header minimums, ensure
q_depth * max_msg_size plus alignment fits in u32, and cap CQ depth to
U16_MAX/2. Carry the depth as u32 -- the device field is 24-bit, so
truncating to u16 on receipt could wrap a large value to a small depth
and silently pass these checks.
If reinit fails, fall back to the bootstrap-depth channel when its
teardown succeeded; if teardown also failed, hardware mappings may still
be active, so abort channel creation instead.
Signed-off-by: Long Li <longli@microsoft.com>
---
.../net/ethernet/microsoft/mana/hw_channel.c | 190 +++++++++++++++++-
include/net/mana/gdma.h | 4 +
include/net/mana/hw_channel.h | 2 +-
3 files changed, 191 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 616fb088c559..0481d4044342 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -214,7 +214,12 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
break;
case HWC_INIT_DATA_QUEUE_DEPTH:
- hwc->hwc_init_q_depth_max = (u16)val;
+ /* HWC_INIT_DATA_QUEUE_DEPTH is a 24-bit field. Keep
+ * the full device-reported value here; it is clamped
+ * and validated in mana_hwc_create_channel() rather
+ * than silently truncated to u16.
+ */
+ hwc->hwc_init_q_depth_max = val;
break;
case HWC_INIT_DATA_MAX_REQUEST:
@@ -808,7 +813,7 @@ static int mana_hwc_test_channel(struct hw_channel_context *hwc, u16 q_depth,
return err;
}
-static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
+static int mana_hwc_establish_channel(struct gdma_context *gc, u32 *q_depth,
u32 *max_req_msg_size,
u32 *max_resp_msg_size)
{
@@ -886,6 +891,12 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
{
int err;
+ /* CQ depth is q_depth * 2 (SQ + RQ) passed as u16 to create_cq.
+ * Cap to prevent u16 truncation.
+ */
+ if (q_depth > U16_MAX / 2)
+ q_depth = U16_MAX / 2;
+
err = mana_hwc_init_inflight_msg(hwc, q_depth);
if (err)
return err;
@@ -926,13 +937,62 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
return err;
}
+/* Tear down all HWC queues and free associated resources. Used on
+ * the reinit-with-higher-queue-depth path and reinit fallback.
+ *
+ * PRECONDITION: must be called only during channel bring-up in
+ * mana_hwc_create_channel(), before the channel is published to
+ * senders. There the setup thread is effectively single-threaded
+ * (serialized against teardown by the PCI/PM device_lock or, on the
+ * service path, GC_IN_SERVICE, with the data path not yet probed),
+ * channel_up is still false, caller_ctx is not yet allocated, and
+ * active_senders is 0 — so no concurrent request/response user can
+ * touch these queues. That is why this skips the hwc_lock-protected
+ * driver_data clear + active_senders drain that
+ * mana_hwc_destroy_channel() needs for the runtime teardown race;
+ * only the CQ-first ordering below (to fence off a pending interrupt)
+ * is required. Calling this on a live, published channel would be a
+ * use-after-free.
+ */
+static void mana_hwc_destroy_queues(struct hw_channel_context *hwc)
+{
+ struct gdma_context *gc = hwc->gdma_dev->gdma_context;
+
+ /* Destroy CQ first to deregister the EQ from the interrupt
+ * handler list before freeing caller_ctx, TXQ, or RXQ memory.
+ * A pending interrupt handler could still reach handle_resp()
+ * which dereferences caller_ctx.
+ */
+ if (hwc->cq) {
+ mana_hwc_destroy_cq(gc, hwc->cq);
+ hwc->cq = NULL;
+ }
+
+ kfree(hwc->caller_ctx);
+ hwc->caller_ctx = NULL;
+
+ if (hwc->txq) {
+ mana_hwc_destroy_wq(hwc, hwc->txq);
+ hwc->txq = NULL;
+ }
+
+ if (hwc->rxq) {
+ mana_hwc_destroy_wq(hwc, hwc->rxq);
+ hwc->rxq = NULL;
+ }
+
+ mana_gd_free_res_map(&hwc->inflight_msg_res);
+ hwc->num_inflight_msg = 0;
+}
+
int mana_hwc_create_channel(struct gdma_context *gc)
{
u32 max_req_msg_size, max_resp_msg_size;
struct gdma_dev *gd = &gc->hwc;
struct hw_channel_context *hwc;
+ struct gdma_queue __rcu **old_cq_table;
unsigned long flags;
- u16 q_depth_max;
+ u32 q_depth_max;
int err;
hwc = kzalloc_obj(*hwc);
@@ -980,8 +1040,130 @@ int mana_hwc_create_channel(struct gdma_context *gc)
goto out;
}
+ /* The channel was bootstrapped at a minimal queue depth. If the
+ * device reports a higher maximum, tear down and rebuild with
+ * the larger depth so more HWC commands can be in flight.
+ */
+ if (q_depth_max > HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH) {
+ /* q_depth_max now carries the full device-reported value
+ * (HWC_INIT_DATA_QUEUE_DEPTH is 24-bit). Clamp it to the
+ * depth the rest of the driver supports — create_cq() takes
+ * q_depth * 2 as a u16 — before the overflow check below, so
+ * an over-large but otherwise-valid depth is reduced to the
+ * maximum instead of wrapping or being rejected.
+ */
+ if (q_depth_max > U16_MAX / 2)
+ q_depth_max = U16_MAX / 2;
+
+ /* Sanity-check device-reported values before using them
+ * to size DMA allocations. Enforce protocol minimums
+ * for message sizes and check that q_depth * max_msg_size
+ * plus alignment headroom fits in u32 (for
+ * mana_hwc_alloc_dma_buf's MANA_PAGE_ALIGN).
+ */
+ if (!max_req_msg_size || !max_resp_msg_size ||
+ max_req_msg_size < sizeof(struct gdma_req_hdr) ||
+ max_resp_msg_size < sizeof(struct gdma_resp_hdr) ||
+ (u64)q_depth_max * max_req_msg_size >
+ U32_MAX - MANA_PAGE_SIZE ||
+ (u64)q_depth_max * max_resp_msg_size >
+ U32_MAX - MANA_PAGE_SIZE) {
+ dev_err(hwc->dev,
+ "HWC: invalid dims q=%u req=%u resp=%u\n",
+ q_depth_max, max_req_msg_size,
+ max_resp_msg_size);
+ q_depth_max = HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH;
+ goto skip_reinit;
+ }
+
+ err = mana_smc_teardown_hwc(&gc->shm_channel, false);
+ if (err) {
+ /* Keep using the bootstrap-depth channel. */
+ dev_err(hwc->dev,
+ "Failed to teardown HWC for reinit: %d\n",
+ err);
+ q_depth_max = HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH;
+ goto skip_reinit;
+ }
+
+ hwc->setup_active = false;
+
+ /* Destroy queues first — mana_gd_destroy_cq inside
+ * unpublishes the CQ from cq_table via
+ * rcu_assign_pointer(NULL) + synchronize_rcu.
+ * Must happen while cq_table is still valid.
+ */
+ mana_hwc_destroy_queues(hwc);
+
+ old_cq_table = rcu_replace_pointer(gc->cq_table, NULL, true);
+ synchronize_rcu();
+ vfree(old_cq_table);
+
+ err = mana_hwc_init_queues(hwc, q_depth_max,
+ max_req_msg_size,
+ max_resp_msg_size);
+ if (err) {
+ dev_err(hwc->dev, "Failed to reinit HWC: %d\n", err);
+ goto reinit_fallback;
+ }
+
+ err = mana_hwc_establish_channel(gc, &q_depth_max,
+ &max_req_msg_size,
+ &max_resp_msg_size);
+ if (err) {
+ dev_err(hwc->dev, "Failed to re-establish HWC: %d\n",
+ err);
+ /* establish_channel does internal teardown on
+ * failure. If teardown succeeded (setup_active
+ * cleared), MST entries are invalidated and we
+ * can try the bootstrap fallback. If teardown
+ * also failed (setup_active still set), hardware
+ * mappings may still be active — skip fallback.
+ */
+ if (hwc->setup_active)
+ goto out;
+ goto reinit_fallback;
+ }
+ }
+
+ goto skip_reinit;
+
+reinit_fallback:
+ /* Restore bootstrap-depth channel so the device remains functional.
+ * Free cq_table if it was allocated by a partially successful
+ * establish attempt.
+ */
+ dev_warn(hwc->dev, "HWC reinit failed, falling back to bootstrap depth\n");
+
+ mana_hwc_destroy_queues(hwc);
+
+ old_cq_table = rcu_replace_pointer(gc->cq_table, NULL, true);
+ synchronize_rcu();
+ vfree(old_cq_table);
+
+ err = mana_hwc_init_queues(hwc, HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
+ HW_CHANNEL_MAX_REQUEST_SIZE,
+ HW_CHANNEL_MAX_RESPONSE_SIZE);
+ if (err) {
+ dev_err(hwc->dev, "Failed to restore bootstrap HWC: %d\n", err);
+ goto out;
+ }
+
+ err = mana_hwc_establish_channel(gc, &q_depth_max, &max_req_msg_size,
+ &max_resp_msg_size);
+ if (err) {
+ dev_err(hwc->dev, "Failed to re-establish bootstrap HWC: %d\n",
+ err);
+ goto out;
+ }
+
+skip_reinit:
+
+ /* No RCU needed: still in mana_hwc_create_channel, the
+ * pointer has not been published to concurrent senders yet.
+ */
err = mana_hwc_test_channel(gc->hwc.driver_data,
- HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
+ hwc->num_inflight_msg,
max_req_msg_size, max_resp_msg_size);
if (err) {
dev_err(hwc->dev, "Failed to test HWC: %d\n", err);
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 01e845237b6a..a4eac6f7c366 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -708,6 +708,9 @@ enum {
/* Driver supports dynamic interrupt moderation - DIM */
#define GDMA_DRV_CAP_FLAG_1_DYN_INTERRUPT_MODERATION BIT(28)
+/* Driver supports dynamic queue depth for HWC */
+#define GDMA_DRV_CAP_FLAG_1_DYN_HWC_QUEUE_DEPTH BIT(29)
+
#define GDMA_DRV_CAP_FLAGS1 \
(GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
@@ -723,6 +726,7 @@ enum {
GDMA_DRV_CAP_FLAG_1_PROBE_RECOVERY | \
GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY | \
GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECOVERY | \
+ GDMA_DRV_CAP_FLAG_1_DYN_HWC_QUEUE_DEPTH | \
GDMA_DRV_CAP_FLAG_1_EQ_MSI_UNSHARE_MULTI_VPORT | \
GDMA_DRV_CAP_FLAG_1_DYN_INTERRUPT_MODERATION)
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 30bc236286dc..eec5d3aa23e1 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -197,7 +197,7 @@ struct hw_channel_context {
u32 max_req_msg_size;
u32 max_resp_msg_size;
- u16 hwc_init_q_depth_max;
+ u32 hwc_init_q_depth_max;
u32 hwc_init_max_req_msg_size;
u32 hwc_init_max_resp_msg_size;
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 3/7] net: mana: free HWC comp_buf after destroying the EQ
2026-07-21 23:43 ` [PATCH net-next v2 3/7] net: mana: free HWC comp_buf after destroying the EQ Long Li
@ 2026-07-22 23:46 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 23:46 UTC (permalink / raw)
To: Long Li; +Cc: linux-hyperv
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free of WQs during HWC channel destruction because WQs are destroyed before the EQ and CQ.
- [High] Immediate reuse of `msg_id` after a timeout leads to cross-request stack corruption.
- [High] Pending `mana_serv_work` items on the global workqueue are not canceled during device removal, causing a Use-After-Free.
- [High] Missing bounds check on untrusted `inline_oob_size_div4` allows out-of-bounds memory read.
- [High] Missing length check on `tx_oob_data_size` allows spoofed completions via stale DMA buffer data.
- [Medium] Destroying the CQ before the EQ triggers a `WARN_ON_ONCE(!cq)` splat if an EQ interrupt fires during the race window.
--
--- Patch [3]: [PATCH net-next v2 3/7] net: mana: free HWC comp_buf after destroying the EQ ---
commit 6326f43efebe30510f1377a79fcf8a92f5abb2b7
Author: Long Li <longli@microsoft.com>
net: mana: free HWC comp_buf after destroying the EQ
This commit changes mana_hwc_destroy_cq() to free hwc_cq->comp_buf after
destroying the CQ and EQ. This prevents a use-after-free where a late EQ
interrupt handler could touch comp_buf while the EQ was still registered.
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 3f011ebbe7b3e..2239fdeda57cb 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);
[Severity: Medium]
This isn't a bug introduced by this patch, but destroying the CQ before the
EQ appears to leave a race window for EQ interrupts. If a hardware interrupt
fires for the EQ, could mana_gd_process_eqe() read a completion event, look
up the CQ ID in the table, find NULL, and trigger the WARN_ON_ONCE(!cq) splat?
>
> + /* comp_buf is reached only by mana_hwc_comp_event(), which the
[ ... ]
> if (hwc_cq->gdma_eq)
> mana_gd_destroy_queue(gc, hwc_cq->gdma_eq);
>
> + kfree(hwc_cq->comp_buf);
> kfree(hwc_cq);
> }
I also noticed some pre-existing issues in related code:
> drivers/net/ethernet/microsoft/mana/hw_channel.c:mana_hwc_destroy_channel()
>
> if (hwc->txq)
> mana_hwc_destroy_wq(hwc, hwc->txq);
>
> if (hwc->rxq)
> mana_hwc_destroy_wq(hwc, hwc->rxq);
>
> if (hwc->cq)
> mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
[Severity: High]
This is a pre-existing issue, but WQs are destroyed and freed here before
the CQ/EQ. Since the EQ and CQ are not yet destroyed and their interrupts
are still active, could an in-flight EQ interrupt process a late completion,
call mana_hwc_rx_event_handler(), and dereference hwc->rxq which points to
freed memory?
> drivers/net/ethernet/microsoft/mana/hw_channel.c:mana_hwc_send_request()
>
> if (!wait_for_completion_timeout(&ctx->comp_event,
> (msecs_to_jiffies(hwc->hwc_timeout)))) {
[ ... ]
> err = -ETIMEDOUT;
> goto out;
> }
> out:
> mana_hwc_put_msg_index(hwc, msg_id);
[Severity: High]
This is a pre-existing issue, but does the timeout path prematurely release
the msg_id without cancelling the request in hardware? If a second thread
allocates the same msg_id and the hardware sends a late response for the
first request, could mana_hwc_handle_resp() copy the response data into
the second thread's stack buffer?
> drivers/net/ethernet/microsoft/mana/hw_channel.c:mana_hwc_rx_event_handler()
>
> sge = (struct gdma_sge *)(wqe + 8 + dma_oob->inline_oob_size_div4 * 4);
>
> /* 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_resp_msg_size;
[Severity: High]
This is a pre-existing issue, but is there a missing bounds check on the
untrusted inline_oob_size_div4 before it is used to calculate the sge pointer?
If a malicious hypervisor supplies an arbitrarily large value, could this
lead to an out-of-bounds memory read when sge->address is dereferenced?
> drivers/net/ethernet/microsoft/mana/hw_channel.c:mana_hwc_rx_event_handler()
>
> 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.
> */
> msg_id = READ_ONCE(resp->response.hwc_msg_id);
> if (msg_id >= hwc->num_inflight_msg) {
[ ... ]
> mana_hwc_handle_resp(hwc, rx_oob->tx_oob_data_size, rx_req, msg_id);
[Severity: High]
This is a pre-existing issue, but does this code read msg_id from the DMA
buffer before verifying that the response size (rx_oob->tx_oob_data_size)
is large enough to contain it? If the hardware sends a small completion,
could the buffer retain stale data containing an old msg_id, causing
mana_hwc_handle_resp() to route the response to an active, unrelated request?
> drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_schedule_serv_work()
>
> INIT_WORK(&mns_wk->serv_work, mana_serv_func);
> schedule_work(&mns_wk->serv_work);
[Severity: High]
This is a pre-existing issue, but the work item is queued on the global
workqueue here. During device unbind, mana_gd_remove() frees the gc
structure via vfree(gc) but does not track or cancel pending work items.
When the delayed work item executes, could mana_serv_func() retrieve the
stale gc pointer via pci_get_drvdata(pdev) and dereference it?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721234339.1476932-1-longli@microsoft.com?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy
2026-07-21 23:43 ` [PATCH net-next v2 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy Long Li
@ 2026-07-22 23:46 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 23:46 UTC (permalink / raw)
To: Long Li; +Cc: linux-hyperv
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Concurrent QP creations sharing the same CQ cause a data race on `send_cq->queue.id`, leaking CQ table entries and triggering a double free of `gdma_cq` in the error path.
- [High] The EQ event handler `mana_gd_process_eqe()` triggers a false positive `WARN_ON_ONCE(!cq)` when encountering stale EQEs for a recently destroyed CQ during normal teardown.
- [High] `mana_gd_destroy_queue()` frees EQ queues immediately after `list_del_rcu()` without waiting for an RCU grace period, causing a use-after-free if an interrupt handler is concurrently traversing the EQ list.
--
--- Patch [1]: [PATCH net-next v2 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy ---
commit 4df31c26faa213971de40d92e7d0a48ab9e5d300
Author: Long Li <longli@microsoft.com>
net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy
This commit fixes a use-after-free race between the EQ interrupt handler and
CQ teardown by placing the cq_table under RCU protection. It annotates the
table, updates readers to use rcu_dereference, and ensures removers clear
the slot and call synchronize_rcu() before freeing the CQ.
> diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
> --- a/drivers/infiniband/hw/mana/cq.c
> +++ b/drivers/infiniband/hw/mana/cq.c
[ ... ]
> @@ -149,23 +157,49 @@ int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
> void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
> {
[ ... ]
> + /* 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);
> }
[Severity: Critical]
This is a pre-existing issue, but does this actually prevent double frees
during concurrent QP creation failures?
If multiple concurrent ib_create_qp() calls share the same send_cq, the IB
core takes a read lock. In mana_ib_create_qp_raw(), the shared
send_cq->queue.id is locklessly overwritten:
drivers/infiniband/hw/mana/qp.c:mana_ib_create_qp_raw() {
...
qp->raw_sq.id = wq_spec.queue_index;
send_cq->queue.id = cq_spec.queue_index;
...
}
If both threads fail after this point (e.g., due to an invalid user pointer),
they both jump to the error path and call mana_ib_remove_cq_cb().
Since the removers are not serialized in this error path, couldn't they both
read the same gdma_cq pointer, both clear it, wait for synchronize_rcu(),
and then both call kfree(gdma_cq)?
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
[ ... ]
> @@ -772,10 +773,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;
[Severity: High]
This isn't a bug introduced by this patch, but could this WARN_ON_ONCE
trigger as a false positive during normal teardown?
During interface teardown, mana_deinit_cq() clears cq_table[id] and waits
for an RCU grace period, but the associated EQ remains active until
mana_destroy_eq() is called later.
If the EQ already contains a completion event for the destroyed CQ, the
interrupt handler will process the stale EQE, read a NULL pointer for cq,
and hit this warning.
Since panic_on_warn is common in cloud environments, could this crash the
system during a standard interface teardown?
[ ... ]
> @@ -1082,15 +1084,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)
> {
[ ... ]
> + /* 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();
> }
[Severity: High]
This is a pre-existing issue, but does mana_gd_destroy_queue() need a similar
synchronize_rcu() to prevent a use-after-free?
While mana_gd_intr() traverses the EQ list under RCU:
drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_intr() {
rcu_read_lock();
list_for_each_entry_rcu(eq, &gic->eq_list, entry)
mana_gd_process_eqe(eq);
rcu_read_unlock();
}
The teardown path removes the queue via list_del_rcu() but then immediately
frees the memory without waiting for a grace period:
drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_destroy_queue() {
...
mana_gd_destroy_dma_region(gc, gmi->dma_region_handle);
mana_gd_free_memory(gmi);
kfree(queue);
}
Could a concurrent reader in mana_gd_intr() dereference the freed EQ if
the memory is released before the RCU grace period ends?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721234339.1476932-1-longli@microsoft.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering
2026-07-21 23:43 ` [PATCH net-next v2 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering Long Li
@ 2026-07-22 23:46 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 23:46 UTC (permalink / raw)
To: Long Li; +Cc: linux-hyperv
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Improper PCI device reset via `pcie_flr()` leaves PCI core state out of sync.
Pre-existing issues:
- [Critical] Use-After-Free in EQ interrupt handler because the CQ is destroyed before the EQ is deregistered.
- [Critical] Tag reuse causing logical corruption when HWC commands time out.
- [High] Race condition leading to Use-After-Free on module unbind due to un-cancelled `mana_serv_work`.
--
--- Patch [5]: [PATCH net-next v2 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering ---
commit 7a0e58ab3c693baa720b6e21219cbe04fb7e9fcf
Author: Long Li <longli@microsoft.com>
net: mana: fix HWC teardown safety with setup_active flag and destroy ordering
This patch adds a setup_active flag to ensure safe hardware channel teardown
on initialization failures. It also reorders queue destruction to prevent
DMA-after-free races during teardown.
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 515c45c4a890f..5bc873d548d58 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
[ ... ]
> @@ -902,11 +926,58 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
[ ... ]
> + /* On systems without IOMMU, freeing DMA memory with
> + * active hardware MST mappings risks memory corruption.
> + * Issue FLR to force-reset the device and invalidate
> + * all hardware state including MST entries.
> + */
> + td_err = pcie_flr(to_pci_dev(gc->dev));
[Severity: High]
Does using pcie_flr() here instead of pci_reset_function() leave the
Linux PCI core completely unaware of the reset?
Since pcie_flr() does not save and restore the PCI configuration space,
such as BARs, MSI-X vectors, and Bus Master enable, could this leave the
hardware's PCI configuration wiped and out of sync with the kernel's
cached state, resulting in a wedged device?
> + if (td_err) {
[ ... ]
> + /* 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);
[Severity: Critical]
This is a pre-existing issue, but does destroying the CQ before the EQ
inside mana_hwc_destroy_cq() create a use-after-free window?
In mana_hwc_destroy_cq(), the CQ structure is freed first:
mana_hwc_destroy_cq() {
if (hwc_cq->gdma_cq)
mana_gd_destroy_queue(gc, hwc_cq->gdma_cq);
...
}
If the hardware posts an initialization event before the EQ is
subsequently destroyed, could the interrupt handler execute:
mana_hwc_init_event_handler() {
...
case HWC_INIT_DATA_CQID:
hwc->cq->gdma_cq->id = val;
break;
}
and write directly into the newly freed gdma_cq structure memory?
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but is there a race condition
leading to a use-after-free on module unbind due to un-cancelled service
work?
If a hardware reset event triggers mana_schedule_serv_work(), it queues
work on the global system_wq. If the driver is unbound via sysfs,
mana_gd_remove() frees the gdma_context via vfree(gc) without cancelling
this pending work.
Could the work then execute concurrently, calling mana_serv_reset() which
fetches the freed context:
mana_serv_reset() {
struct gdma_context *gc = pci_get_drvdata(pdev);
struct hw_channel_context *hwc = gc->hwc.driver_data;
...
}
and dereference the freed memory?
[Severity: Critical]
This is a pre-existing issue, but can tag reuse cause logical data
corruption when hardware channel commands time out?
When wait_for_completion_timeout() times out in mana_hwc_send_request(),
the function calls mana_hwc_put_msg_index() to free the msg_id for
immediate reuse.
Since the hardware is not reset, the timed-out command is still pending.
If another thread sends a new command and allocates the same msg_id,
what happens when the hardware eventually responds to the original
command?
Could mana_hwc_handle_resp() match this late response to the new command:
mana_hwc_handle_resp() {
ctx = hwc->caller_ctx + msg_id;
...
memcpy(ctx->output_buf, resp_msg, resp_len);
complete(&ctx->comp_event);
}
and complete it with stale data?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721234339.1476932-1-longli@microsoft.com?part=5
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 4/7] net: mana: validate hardware-supplied values in the HWC RX path
2026-07-21 23:43 ` [PATCH net-next v2 4/7] net: mana: validate hardware-supplied values in the HWC RX path Long Li
@ 2026-07-22 23:46 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 23:46 UTC (permalink / raw)
To: Long Li; +Cc: linux-hyperv
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but a race condition between the timeout path in `mana_hwc_send_request` and the hardware response handler leads to use-after-free stack corruption or tag reuse.
--
--- Patch [4]: [PATCH net-next v2 4/7] net: mana: validate hardware-supplied values in the HWC RX path ---
commit 20acf32024b20aecf06a023b18198388ad46e67f
Author: Long Li <longli@microsoft.com>
net: mana: validate hardware-supplied values in the HWC RX path
This commit adds validation to the hardware-supplied values in the MANA HWC
RX path. It prevents out-of-bounds access and stale response processing by
checking the SGE pointer, address, and response length against posted buffers.
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 2239fdeda57cb..515c45c4a890f 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);
[Severity: Critical]
This isn't a bug introduced by this patch, but is it possible for a delayed
hardware response to corrupt the stack here?
If a request times out in mana_hwc_send_request(), wait_for_completion_timeout()
expires and the function jumps to the "out" label where it calls
mana_hwc_put_msg_index(hwc, msg_id). This releases the msg_id for reuse without
clearing ctx->output_buf (which points to a buffer on the caller's stack).
If another thread subsequently allocates this same msg_id, it will set the
in-flight bit. If the delayed hardware response for the original request arrives
at this exact moment, mana_hwc_handle_resp() will pass the test_bit() check
above.
It will then proceed to copy the response data into ctx->output_buf:
drivers/net/ethernet/microsoft/mana/hw_channel.c:mana_hwc_handle_resp() {
...
if (!err && ctx->output_buf) {
ctx->status_code = resp_msg->status;
memcpy(ctx->output_buf, resp_msg, resp_len);
ctx->error = 0;
}
...
}
If the new thread hasn't yet updated ctx->output_buf, wouldn't this memcpy()
write DMA data directly into the timed-out caller's freed stack frame, causing
a use-after-free stack corruption?
Alternatively, if the new thread has already updated ctx->output_buf, could this
prematurely complete the new sender's request with stale data from the delayed
response?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721234339.1476932-1-longli@microsoft.com?part=4
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 2/7] net: mana: fix HWC RQ/SQ buffer size swap
2026-07-21 23:43 ` [PATCH net-next v2 2/7] net: mana: fix HWC RQ/SQ buffer size swap Long Li
@ 2026-07-22 23:46 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 23:46 UTC (permalink / raw)
To: Long Li; +Cc: linux-hyperv
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing hardware teardown on HWC establishment timeout leaves hardware with dangling DMA pointers, leading to Use-After-Free and Memory Corruption.
- [High] A malicious host/hypervisor can trigger an out-of-bounds read of kernel memory via a TOCTOU vulnerability in `mana_hwc_rx_event_handler`, leading to an information leak in Confidential VMs.
--
--- Patch [2]: [PATCH net-next v2 2/7] net: mana: fix HWC RQ/SQ buffer size swap ---
commit 1cb6d6f825cbbe1634272e9bfe16cb7adaa2c6b1
Author: Long Li <longli@microsoft.com>
net: mana: fix HWC RQ/SQ buffer size swap
The commit fixes a bug where mana_hwc_init_queues() sized the RQ and SQ
using the incorrect maximum message size variables. It swaps them to correct
the buffer sizes, stores max_resp_msg_size in the hardware channel context,
and uses it properly as the RX slot stride.
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 409e20caeccdc..3f011ebbe7b3e 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",
[Severity: High]
This is a pre-existing issue, but could this function be vulnerable to an
out-of-bounds read if the hypervisor provides a malicious
inline_oob_size_div4?
In mana_hwc_rx_event_handler(), the dma_oob pointer and its
inline_oob_size_div4 field are read from shared, unencrypted DMA memory:
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);
If a malicious hypervisor sends a CQE pointing to the last WQE slot in
the ring buffer and sets the 3-bit inline_oob_size_div4 field to 7, the
calculated sge pointer would point 4 bytes past the end of the 32-byte
WQE buffer allocation.
The driver then reads sge->address (an 8-byte out-of-bounds read) to
compute rx_req_idx. Since this value is derived from out-of-bounds
memory, it will likely exceed num_reqs and be logged to the kernel ring
buffer.
Could an untrusted hypervisor use this paravirtualized serial console log
to reverse the math and recover leaked encrypted guest memory plaintext
in a Confidential VM setup?
[ ... ]
> @@ -733,14 +733,14 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
> goto out;
> }
>
> - err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth, max_req_msg_size,
> + err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth, max_resp_msg_size,
> hwc->cq, &hwc->rxq);
[Severity: High]
This isn't a bug introduced by this patch, but is there a missing hardware
teardown when channel establishment times out?
In mana_hwc_establish_channel(), mana_smc_setup_hwc() hands the DMA
addresses of the queues to the hardware. If the driver then times out
waiting for the hwc_init_eqe_comp completion, it returns -ETIMEDOUT.
On the error path, this leads to mana_hwc_destroy_channel(), where the
teardown is gated by the number of completion queues:
if (gc->max_num_cqs > 0)
mana_smc_teardown_hwc(&gc->shm_channel, false);
Because the initialization event was never processed due to the timeout,
gc->max_num_cqs is still 0, causing the teardown to be skipped. The
driver then immediately frees the DMA queue memory.
If the hardware later wakes up and processes the setup request, could it
write to the freed DMA memory, resulting in memory corruption?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721234339.1476932-1-longli@microsoft.com?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-07-22 23:46 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21 23:43 [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth Long Li
2026-07-21 23:43 ` [PATCH net-next v2 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 2/7] net: mana: fix HWC RQ/SQ buffer size swap Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 3/7] net: mana: free HWC comp_buf after destroying the EQ Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 4/7] net: mana: validate hardware-supplied values in the HWC RX path Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering Long Li
2026-07-22 23:46 ` sashiko-bot
2026-07-21 23:43 ` [PATCH net-next v2 6/7] net: mana: support concurrent HWC requests with proper synchronization Long Li
2026-07-21 23:43 ` [PATCH net-next v2 7/7] net: mana: add dynamic HWC queue depth with reinit path Long Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox