Re: [PATCH net v3 2/6] net: mana: fix HWC RQ/SQ buffer size swap

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Out-of-Bounds Memory Read due to unvalidated `inline_oob_size_div4` from untrusted DMA memory.
- [High] Use-After-Free during hardware channel teardown due to incorrect destruction order and premature freeing of buffers.
- [High] Race condition / UAF in `mana_hwc_send_request` due to uncoordinated timeout handling.
--

--- Patch [2]: [PATCH net v3 2/6] net: mana: fix HWC RQ/SQ buffer size swap ---
commit 2b43add14b8efbca0def0e3777e0b101884e1da7
Author: Long Li <longli@microsoft.com>

net: mana: fix HWC RQ/SQ buffer size swap

This commit fixes an issue where the HWC RQ and SQ buffer sizes were
swapped during initialization. It ensures the RQ is sized with
max_resp_msg_size and the SQ with max_req_msg_size. The commit also
reorders the assignment of these queue dimensions before creating the CQ,
as the RX completion handler depends on them being set.

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 409e20caeccdc..cbb56c7647876 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -263,7 +263,7 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
>  
>  	/* 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;
> +	rx_req_idx = (sge->address - rq_base_addr) / hwc->max_resp_msg_size;

[Severity: High]
This is a pre-existing issue, but does this function have an out-of-bounds
memory read when parsing the WQE?

Looking at the code just before this modification in
mana_hwc_rx_event_handler():

	dma_oob = (struct gdma_wqe *)wqe;
	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_resp_msg_size;

Because inline_oob_size_div4 is read directly from the untrusted DMA buffer
without validation, could a compromised hypervisor provide a large value
that causes the sge pointer to point past the end of the mapped buffer,
resulting in a kernel panic when sge->address is dereferenced?

>  
>  	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",

[ ... ]

[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk during
hardware channel teardown?

In mana_hwc_destroy_channel():

	if (hwc->txq)
		mana_hwc_destroy_wq(hwc, hwc->txq);

	if (hwc->rxq)
		mana_hwc_destroy_wq(hwc, hwc->rxq);

	if (hwc->cq)
		mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);

The queues (txq and rxq) are destroyed and their memory is freed before the
CQ's RCU synchronization barrier is reached.

Inside mana_hwc_destroy_cq():

	kfree(hwc_cq->comp_buf);

	if (hwc_cq->gdma_cq)
		mana_gd_destroy_queue(gc, hwc_cq->gdma_cq);

The comp_buf array is freed immediately, but the synchronize_rcu() barrier
sits inside mana_gd_destroy_queue(). Does this allow in-flight IRQ handlers
to access freed memory (txq, rxq, and comp_buf) before the RCU barrier
completes?

[Severity: High]
This is a pre-existing issue, but is there a race condition between the
timeout path and the response handler that could cause stack corruption?

In mana_hwc_send_request():

	if (!wait_for_completion_timeout(&ctx->comp_event,
					 (msecs_to_jiffies(hwc->hwc_timeout)))) {
		...
		err = -ETIMEDOUT;
		goto out;
	}
	...
out:
	mana_hwc_put_msg_index(hwc, msg_id);
	return err;

The timeout path exits and frees the message ID without synchronizing with
the receiver thread or clearing ctx->output_buf.

Meanwhile, in mana_hwc_handle_resp():

	if (!test_bit(msg_id, hwc->inflight_msg_res.map)) {
		...
	ctx = hwc->caller_ctx + msg_id;
	...
	memcpy(ctx->output_buf, resp_msg, resp_len);

The test_bit() check is done locklessly. If a timeout occurs right after
test_bit() succeeds, mana_hwc_send_request() will return and pop the
caller's stack frame. Will the response handler then memcpy() into a
dangling stack pointer (ctx->output_buf), corrupting the stack?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803234355.636038-1-longli@microsoft.com?part=2