Re: [PATCH net-next v2 4/7] 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] This problem wasn't introduced by this patch, but a race condition between the timeout path in `mana_hwc_send_request` and the hardware response handler leads to use-after-free stack corruption or tag reuse.
--

--- Patch [4]: [PATCH net-next v2 4/7] net: mana: validate hardware-supplied values in the HWC RX path ---
commit 20acf32024b20aecf06a023b18198388ad46e67f
Author: Long Li <longli@microsoft.com>

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

This commit adds validation to the hardware-supplied values in the MANA HWC
RX path. It prevents out-of-bounds access and stale response processing by
checking the SGE pointer, address, and response length against posted buffers.

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 2239fdeda57cb..515c45c4a890f 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);

[Severity: Critical]
This isn't a bug introduced by this patch, but is it possible for a delayed
hardware response to corrupt the stack here?

If a request times out in mana_hwc_send_request(), wait_for_completion_timeout()
expires and the function jumps to the "out" label where it calls
mana_hwc_put_msg_index(hwc, msg_id). This releases the msg_id for reuse without
clearing ctx->output_buf (which points to a buffer on the caller's stack).

If another thread subsequently allocates this same msg_id, it will set the
in-flight bit. If the delayed hardware response for the original request arrives
at this exact moment, mana_hwc_handle_resp() will pass the test_bit() check
above.

It will then proceed to copy the response data into ctx->output_buf:

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;
    }
    ...
}

If the new thread hasn't yet updated ctx->output_buf, wouldn't this memcpy()
write DMA data directly into the timed-out caller's freed stack frame, causing
a use-after-free stack corruption?

Alternatively, if the new thread has already updated ctx->output_buf, could this
prematurely complete the new sender's request with stale data from the delayed
response?

[ ... ]

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