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

[PATCH v2] 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 clear rw_ctxs after freeing the heap
allocation before returning an error.

Fixes: b99f8e4d7bcd ("IB/srpt: convert to the generic RDMA READ/WRITE API")
Signed-off-by: TanZheng <tanzheng@kylinos.cn>
---
v2:
- After kfree(), set rw_ctxs to NULL instead of &s_rw_ctx
  (Leon Romanovsky)

 drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index f66cfd70c263..a9c4995af7a3 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1014,8 +1014,12 @@ static int srpt_alloc_rw_ctxs(struct srpt_send_ioctx *ioctx,
 				ctx->sg, ctx->nents, dir);
 		target_free_sgl(ctx->sg, ctx->nents);
 	}
-	if (ioctx->rw_ctxs != &ioctx->s_rw_ctx)
+	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;
 	return ret;
 }
 
-- 
2.25.1

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

[Leon Romanovsky]

On Wed, 15 Jul 2026 18:15:50 +0800, kensanya@163.com wrote:
> 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 clear rw_ctxs after freeing the heap
> allocation before returning an error.
> 
> [...]

Applied, thanks!

[1/1] RDMA/srpt: Fix srpt_alloc_rw_ctxs() unwind counters
      https://git.kernel.org/rdma/rdma/c/91997e188f68af

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>

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

[Bart Van Assche]

On 7/15/26 3:15 AM, 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 clear rw_ctxs after freeing the heap
> allocation before returning an error.
> 
> Fixes: b99f8e4d7bcd ("IB/srpt: convert to the generic RDMA READ/WRITE API")
> Signed-off-by: TanZheng <tanzheng@kylinos.cn>
> ---
> v2:
> - After kfree(), set rw_ctxs to NULL instead of &s_rw_ctx
>    (Leon Romanovsky)
> 
>   drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index f66cfd70c263..a9c4995af7a3 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -1014,8 +1014,12 @@ static int srpt_alloc_rw_ctxs(struct srpt_send_ioctx *ioctx,
>   				ctx->sg, ctx->nents, dir);
>   		target_free_sgl(ctx->sg, ctx->nents);
>   	}
> -	if (ioctx->rw_ctxs != &ioctx->s_rw_ctx)
> +	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;
>   	return ret;
>   }

The above looks wrong to me. In the error path ioctx->n_rw_ctx should be
restored to the value it had at the start of the function instead of
resetting it to zero.

Bart.

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

[Leon Romanovsky]

On Thu, Jul 16, 2026 at 10:37:39AM -0700, Bart Van Assche wrote:
> On 7/15/26 3:15 AM, 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 clear rw_ctxs after freeing the heap
> > allocation before returning an error.
> > 
> > Fixes: b99f8e4d7bcd ("IB/srpt: convert to the generic RDMA READ/WRITE API")
> > Signed-off-by: TanZheng <tanzheng@kylinos.cn>
> > ---
> > v2:
> > - After kfree(), set rw_ctxs to NULL instead of &s_rw_ctx
> >    (Leon Romanovsky)
> > 
> >   drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > index f66cfd70c263..a9c4995af7a3 100644
> > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > @@ -1014,8 +1014,12 @@ static int srpt_alloc_rw_ctxs(struct srpt_send_ioctx *ioctx,
> >   				ctx->sg, ctx->nents, dir);
> >   		target_free_sgl(ctx->sg, ctx->nents);
> >   	}
> > -	if (ioctx->rw_ctxs != &ioctx->s_rw_ctx)
> > +	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;
> >   	return ret;
> >   }
> 
> The above looks wrong to me. In the error path ioctx->n_rw_ctx should be
> restored to the value it had at the start of the function instead of
> resetting it to zero.

Does this patch address your concerns?
https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/leon-for-next&id=f65ababb556ae0110d14294747dd6df5dcb8f597

Thanks

> 
> Bart.

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

[Leon Romanovsky]

On Thu, Jul 16, 2026 at 09:22:00PM +0300, Leon Romanovsky wrote:
> On Thu, Jul 16, 2026 at 10:37:39AM -0700, Bart Van Assche wrote:
> > On 7/15/26 3:15 AM, 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 clear rw_ctxs after freeing the heap
> > > allocation before returning an error.
> > > 
> > > Fixes: b99f8e4d7bcd ("IB/srpt: convert to the generic RDMA READ/WRITE API")
> > > Signed-off-by: TanZheng <tanzheng@kylinos.cn>
> > > ---
> > > v2:
> > > - After kfree(), set rw_ctxs to NULL instead of &s_rw_ctx
> > >    (Leon Romanovsky)
> > > 
> > >   drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++++-
> > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > > index f66cfd70c263..a9c4995af7a3 100644
> > > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> > > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > > @@ -1014,8 +1014,12 @@ static int srpt_alloc_rw_ctxs(struct srpt_send_ioctx *ioctx,
> > >   				ctx->sg, ctx->nents, dir);
> > >   		target_free_sgl(ctx->sg, ctx->nents);
> > >   	}
> > > -	if (ioctx->rw_ctxs != &ioctx->s_rw_ctx)
> > > +	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;
> > >   	return ret;
> > >   }
> > 
> > The above looks wrong to me. In the error path ioctx->n_rw_ctx should be
> > restored to the value it had at the start of the function instead of
> > resetting it to zero.
> 
> Does this patch address your concerns?
> https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/leon-for-next&id=f65ababb556ae0110d14294747dd6df5dcb8f597

I'm sorry, this one
https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/leon-for-next&id=07f5f0308bef

Thanks

> 
> Thanks
> 
> > 
> > Bart.
> 

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

[Bart Van Assche]

On 7/16/26 11:24 AM, Leon Romanovsky wrote:
> I'm sorry, this one
> https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/leon-for-next&id=07f5f0308bef

Looks good to me. Thanks Leon!

Bart.

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

[kensanya]

At 2026-07-17 01:37:39, "Bart Van Assche" <bvanassche@acm.org> wrote:
>On 7/15/26 3:15 AM, 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 clear rw_ctxs after freeing the heap
>> allocation before returning an error.
>> 
>> Fixes: b99f8e4d7bcd ("IB/srpt: convert to the generic RDMA READ/WRITE API")
>> Signed-off-by: TanZheng <tanzheng@kylinos.cn>
>> ---
>> v2:
>> - After kfree(), set rw_ctxs to NULL instead of &s_rw_ctx
>>    (Leon Romanovsky)
>> 
>>   drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
>> index f66cfd70c263..a9c4995af7a3 100644
>> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
>> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
>> @@ -1014,8 +1014,12 @@ static int srpt_alloc_rw_ctxs(struct srpt_send_ioctx *ioctx,
>>   				ctx->sg, ctx->nents, dir);
>>   		target_free_sgl(ctx->sg, ctx->nents);
>>   	}
>> -	if (ioctx->rw_ctxs != &ioctx->s_rw_ctx)
>> +	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;
>>   	return ret;
>>   }
>
>The above looks wrong to me. In the error path ioctx->n_rw_ctx should be
>restored to the value it had at the start of the function instead of
>resetting it to zero.
>
>Bart.

Hi Bart,

I have a question about restoring n_rw_ctx/n_rdma from local
snapshots versus clearing them to 0 on the unwind path.

Looking at the call chain:
  srpt_handle_new_iu()
    -> srpt_get_send_ioctx()   /* sets n_rdma = 0, n_rw_ctx = 0 */
    -> srpt_get_desc_tbl()
         -> srpt_alloc_rw_ctxs()

so when srpt_alloc_rw_ctxs() is entered, both counters are already
0.  On the current call path, assigning 0 on unwind seems
equivalent to restoring the values saved at function entry.

Is the save/restore preferred because the loop starts from
ioctx->n_rw_ctx (i.e. the function is written as if it may extend
an existing allocation), or is there another reason to prefer it
over clearing to 0?

Thanks,
TanZheng