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

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

[kensanya]

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;
+	ioctx->n_rw_ctx = 0;
+	ioctx->n_rdma = 0;
 	return ret;
 }
 
-- 
2.25.1

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

[Leon Romanovsky]

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
> 

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

[kensanya]

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

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

[kensanya]

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