Thread (8 messages) flat view 8 messages, 1 author, 3d ago
WARM2d

Revision v7 of 4 in this series.

Revisions (4)
  1. v3 [diff vs current]
  2. v5 [diff vs current]
  3. v6 [diff vs current]
  4. v7 current

[PATCH net v7 7/7] net: mana: keep max_num_cqs immutable once cq_table is allocated

From: Long Li <longli@microsoft.com>
Date: 2026-08-13 17:43:16
Also in: linux-hyperv, linux-rdma, lkml
Subsystem: hyper-v/azure core and drivers, networking drivers, networking [general], the rest · Maintainers: "K. Y. Srinivasan", Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help