Re:Re:Re: [PATCH] RDMA/srpt: Fix srpt_alloc_rw_ctxs() unwind counters

[kensanya <kensanya@163.com>]

At 2026-07-15 17:57:59, "kensanya" <kensanya@163.com> wrote:
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>At 2026-07-15 17:05:57, "Leon Romanovsky" <leon@kernel.org> wrote:
>>On Wed, Jul 15, 2026 at 10:30:16AM +0800, kensanya@163.com wrote:
>>> From: TanZheng <tanzheng@kylinos.cn>
>>> 
>>> When srpt_alloc_rw_ctxs() fails partway through a multi-buffer indirect
>>> descriptor, the unwind path destroys RDMA contexts but leaves stale
>>> n_rw_ctx and n_rdma values (and a dangling rw_ctxs pointer). Later
>>> sq_wr_avail accounting in srpt_queue_response() or srpt_write_pending()
>>> can then subtract the wrong number of send queue credits.
>>> 
>>> Reset the counters and rw_ctxs pointer before returning an error.
>>> 
>>> Fixes: b99f8e4d7bcd ("IB/srpt: convert to the generic RDMA READ/WRITE API")
>>> Signed-off-by: TanZheng <tanzheng@kylinos.cn>
>>> ---
>>>  drivers/infiniband/ulp/srpt/ib_srpt.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>> 
>>> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
>>> index f66cfd70c263..4644429fae14 100644
>>> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
>>> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
>>> @@ -1016,6 +1016,9 @@ static int srpt_alloc_rw_ctxs(struct srpt_send_ioctx *ioctx,
>>>  	}
>>>  	if (ioctx->rw_ctxs != &ioctx->s_rw_ctx)
>>>  		kfree(ioctx->rw_ctxs);
>>> +	ioctx->rw_ctxs = &ioctx->s_rw_ctx;
>>
>>This line seems questionable to me.
>>You probably need to write:
>>if (ioctx->rw_ctxs != &ioctx->s_rw_ctx) {
>>  kfree(ioctx->rw_ctxs);
>>  ioctx->rw_ctxs = NULL;
>>}
>>
>>> +	ioctx->n_rw_ctx = 0;
>>> +	ioctx->n_rdma = 0;
>>
>>These lines seem correct to me.
>>
>>>  	return ret;
>>>  }
>>>  
>>> -- 
>>> 2.25.1
>>> 
>Hi Bart,
>
>Thanks for the review.
>
>I originally set rw_ctxs back to &ioctx->s_rw_ctx after kfree()
>because that is the only other valid value used in this file
>(nbufs == 1 path), and I wanted to clear the dangling pointer
>while keeping the existing "embedded vs heap" convention.
>
>This is indeed misleading.I will change it to:
>	if (ioctx->rw_ctxs != &ioctx->s_rw_ctx) {
>		kfree(ioctx->rw_ctxs);
>		ioctx->rw_ctxs = NULL;
>	}
>and keep the n_rw_ctx / n_rdma resets. Will send a v2.
>
>Thanks,
>TanZheng
>
Hi Leon.

I'm very sorry, I made a mistake.

Thanks,
TanZheng