All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE
@ 2026-07-11 15:06 Michael Bommarito
  2026-07-12 15:06 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Michael Bommarito @ 2026-07-11 15:06 UTC (permalink / raw)
  To: Haiyang Zhang, Dexuan Cui, Long Li
  Cc: K . Y . Srinivasan, Wei Liu, Andrew Lunn, Jakub Kicinski,
	Paolo Abeni, netdev, linux-hyperv, linux-kernel, stable

mana_hwc_init_event_handler() in hw_channel.c stores device-advertised
HWC_INIT_DATA_MAX_REQUEST and HWC_INIT_DATA_MAX_RESPONSE values
without bounds checking. mana_hwc_alloc_dma_buf() later computes the
DMA buffer size as MANA_PAGE_ALIGN(q_depth * max_msg_size) in 32-bit
arithmetic. A malicious device returning a large max_msg_size causes
the product to wrap, allocating a small buffer while laying out
q_depth request slots at the unwrapped stride, placing slots outside
the allocation.

Impact: a compromised hypervisor device model or malicious MANA PCI
device can cause out-of-bounds DMA buffer writes during HWC channel
initialization. A reproducer is available on request.

Clamp both values to HW_CHANNEL_MAX_REQUEST_SIZE (4096), consistent
with the cap already applied at the channel-create callsite.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 48a9acea4ab6c..a0916b50cffce 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -152,10 +152,14 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
 			break;
 
 		case HWC_INIT_DATA_MAX_REQUEST:
+			if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
+				val = HW_CHANNEL_MAX_REQUEST_SIZE;
 			hwc->hwc_init_max_req_msg_size = val;
 			break;
 
 		case HWC_INIT_DATA_MAX_RESPONSE:
+			if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
+				val = HW_CHANNEL_MAX_REQUEST_SIZE;
 			hwc->hwc_init_max_resp_msg_size = val;
 			break;
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE
  2026-07-11 15:06 [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE Michael Bommarito
@ 2026-07-12 15:06 ` sashiko-bot
  2026-07-20 10:40 ` Simon Horman
  2026-07-20 20:07 ` Erni Sri Satya Vennela
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-07-12 15:06 UTC (permalink / raw)
  To: Michael Bommarito; +Cc: linux-hyperv

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] The commit message describes an integer overflow vulnerability in `mana_hwc_alloc_dma_buf()` caused by using unbounded device-advertised message sizes, but this vulnerability is entirely hallucinated and the modified variables are dead code.
- [Low] The patch incorrectly clamps `HWC_INIT_DATA_MAX_RESPONSE` using `HW_CHANNEL_MAX_REQUEST_SIZE` instead of `HW_CHANNEL_MAX_RESPONSE_SIZE`.

Pre-existing issues:
- [High] An unchecked `eq_db.doorbell` index from a hardware event causes an out-of-bounds MMIO write during doorbell ringing.
- [High] An unchecked `max_num_cqs` value retrieved from the hardware event allows a malicious device to trigger enormous kernel memory allocations.
--

--- Patch [1]: [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE ---
commit ea80048daa839e9f12f06618139e15fddebc0ac5
Author: Michael Bommarito <michael.bommarito@gmail.com>

net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE

The commit adds bounds checking for max message size during HWC initialization.
It claims this prevents an out-of-bounds DMA buffer write in
mana_hwc_alloc_dma_buf().

>    mana_hwc_init_event_handler() in hw_channel.c stores device-advertised
>    HWC_INIT_DATA_MAX_REQUEST and HWC_INIT_DATA_MAX_RESPONSE values
>    without bounds checking. mana_hwc_alloc_dma_buf() later computes the
>    DMA buffer size as MANA_PAGE_ALIGN(q_depth * max_msg_size) in 32-bit
>    arithmetic. A malicious device returning a large max_msg_size causes
>    the product to wrap, allocating a small buffer while laying out
>    q_depth request slots at the unwrapped stride, placing slots outside
>    the allocation.

[Severity: Medium]
Is this description accurate?

The modified variables (hwc->hwc_init_max_req_msg_size and
hwc->hwc_init_max_resp_msg_size) appear to be dead code that do not affect
memory allocation. mana_hwc_alloc_dma_buf() is invoked before the event
handler runs (via mana_hwc_init_queues()) using hardcoded sizes (4096),
not the hardware-provided ones.

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index e3c24d50dad07..94594fba112f9 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -150,10 +150,14 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
>  			break;
>  
>  		case HWC_INIT_DATA_MAX_REQUEST:
> +			if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
> +				val = HW_CHANNEL_MAX_REQUEST_SIZE;
>  			hwc->hwc_init_max_req_msg_size = val;
>  			break;
>  
>  		case HWC_INIT_DATA_MAX_RESPONSE:
> +			if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
> +				val = HW_CHANNEL_MAX_REQUEST_SIZE;

[Severity: Low]
Does this incorrectly clamp HWC_INIT_DATA_MAX_RESPONSE using
HW_CHANNEL_MAX_REQUEST_SIZE instead of HW_CHANNEL_MAX_RESPONSE_SIZE?

>  			hwc->hwc_init_max_resp_msg_size = val;
>  			break;
>  

[Severity: High]
This is a pre-existing issue, but is it possible for a malicious device
to trigger an out-of-bounds MMIO write via an unchecked doorbell index?

Earlier in mana_hwc_init_event_handler():

	case GDMA_EQE_HWC_INIT_EQ_ID_DB:
		eq_db.as_uint32 = event->details[0];
		hwc->cq->gdma_eq->id = eq_db.eq_id;
		gd->doorbell = eq_db.doorbell;

When the driver posts WQEs and rings the doorbell via mana_gd_ring_doorbell():

drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_ring_doorbell() {
    ...
    void __iomem *addr = gc->db_page_base + gc->db_page_size * db_index;
    ...
    writeq(e.as_uint64, addr);
    ...
}

If eq_db.doorbell is excessively large, could it calculate a memory-mapped
address that exceeds gc->bar0_size, resulting in an out-of-bounds writeq()
that corrupts adjacent mappings?


[Severity: High]
This is a pre-existing issue, but does the unchecked max_num_cqs value
allow a malicious device to exhaust system memory?

Later in mana_hwc_init_event_handler():

		case HWC_INIT_DATA_MAX_NUM_CQS:
			gd->gdma_context->max_num_cqs = val;
			break;

This unbounded max_num_cqs value is later used for memory allocation:

drivers/net/ethernet/microsoft/mana/hw_channel.c:mana_hwc_establish_channel() {
    ...
	gc->cq_table = vcalloc(gc->max_num_cqs, sizeof(struct gdma_queue *));
    ...
}

If max_num_cqs is an exceptionally large 32-bit value (e.g., 0x1FFFFFFF),
could vcalloc() attempt to allocate multiple gigabytes of memory and cause a
host-level denial of service?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711150628.2914205-1-michael.bommarito@gmail.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE
  2026-07-11 15:06 [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE Michael Bommarito
  2026-07-12 15:06 ` sashiko-bot
@ 2026-07-20 10:40 ` Simon Horman
  2026-07-20 20:07 ` Erni Sri Satya Vennela
  2 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2026-07-20 10:40 UTC (permalink / raw)
  To: michael.bommarito
  Cc: Simon Horman, haiyangz, decui, longli, kys, wei.liu,
	andrew+netdev, kuba, pabeni, netdev, linux-hyperv, linux-kernel,
	stable

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
https://sashiko.dev/#/patchset/20260711150628.2914205-1-michael.bommarito@gmail.com?part=1
---
--- Patch [1]: [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE ---
commit ea80048daa839e9f12f06618139e15fddebc0ac5
Author: Michael Bommarito <michael.bommarito@gmail.com>

net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE

The commit adds bounds checking for max message size during HWC initialization.
It claims this prevents an out-of-bounds DMA buffer write in
mana_hwc_alloc_dma_buf().

>    mana_hwc_init_event_handler() in hw_channel.c stores device-advertised
>    HWC_INIT_DATA_MAX_REQUEST and HWC_INIT_DATA_MAX_RESPONSE values
>    without bounds checking. mana_hwc_alloc_dma_buf() later computes the
>    DMA buffer size as MANA_PAGE_ALIGN(q_depth * max_msg_size) in 32-bit
>    arithmetic. A malicious device returning a large max_msg_size causes
>    the product to wrap, allocating a small buffer while laying out
>    q_depth request slots at the unwrapped stride, placing slots outside
>    the allocation.

[Severity: Medium]
Is this description accurate?

The modified variables (hwc->hwc_init_max_req_msg_size and
hwc->hwc_init_max_resp_msg_size) appear to be dead code that do not affect
memory allocation. mana_hwc_alloc_dma_buf() is invoked before the event
handler runs (via mana_hwc_init_queues()) using hardcoded sizes (4096),
not the hardware-provided ones.

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index e3c24d50dad07..94594fba112f9 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -150,10 +150,14 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
>  			break;
>  
>  		case HWC_INIT_DATA_MAX_REQUEST:
> +			if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
> +				val = HW_CHANNEL_MAX_REQUEST_SIZE;
>  			hwc->hwc_init_max_req_msg_size = val;
>  			break;
>  
>  		case HWC_INIT_DATA_MAX_RESPONSE:
> +			if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
> +				val = HW_CHANNEL_MAX_REQUEST_SIZE;

[Severity: Low]
Does this incorrectly clamp HWC_INIT_DATA_MAX_RESPONSE using
HW_CHANNEL_MAX_REQUEST_SIZE instead of HW_CHANNEL_MAX_RESPONSE_SIZE?

>  			hwc->hwc_init_max_resp_msg_size = val;
>  			break;
>  

[Severity: High]
This is a pre-existing issue, but is it possible for a malicious device
to trigger an out-of-bounds MMIO write via an unchecked doorbell index?

Earlier in mana_hwc_init_event_handler():

	case GDMA_EQE_HWC_INIT_EQ_ID_DB:
		eq_db.as_uint32 = event->details[0];
		hwc->cq->gdma_eq->id = eq_db.eq_id;
		gd->doorbell = eq_db.doorbell;

When the driver posts WQEs and rings the doorbell via mana_gd_ring_doorbell():

drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_ring_doorbell() {
    ...
    void __iomem *addr = gc->db_page_base + gc->db_page_size * db_index;
    ...
    writeq(e.as_uint64, addr);
    ...
}

If eq_db.doorbell is excessively large, could it calculate a memory-mapped
address that exceeds gc->bar0_size, resulting in an out-of-bounds writeq()
that corrupts adjacent mappings?


[Severity: High]
This is a pre-existing issue, but does the unchecked max_num_cqs value
allow a malicious device to exhaust system memory?

Later in mana_hwc_init_event_handler():

		case HWC_INIT_DATA_MAX_NUM_CQS:
			gd->gdma_context->max_num_cqs = val;
			break;

This unbounded max_num_cqs value is later used for memory allocation:

drivers/net/ethernet/microsoft/mana/hw_channel.c:mana_hwc_establish_channel() {
    ...
	gc->cq_table = vcalloc(gc->max_num_cqs, sizeof(struct gdma_queue *));
    ...
}

If max_num_cqs is an exceptionally large 32-bit value (e.g., 0x1FFFFFFF),
could vcalloc() attempt to allocate multiple gigabytes of memory and cause a
host-level denial of service?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE
  2026-07-11 15:06 [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE Michael Bommarito
  2026-07-12 15:06 ` sashiko-bot
  2026-07-20 10:40 ` Simon Horman
@ 2026-07-20 20:07 ` Erni Sri Satya Vennela
  2 siblings, 0 replies; 4+ messages in thread
From: Erni Sri Satya Vennela @ 2026-07-20 20:07 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: Haiyang Zhang, Dexuan Cui, Long Li, K . Y . Srinivasan, Wei Liu,
	Andrew Lunn, Jakub Kicinski, Paolo Abeni, netdev, linux-hyperv,
	linux-kernel, stable

On Sat, Jul 11, 2026 at 11:06:28AM -0400, Michael Bommarito wrote:
> mana_hwc_init_event_handler() in hw_channel.c stores device-advertised
> HWC_INIT_DATA_MAX_REQUEST and HWC_INIT_DATA_MAX_RESPONSE values
> without bounds checking. mana_hwc_alloc_dma_buf() later computes the
> DMA buffer size as MANA_PAGE_ALIGN(q_depth * max_msg_size) in 32-bit
> arithmetic. A malicious device returning a large max_msg_size causes
> the product to wrap, allocating a small buffer while laying out
> q_depth request slots at the unwrapped stride, placing slots outside
> the allocation.

I don't think the described data flow actually
exists in the current tree, so the security framing looks inaccurate.
Please check the comment below.
> 
> Impact: a compromised hypervisor device model or malicious MANA PCI
> device can cause out-of-bounds DMA buffer writes during HWC channel
> initialization. A reproducer is available on request.
> 
> Clamp both values to HW_CHANNEL_MAX_REQUEST_SIZE (4096), consistent
> with the cap already applied at the channel-create callsite.
> 
> Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 48a9acea4ab6c..a0916b50cffce 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -152,10 +152,14 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
>  			break;
>  
>  		case HWC_INIT_DATA_MAX_REQUEST:
> +			if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
> +				val = HW_CHANNEL_MAX_REQUEST_SIZE;
>  			hwc->hwc_init_max_req_msg_size = val;
>  			break;
>  
>  		case HWC_INIT_DATA_MAX_RESPONSE:
> +			if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
> +				val = HW_CHANNEL_MAX_REQUEST_SIZE;
>  			hwc->hwc_init_max_resp_msg_size = val;
>  			break;
>  

The clamp is applied to hwc->hwc_init_max_req_msg_size and
hwc->hwc_init_max_resp_msg_size. Tracing where those two fields are
consumed:

  mana_hwc_init_event_handler()
	|
  mana_hwc_establish_channel() // copies them out to *max_req_msg_size
        |                          and *max_resp_msg_size
  mana_hwc_create_channel()    // passes those locals only to
        |                          mana_hwc_test_channel()
  mana_hwc_test_channel()      // passed as parameters but never
                                   used them

The DMA buffers that alloc_dma_buf() sizes are created from
mana_hwc_init_queues(), which is called with the compile-time constants:

    err = mana_hwc_init_queues(hwc, HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
                   HW_CHANNEL_MAX_REQUEST_SIZE,
                   HW_CHANNEL_MAX_RESPONSE_SIZE);

Therefore, q_depth * max_msg_size cannot wrap from a device-controlled
value here.

Thanks,
Vennela
> -- 
> 2.53.0
> 

> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-20 20:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-11 15:06 [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE Michael Bommarito
2026-07-12 15:06 ` sashiko-bot
2026-07-20 10:40 ` Simon Horman
2026-07-20 20:07 ` Erni Sri Satya Vennela

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.