> > @@ -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
This issue is already addressed in a separate patch by Aditya Garg
<[email protected]>, which is currently under internal
review.
- Vennela
