[PATCH] RDMA/rtrs: server: Fix error handling in get_or_create_srv

[PATCH] RDMA/rtrs: server: Fix error handling in get_or_create_srv

[Ma Ke]

get_or_create_srv() fails to call put_device() after
device_initialize() when memory allocation fails. This could cause
reference count leaks during error handling, preventing proper device
cleanup and resulting in memory leaks.

Found by code review.

Cc: stable@vger.kernel.org
Fixes: 9cb837480424 ("RDMA/rtrs: server: main functionality")
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index ef4abdea3c2d..9ecc6343455d 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1450,7 +1450,7 @@ static struct rtrs_srv_sess *get_or_create_srv(struct rtrs_srv_ctx *ctx,
 	kfree(srv->chunks);
 
 err_free_srv:
-	kfree(srv);
+	put_device(&srv->dev);
 	return ERR_PTR(-ENOMEM);
 }
 
-- 
2.17.1

Re: [PATCH] RDMA/rtrs: server: Fix error handling in get_or_create_srv

[Jinpu Wang]

On Tue, Nov 4, 2025 at 3:19 AM Ma Ke <make24@iscas.ac.cn> wrote:
>
> get_or_create_srv() fails to call put_device() after
> device_initialize() when memory allocation fails. This could cause
> reference count leaks during error handling, preventing proper device
> cleanup and resulting in memory leaks.
>
> Found by code review.
>
> Cc: stable@vger.kernel.org
> Fixes: 9cb837480424 ("RDMA/rtrs: server: main functionality")
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
lgtm, thx!
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index ef4abdea3c2d..9ecc6343455d 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -1450,7 +1450,7 @@ static struct rtrs_srv_sess *get_or_create_srv(struct rtrs_srv_ctx *ctx,
>         kfree(srv->chunks);
>
>  err_free_srv:
> -       kfree(srv);
> +       put_device(&srv->dev);
>         return ERR_PTR(-ENOMEM);
>  }
>
> --
> 2.17.1
>

Re: [PATCH] RDMA/rtrs: server: Fix error handling in get_or_create_srv

[Markus Elfring]

> get_or_create_srv() fails to call put_device() after
> device_initialize() when memory allocation fails. …

Why do you propose then to replace a kfree(srv) call by put_device(&srv->dev)?


Would an other word wrapping be a bit nicer for such a change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc4#n658

Regards,
Markus

Re: [PATCH] RDMA/rtrs: server: Fix error handling in get_or_create_srv

[Leon Romanovsky]

On Tue, Nov 04, 2025 at 10:19:00AM +0800, Ma Ke wrote:
> get_or_create_srv() fails to call put_device() after
> device_initialize() when memory allocation fails. This could cause
> reference count leaks during error handling, preventing proper device
> cleanup and resulting in memory leaks.

Nothing from above is true. put_device is preferable way to release
memory after call to device_initialize(), but direct call to kfree is
also fine.

> 
> Found by code review.
> 
> Cc: stable@vger.kernel.org

There is no need in this line at all, it is not fixing anything.

Please rewrite commit message, thanks.

> Fixes: 9cb837480424 ("RDMA/rtrs: server: main functionality")
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index ef4abdea3c2d..9ecc6343455d 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -1450,7 +1450,7 @@ static struct rtrs_srv_sess *get_or_create_srv(struct rtrs_srv_ctx *ctx,
>  	kfree(srv->chunks);
>  
>  err_free_srv:
> -	kfree(srv);
> +	put_device(&srv->dev);
>  	return ERR_PTR(-ENOMEM);
>  }
>  
> -- 
> 2.17.1
> 

Re: [PATCH] RDMA/rtrs: server: Fix error handling in get_or_create_srv

[Jason Gunthorpe]

On Wed, Nov 05, 2025 at 02:57:13PM +0200, Leon Romanovsky wrote:
> On Tue, Nov 04, 2025 at 10:19:00AM +0800, Ma Ke wrote:
> > get_or_create_srv() fails to call put_device() after
> > device_initialize() when memory allocation fails. This could cause
> > reference count leaks during error handling, preventing proper device
> > cleanup and resulting in memory leaks.
> 
> Nothing from above is true. put_device is preferable way to release
> memory after call to device_initialize(), but direct call to kfree is
> also fine.

Once device_initialize() happens you must call put_device(), it is one
of Greg's rules.

Jason

Re: [PATCH] RDMA/rtrs: server: Fix error handling in get_or_create_srv

[Leon Romanovsky]

On Wed, Nov 05, 2025 at 09:46:59AM -0400, Jason Gunthorpe wrote:
> On Wed, Nov 05, 2025 at 02:57:13PM +0200, Leon Romanovsky wrote:
> > On Tue, Nov 04, 2025 at 10:19:00AM +0800, Ma Ke wrote:
> > > get_or_create_srv() fails to call put_device() after
> > > device_initialize() when memory allocation fails. This could cause
> > > reference count leaks during error handling, preventing proper device
> > > cleanup and resulting in memory leaks.
> > 
> > Nothing from above is true. put_device is preferable way to release
> > memory after call to device_initialize(), but direct call to kfree is
> > also fine.
> 
> Once device_initialize() happens you must call put_device(), it is one
> of Greg's rules.

According to the documentation it is not must, but is very good to have.

This sentence from above commit message is wrong:
"This could cause reference count leaks during error handling, preventing proper device
cleanup and resulting in memory leaks."

It won't cause to reference count leaks and doesn't have memory leaks in
this flow.

Thanks

> 
> Jason