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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox