From: Long Li <longli@microsoft.com>
To: Long Li <longli@microsoft.com>,
Konstantin Taranov <kotaranov@microsoft.com>,
Jakub Kicinski <kuba@kernel.org>,
"David S . Miller" <davem@davemloft.net>,
Paolo Abeni <pabeni@redhat.com>,
Eric Dumazet <edumazet@google.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
Jason Gunthorpe <jgg@ziepe.ca>, Leon Romanovsky <leon@kernel.org>,
Haiyang Zhang <haiyangz@microsoft.com>,
"K . Y . Srinivasan" <kys@microsoft.com>,
Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
shradhagupta@linux.microsoft.com, Simon Horman <horms@kernel.org>
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH net-next 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering
Date: Tue, 14 Jul 2026 20:29:39 -0700 [thread overview]
Message-ID: <20260715032942.3945317-6-longli@microsoft.com> (raw)
In-Reply-To: <20260715032942.3945317-1-longli@microsoft.com>
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 | 106 ++++++++++++++----
include/net/mana/gdma.h | 8 +-
include/net/mana/hw_channel.h | 8 ++
3 files changed, 100 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 68236727aee8..b26c2122ebf5 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)
@@ -744,20 +745,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
@@ -766,6 +780,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,
@@ -878,11 +902,62 @@ 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 is wedged: teardown and FLR both failed.
+ * Hardware may still have active MST entries that
+ * allow DMA into our queue buffers.
+ *
+ * On IOMMU systems: dma_free_coherent() would unmap
+ * the IOVA, causing hardware DMA to fault at the
+ * IOMMU (safe). But on non-IOMMU systems, freeing
+ * the physical pages allows them to be reused for
+ * other purposes while hardware can still DMA to
+ * them (unsafe).
+ *
+ * but leak all DMA buffers to prevent corruption.
+ */
+
+ dev_warn(gc->dev,
+ "Leaked HWC DMA buffers (CQ/RQ/TXQ) to prevent memory corruption. Device is no longer usable.\n");
+
+ /* Do NOT proceed to mana_hwc_destroy_cq/wq — they
+ * would call dma_free_coherent(). Leave hwc, cq,
+ * rxq, txq allocated forever.
+ */
+ 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);
@@ -890,17 +965,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
next prev parent reply other threads:[~2026-07-15 3:30 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-15 3:29 [PATCH net-next 0/7] net: mana: harden the HWC and add dynamic queue depth Long Li
2026-07-15 3:29 ` [PATCH net-next 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy Long Li
2026-07-15 3:29 ` [PATCH net-next 2/7] net: mana: fix HWC RQ/SQ buffer size swap Long Li
2026-07-15 3:29 ` [PATCH net-next 3/7] net: mana: free HWC comp_buf after destroying the EQ Long Li
2026-07-15 3:29 ` [PATCH net-next 4/7] net: mana: validate hardware-supplied values in the HWC RX path Long Li
2026-07-15 3:29 ` Long Li [this message]
2026-07-15 3:29 ` [PATCH net-next 6/7] net: mana: support concurrent HWC requests with proper synchronization Long Li
2026-07-15 3:29 ` [PATCH net-next 7/7] net: mana: add dynamic HWC queue depth with reinit path Long Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260715032942.3945317-6-longli@microsoft.com \
--to=longli@microsoft.com \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=decui@microsoft.com \
--cc=edumazet@google.com \
--cc=haiyangz@microsoft.com \
--cc=horms@kernel.org \
--cc=jgg@ziepe.ca \
--cc=kotaranov@microsoft.com \
--cc=kuba@kernel.org \
--cc=kys@microsoft.com \
--cc=leon@kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=shradhagupta@linux.microsoft.com \
--cc=wei.liu@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.