Re: [PATCH net] net: mana: Fix TOCTOU double-fetch of hwc_msg_id from DMA buffer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Unvalidated array index `rx_req_idx` derived from untrusted DMA memory allows arbitrary out-of-bounds memory access.
--

commit b79489b09c3449da038fde04cce5dc4d6a1fe538
Author: Erni Sri Satya Vennela <ernis@linux.microsoft.com>

net: mana: Fix TOCTOU double-fetch of hwc_msg_id from DMA buffer

This commit fixes a TOCTOU vulnerability in mana_hwc_rx_event_handler() where
the hwc_msg_id is read twice from DMA-coherent memory. It reads the msg_id once
using READ_ONCE() and passes it to mana_hwc_handle_resp() to prevent a malicious
host in a Confidential VM from bypassing bounds checks.

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index dbbde0fa57e71..fd8b324d7fb68 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c

[ ... ]

> @@ -269,13 +268,17 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
>  	rx_req = &hwc_rxq->msg_buf->reqs[rx_req_idx];

Since this patch addresses host-controlled DMA memory vulnerabilities in
Confidential VMs, could there be another out-of-bounds access just before
the msg_id check?

Looking at how rx_req_idx is calculated earlier in this function:

	sge = (struct gdma_sge *)(wqe + 8 + dma_oob->inline_oob_size_div4 * 4);
	rq_base_addr = hwc_rxq->msg_buf->mem_info.dma_handle;
	rx_req_idx = (sge->address - rq_base_addr) / hwc->max_req_msg_size;

Because sge->address is read directly from the untrusted DMA queue, a malicious
host could provide an arbitrary value, resulting in an out-of-bounds
rx_req_idx.

Using this unvalidated index to access hwc_rxq->msg_buf->reqs[rx_req_idx]
yields an attacker-controlled rx_req pointer. When this corrupted rx_req is
subsequently passed to mana_hwc_post_rx_wqe(), the driver writes to req->sge
and req->wqe_req, which could allow arbitrary writes to guest kernel memory.

Should rx_req_idx be bounds-checked against the size of the reqs array
before it is used to index the array?

>  	resp = (struct gdma_resp_hdr *)rx_req->buf_va;
>  
> -	if (resp->response.hwc_msg_id >= hwc->num_inflight_msg) {
> -		dev_err(hwc->dev, "HWC RX: wrong msg_id=%u\n",
> -			resp->response.hwc_msg_id);
> +	/* Read msg_id once from DMA buffer to prevent TOCTOU:
> +	 * DMA memory is shared/unencrypted in CVMs - host can
> +	 * modify it between reads.
> +	 */
> +	msg_id = READ_ONCE(resp->response.hwc_msg_id);

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