Re: [PATCH net-next] net: mana: Use the HWC destination queues reported by the hardware

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Timeout handling releases msg_id for immediate reuse without flushing the hardware queue or reinitializing completions, leading to data corruption and race conditions from late hardware completions.
--

commit fa131293a7bcd61d0524a5c94944a9ade71d8bda
Author: Manish Awasthi <mawasthi@linux.microsoft.com>

net: mana: Use the HWC destination queues reported by the hardware

This commit simplifies the MANA driver by unconditionally using the hardware-
reported destination queues for HWC requests. It removes the PF-specific
checks since the hardware already supplies values that are valid for the given
function.

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 263e7c4e29341..5d75c2478a8a8 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c

[ ... ]

> @@ -890,10 +889,13 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
>  	tx_wr->msg_size = req_len;
>  	command = req_msg->req.msg_type;
>  
> -	if (gc->is_pf) {
> -		dest_vrq = hwc->pf_dest_vrq_id;
> -		dest_vrcq = hwc->pf_dest_vrcq_id;
> -	}
> +	/* The hardware reports the HWC destination queues through
> +	 * HWC_INIT_DATA_PF_DEST_RQ_ID and HWC_INIT_DATA_PF_DEST_CQ_ID, and
> +	 * always supplies values that are valid for this function, so no
> +	 * PF-specific handling is needed here.
> +	 */
> +	dest_vrq = hwc->pf_dest_vrq_id;
> +	dest_vrcq = hwc->pf_dest_vrcq_id;
>  
>  	err = mana_hwc_post_tx_wqe(txq, tx_wr, dest_vrq, dest_vrcq, false);
>  	if (err) {

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing issue
in drivers/net/ethernet/microsoft/mana/hw_channel.c within 
mana_hwc_send_request() and mana_hwc_handle_resp().

If mana_hwc_send_request() times out waiting for the hardware:

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

It proceeds to release the msg_id for immediate reuse without flushing the
hardware queue:

out:
	mana_hwc_put_msg_index(hwc, msg_id);
	return err;

If a concurrent or subsequent thread calls mana_hwc_send_request() and reuses
this msg_id, could a late hardware response for the original timed-out request
cause memory corruption?

When the late response arrives, mana_hwc_handle_resp() would see that the
msg_id is currently active, copy the stale response into the newly allocated
request's buffer, and prematurely signal its completion event:

	if (!test_bit(msg_id, hwc->inflight_msg_res.map)) {
        ...
	}
	ctx = hwc->caller_ctx + msg_id;
	...
	memcpy(ctx->output_buf, resp_msg, resp_len);
out:
	...
	complete(&ctx->comp_event);

Because reinit_completion() is not called upon reuse, the completion event
would also be left in an incorrect signaled state. Does this race need to be
addressed to prevent data corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903184104.3194819-1-mawasthi@linux.microsoft.com?part=1