Re: [PATCH net-next v3 2/3] net/smc: transition to RDMA core CQ pooling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Race condition in `smc_wr_reg_send` causes a Use-After-Free when posting a work request to a terminating link.
- [High] Link teardown stall/deadlock due to inverted QP destruction sequence.
--

--- Patch [2]: [PATCH net-next v3 2/3] net/smc: transition to RDMA core CQ pooling ---
commit 7cb20cb1fe260499ce1bc5f3d7c63d07feedaebd
Author: D. Wythe <alibuda@linux.alibaba.com>
Subject: net/smc: transition to RDMA core CQ pooling

Refactors completion handling to use the ib_cqe API and standard RDMA core 
CQ pooling. This parallelizes completion processing across multiple CPU cores 
and removes redundant manual CQ management logic.

> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index cf6b620fef05f..5073af92e0868 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1373,7 +1368,8 @@ void smcr_link_clear(struct smc_link *lnk, bool log)
>  	smc_llc_link_clear(lnk, log);
>  	smcr_buf_unmap_lgr(lnk);
>  	smcr_rtoken_clear_link(lnk);
> -	smc_ib_modify_qp_error(lnk);
> +	smc_wr_stop_link(lnk);
> +	smc_wr_drain_qp(lnk);

[Severity: High]
Will this order of operations cause a multi-second stall during link teardown?

In smc_wr_stop_link, the teardown thread blocks on wait_for_completion
for the sender refcounts to drop. However, the queue pair is no longer put 
into an error state before this wait, as that now happens in smc_wr_drain_qp 
after the wait completes.

Without the queue pair in an error state, completion queue events are not 
generated. The quick wakeups in smc_wr_stop_link like smc_wr_wakeup_reg_wait 
appear ineffective because the condition check (such as 
link->wr_reg_state != POSTED) will remain false.

Could this cause the teardown thread to stall until the senders hit their 
timeout?

>  	smc_wr_free_link(lnk);
>  	smc_ib_destroy_queue_pair(lnk);
>  	smc_ib_dealloc_protection_domain(lnk);
> diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
> index 6b5add9229937..675bfe633e395 100644
> --- a/net/smc/smc_wr.c
> +++ b/net/smc/smc_wr.c
> [ ... ]
> @@ -367,17 +306,16 @@ int smc_wr_reg_send(struct smc_link *link, struct ib_mr *mr)
>  {
>  	int rc;
>  
> -	ib_req_notify_cq(link->smcibdev->roce_cq_send,
> -			 IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS);
>  	link->wr_reg_state = POSTED;
> -	link->wr_reg.wr.wr_id = (u64)(uintptr_t)mr;
>  	link->wr_reg.mr = mr;
>  	link->wr_reg.key = mr->rkey;
>  	rc = ib_post_send(link->roce_qp, &link->wr_reg.wr, NULL);
>  	if (rc)
>  		return rc;
>  
> -	percpu_ref_get(&link->wr_reg_refs);
> +	if (!percpu_ref_tryget_live(&link->wr_reg_refs))
> +		return -EPIPE;
> +

[Severity: High]
Can this lead to a use-after-free if the link is being torn down concurrently?

If percpu_ref_tryget_live fails, the function returns -EPIPE immediately 
and the caller may proceed to free the link memory. However, the work request 
has already been posted to the hardware queue via ib_post_send on the 
previous lines.

When the completion event eventually arrives, the completion handler 
smc_wr_reg_process_cqe executes:

smc_wr_reg_process_cqe()
        ...
        if (wc->status)
                link->wr_reg_state = FAILED;

If the link memory was already freed, won't writing to link->wr_reg_state 
corrupt memory?

>  	rc = wait_event_interruptible_timeout(link->wr_reg_wait,
>  					      (link->wr_reg_state != POSTED),
>  					      SMC_WR_REG_MR_WAIT_TIME);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710033356.16460-1-alibuda@linux.alibaba.com?part=2