Re: [PATCH net] net: mana: validate rx_req_idx to prevent out-of-bounds array access

[sashiko-bot@kernel.org]

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

New issues:
- [High] Missing `READ_ONCE()` for `sge->address` risks Time-of-Check to Time-of-Use (TOCTOU) bypass of the newly added bounds check.

Pre-existing issues:
- [High] Missing bounds check on `dma_oob->inline_oob_size_div4` allows an out-of-bounds memory read past the DMA WQE ring buffer.
- [High] Returning early without reposting the RX WQE buffer leads to RX queue starvation.
--

commit 4f8436d78a8ca78fb96f7252f756af92fc42ccaa
Author: Aditya Garg <gargaditya@linux.microsoft.com>

net: mana: validate rx_req_idx to prevent out-of-bounds array access

This commit adds a bounds check on rx_req_idx in mana_hwc_rx_event_handler()
to prevent an out-of-bounds access into the reqs[] array. The index is
derived from unencrypted DMA-coherent memory which can be modified by the
hardware at any time in Confidential VMs.

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -258,11 +258,17 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
>  	wqe = mana_gd_get_wqe_ptr(rq, rx_oob->wqe_offset / GDMA_WQE_BU_SIZE);
>  	dma_oob = (struct gdma_wqe *)wqe;
>  
>  	sge = (struct gdma_sge *)(wqe + 8 + dma_oob->inline_oob_size_div4 * 4);

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing bounds
check on dma_oob->inline_oob_size_div4?

Since dma_oob->inline_oob_size_div4 is read directly from unencrypted, shared
DMA memory, a potentially malicious host could manipulate it. It is a 3-bit
field, so an attacker could set it to 7.

Could this cause the sge pointer calculation to point 36 bytes into the
32-byte WQE buffer, resulting in an out-of-bounds read of up to 12 bytes
when reading sge->address?

If the WQE is at the end of the page-aligned queue memory buffer, could this
read past the allocated DMA region and trigger a fatal page fault?

>  
>  	/* Select the RX work request for virtual address and for reposting. */
>  	rq_base_addr = hwc_rxq->msg_buf->mem_info.dma_handle;
>  	rx_req_idx = (sge->address - rq_base_addr) / hwc->max_req_msg_size;

[Severity: High]
Does sge->address need to be read using READ_ONCE()?

Since sge->address resides in unencrypted, shared DMA memory, the compiler
might fetch its value multiple times. If the division by hwc->max_req_msg_size
is optimized into a bitwise shift, the compiler might re-read sge->address
from DMA memory after the bounds check has passed when indexing the array.

Could a malicious host concurrently modify the memory to bypass the new bounds
check completely?

>  
> +	if (rx_req_idx >= hwc_rxq->msg_buf->num_reqs) {
> +		dev_err(hwc->dev, "HWC RX: wrong rx_req_idx=%llu, num_reqs=%u\n",
> +			rx_req_idx, hwc_rxq->msg_buf->num_reqs);
> +		return;
> +	}
> +

[Severity: High]
This is a pre-existing issue, but does returning early here (and further
below for invalid msg_id) drop the request without reposting the RX WQE
buffer?

If a malicious host repeatedly sends valid CQEs with an invalid msg_id or
an out-of-bounds rx_req_idx, it seems the function drops the response without
replenishing the queue via mana_hwc_post_rx_wqe().

Could this permanently leak the RX WQE buffers and starve the queue, causing
a denial of service for the hardware channel?

>  	rx_req = &hwc_rxq->msg_buf->reqs[rx_req_idx];
>  	resp = (struct gdma_resp_hdr *)rx_req->buf_va;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520051553.857120-1-gargaditya@linux.microsoft.com?part=1