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 <[email protected]> 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/[email protected]?part=1
