All of lore.kernel.org
 help / color / mirror / Atom feed
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>,
	ernis@linux.microsoft.com, stephen@networkplumber.org
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH net v7 7/7] net: mana: keep max_num_cqs immutable once cq_table is allocated
Date: Thu, 13 Aug 2026 10:42:39 -0700	[thread overview]
Message-ID: <20260813174243.3044348-8-longli@microsoft.com> (raw)
In-Reply-To: <20260813174243.3044348-1-longli@microsoft.com>

mana_hwc_init_event_handler() applied every HWC_INIT_DATA_MAX_NUM_CQS
event straight to gc->max_num_cqs, but that handler stays live for the
whole channel lifetime, not just bootstrap.

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

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

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

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

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index b1269f7da0563a22c3cbe599df55572e36908139..d9bff4634dc35be772eaeea2c56bfe2562c9d6aa 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -209,7 +209,11 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
 			break;
 
 		case HWC_INIT_DATA_MAX_NUM_CQS:
-			gd->gdma_context->max_num_cqs = val;
+			/* Store only; establish_channel() commits it to
+			 * max_num_cqs once, so a later event cannot grow the
+			 * bound past the allocation.  Pairs with its READ_ONCE().
+			 */
+			WRITE_ONCE(hwc->hwc_init_max_num_cqs, val);
 			break;
 
 		case HWC_INIT_DATA_PDID:
@@ -783,6 +787,8 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	struct gdma_queue *eq = hwc->cq->gdma_eq;
 	struct gdma_queue *cq = hwc->cq->gdma_cq;
 	struct gdma_queue __rcu **cq_table;
+	u32 num_cqs;
+	u32 cq_id;
 	int err;
 
 	init_completion(&hwc->hwc_init_eqe_comp);
@@ -810,17 +816,32 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	*max_req_msg_size = hwc->hwc_init_max_req_msg_size;
 	*max_resp_msg_size = hwc->hwc_init_max_resp_msg_size;
 
-	/* Both were set in mana_hwc_init_event_handler(). */
-	if (WARN_ON(cq->id >= gc->max_num_cqs))
+	/* Snapshot the device-reported count and id once, so the same value
+	 * sizes, bounds and indexes cq_table even across the sleeping
+	 * vcalloc() and a concurrent init event.
+	 */
+	num_cqs = READ_ONCE(hwc->hwc_init_max_num_cqs);
+	cq_id = READ_ONCE(cq->id);
+
+	/* Both operands come from untrusted HWC bootstrap events; a missing
+	 * MAX_NUM_CQS leaves num_cqs at 0.  Reject rather than WARN_ON() so a
+	 * malformed device response cannot panic a panic_on_warn guest.
+	 */
+	if (cq_id >= num_cqs) {
+		dev_err_ratelimited(hwc->dev,
+				    "HWC: bad CQ id %u >= max %u\n",
+				    cq_id, num_cqs);
 		return -EPROTO;
+	}
 
-	cq_table = vcalloc(gc->max_num_cqs, sizeof(*cq_table));
+	cq_table = vcalloc(num_cqs, sizeof(*cq_table));
 	if (!cq_table)
 		return -ENOMEM;
 
-	/* Publish the initialised table; pairs with smp_load_acquire()
-	 * in mana_gd_get_cq().
+	/* Publish the bound and the initialised table together; the release
+	 * pairs with smp_load_acquire() in mana_gd_get_cq().
 	 */
+	gc->max_num_cqs = num_cqs;
 	smp_store_release(&gc->cq_table, cq_table);
 
 	/* Publish the HWC CQ now that the table is in place. */
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index ceabdc6242573f13d1284a967ea73bc01698e37d..f6cac0b0e44c5abd72c308ddc9278e4d85ed41ab 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -202,6 +202,7 @@ struct hw_channel_context {
 	u16 hwc_init_q_depth_max;
 	u32 hwc_init_max_req_msg_size;
 	u32 hwc_init_max_resp_msg_size;
+	u32 hwc_init_max_num_cqs;
 
 	struct completion hwc_init_eqe_comp;
 
-- 
2.43.0


      parent reply	other threads:[~2026-08-13 17:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13 17:42 [PATCH net v7 0/7] net: mana: HW channel reliability and hardening fixes Long Li
2026-08-13 17:42 ` [PATCH net v7 1/7] net: mana: reference-count CQs looked up from the EQ handler Long Li
2026-08-13 17:42 ` [PATCH net v7 2/7] net: mana: fix HWC RQ/SQ buffer size swap Long Li
2026-08-13 17:42 ` [PATCH net v7 3/7] net: mana: free HWC comp_buf after destroying the EQ Long Li
2026-08-13 17:42 ` [PATCH net v7 4/7] net: mana: validate hardware-supplied values in the HWC RX path Long Li
2026-08-13 17:42 ` [PATCH net v7 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering Long Li
2026-08-13 17:42 ` [PATCH net v7 6/7] net: mana: fix stale HWC response after command timeout Long Li
2026-08-13 17:42 ` Long Li [this message]

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=20260813174243.3044348-8-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=ernis@linux.microsoft.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=stephen@networkplumber.org \
    --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.