Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [Critical] A lack of synchronization between the request timeout path and the interrupt response handler leads to severe stack memory corruption and data leakage due to reused message IDs. - [High] An unvalidated `inline_oob_size_div4` value read from shared DMA memory is used to calculate the `sge` pointer, leading to an out-of-bounds read and kernel panic. -- --- Patch [4]: [PATCH net-next 4/7] net: mana: validate hardware-supplied values in the HWC RX path --- commit 2b4c8935e9727b2b08f9773b13a4caaa94f11d99 Author: Long Li <[email protected]> net: mana: validate hardware-supplied values in the HWC RX path This commit adds validation for lengths and indices received from device DMA in the hardware channel receive path. It verifies the SGE address matches the posted slot, ensures the response covers the header, bounds checks the message ID, and rejects oversized responses to prevent out of bounds access. > diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c > b/drivers/net/ethernet/microsoft/mana/hw_channel.c > index 2239fdeda57cb..68236727aee8b 100644 > --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c > +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c > @@ -83,6 +83,17 @@ static void mana_hwc_handle_resp(struct hw_channel_context > *hwc, u32 resp_len, > struct hwc_caller_ctx *ctx; > int err; > > + /* Validate msg_id is in range before using it to index bitmap > + * and caller_ctx array. Malicious firmware could send > + * out-of-range msg_id causing out-of-bounds access. > + */ > + if (msg_id >= hwc->num_inflight_msg) { > + dev_err(hwc->dev, "hwc_rx: msg_id %u >= max %u\n", > + msg_id, hwc->num_inflight_msg); > + mana_hwc_post_rx_wqe(hwc->rxq, rx_req); > + return; > + } > + > if (!test_bit(msg_id, hwc->inflight_msg_res.map)) { > dev_err(hwc->dev, "hwc_rx: invalid msg_id = %u\n", msg_id); > mana_hwc_post_rx_wqe(hwc->rxq, rx_req); > @@ -90,6 +101,18 @@ static void mana_hwc_handle_resp(struct > hw_channel_context *hwc, u32 resp_len, > } > > ctx = hwc->caller_ctx + msg_id; > + > + /* Reject responses larger than the RX DMA buffer — the SGE > + * limits what hardware can DMA, so an oversized resp_len > + * indicates a firmware bug. Fail rather than silently > + * truncating. > + */ > + if (resp_len > rx_req->buf_len) { > + dev_err(hwc->dev, "HWC RX: resp_len %u > buf_len %u\n", > + resp_len, rx_req->buf_len); > + resp_len = 0; > + } > + > err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len); > if (err) > goto out; [Severity: Critical] This isn't a bug introduced by this patch, but is there a race condition between the request timeout path and this interrupt response handler? If wait_for_completion_timeout() expires in mana_hwc_send_request(), the function clears the message ID bit and returns. This leaves ctx->output_buf dangling, pointing to the caller's stack frame. If the hardware response arrives immediately after, or if the message ID is reused by a new thread, could mana_hwc_handle_resp() read the ID and execute memcpy() into the dangling output_buf? Since no lock is held to verify the sender thread is still waiting, it seems this could lead to stack memory corruption and cross-request data leakage. [ ... ] > @@ -261,19 +284,45 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 > gdma_rxq_id, > > sge = (struct gdma_sge *)(wqe + 8 + dma_oob->inline_oob_size_div4 * 4); [Severity: High] This is a pre-existing issue, but does this calculation lack bounds checking for the sge pointer? The inline_oob_size_div4 field is read directly from the untrusted DMA WQE, multiplied by 4, and added to the WQE pointer. If the WQE is located at the very end of the page-aligned DMA ring buffer (for example, at offset 4064 in a 4096-byte page), could this unchecked addition cause the resulting sge pointer to exceed the queue buffer boundary? This could lead to an out-of-bounds read and kernel panic when sge->address is later dereferenced, which might be reachable by a malicious host in a confidential VM environment. > - /* Select the RX work request for virtual address and for reposting. */ > + /* Recover the originating RX slot from the SGE address. Of the three > + * terms here only sge->address lives in device-accessible RQ memory; > + * rq_base_addr and max_resp_msg_size are driver-private constants. An > + * in-range but wrong/unaligned SGE (corrupted WQE, or a malicious host > + * in a CVM) would otherwise truncate onto a neighbouring slot, letting > + * us read a stale response that could complete the wrong, reused > + * in-flight request. Require the index to be in range AND the address > + * to exactly match the value the driver posted for that slot. > + */ > rq_base_addr = hwc_rxq->msg_buf->mem_info.dma_handle; > rx_req_idx = (sge->address - rq_base_addr) / hwc->max_resp_msg_size; [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
