Re: [PATCH net v3 4/6] net: mana: validate hardware-supplied values in the HWC RX path

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] A timeout in mana_hwc_send_request() can lead to stack corruption and use-after-free if the hardware response arrives concurrently or is delayed.
--

--- Patch [4]: [PATCH net v3 4/6] net: mana: validate hardware-supplied values in the HWC RX path ---
commit c5cc55c1f3f3c4968b8391ae19f32c3ab101adc9
Author: Long Li <longli@microsoft.com>

net: mana: validate hardware-supplied values in the HWC RX path

This commit validates lengths and indices from the device DMA in the HWC RX
path. It adds bounds checks and address matching to prevent out-of-bounds
accesses or reading stale responses. A reposting mechanism is added for
rejected responses to maintain posted RQ depth.

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index d701c427fe47b..e378b8ec97c9a 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;
>  
[ ... ]
>  	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 is a pre-existing issue, but I have a question about how timeouts are
handled when processing these contexts.

If a thread times out in mana_hwc_send_request() while waiting for
ctx->comp_event, does it return and free the stack frame where the
response buffer (ctx->output_buf) might be allocated?

drivers/net/ethernet/microsoft/mana/hw_channel.c: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;
}

Because the sender doesn't clear ctx->output_buf or synchronize with the RX
handler when returning on a timeout, could a delayed hardware response arriving
later lead to stack corruption when mana_hwc_handle_resp() executes the
memcpy() into the now-dangling ctx->output_buf pointer?

drivers/net/ethernet/microsoft/mana/hw_channel.c:mana_hwc_handle_resp() {
    ...
	if (!err && ctx->output_buf) {
		ctx->status_code = resp_msg->status;
		memcpy(ctx->output_buf, resp_msg, resp_len);
		ctx->error = 0;
	}
    ...
}

Additionally, since there is no reinit_completion() call when allocating a
message slot, could a newly reused slot immediately consume a stale completion
token from a delayed response and return prematurely?

Could a malicious host or firmware in a Confidential VM environment exploit
this by intentionally delaying responses to overwrite the guest kernel stack?

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