[PATCH v2] RDMA/rtrs-clt: Fix possible double free in error case

[PATCH v2] RDMA/rtrs-clt: Fix possible double free in error case

[Md Haris Iqbal]

Callback function rtrs_clt_dev_release() for put_device()
calls kfree(clt) to free memory. We shouldn't call kfree(clt) again,
and we can't use the clt after kfree too.

free clt->pcpu_path and clt explicitly when dev_set_name fails.
For other errors after that, let the release function take care of
freeing them. Also remove free_percpu(clt->pcpu_path) from free_clt()

Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
Reported-by: Miaoqian Lin <linmq006@gmail.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Reviewed-by: Gioh Kim <gi-oh.kim@ionos.com>
Reviewed-by: Santosh Kumar Pradhan <santosh.pradhan@ionos.com>
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
---
Changes since v1:
 - Explicitly free clt->pcpu_path and clt when dev_set_name fails
 - Remove free_percpu(clt->pcpu_path) from free_clt function

---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 7c3f98e57889..b8fc6ee4bb7f 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2682,6 +2682,7 @@ static void rtrs_clt_dev_release(struct device *dev)
 	struct rtrs_clt_sess *clt = container_of(dev, struct rtrs_clt_sess,
 						 dev);
 
+	free_percpu(clt->pcpu_path);
 	kfree(clt);
 }
 
@@ -2731,8 +2732,11 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
 	clt->dev.class = rtrs_clt_dev_class;
 	clt->dev.release = rtrs_clt_dev_release;
 	err = dev_set_name(&clt->dev, "%s", sessname);
-	if (err)
+	if (err) {
+		free_percpu(clt->pcpu_path);
+		kfree(clt);
 		goto err;
+	}
 	/*
 	 * Suppress user space notification until
 	 * sysfs files are created
@@ -2762,18 +2766,17 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
 err_dev:
 	device_unregister(&clt->dev);
 err:
-	free_percpu(clt->pcpu_path);
-	kfree(clt);
 	return ERR_PTR(err);
 }
 
 static void free_clt(struct rtrs_clt_sess *clt)
 {
 	free_permits(clt);
-	free_percpu(clt->pcpu_path);
 	mutex_destroy(&clt->paths_ev_mutex);
 	mutex_destroy(&clt->paths_mutex);
-	/* release callback will free clt in last put */
+	/*
+	 * release callback will free clt->pcpu_path and clt in last put
+	 */
 	device_unregister(&clt->dev);
 }
 
-- 
2.25.1

Re: [PATCH v2] RDMA/rtrs-clt: Fix possible double free in error case

[Leon Romanovsky]

On Thu, Jan 20, 2022 at 06:36:32PM +0100, Md Haris Iqbal wrote:
> Callback function rtrs_clt_dev_release() for put_device()
> calls kfree(clt) to free memory. We shouldn't call kfree(clt) again,
> and we can't use the clt after kfree too.
> 
> free clt->pcpu_path and clt explicitly when dev_set_name fails.
> For other errors after that, let the release function take care of
> freeing them. Also remove free_percpu(clt->pcpu_path) from free_clt()
> 
> Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
> Reported-by: Miaoqian Lin <linmq006@gmail.com>
> Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> Reviewed-by: Gioh Kim <gi-oh.kim@ionos.com>
> Reviewed-by: Santosh Kumar Pradhan <santosh.pradhan@ionos.com>
> Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> Changes since v1:
>  - Explicitly free clt->pcpu_path and clt when dev_set_name fails
>  - Remove free_percpu(clt->pcpu_path) from free_clt function
> 
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>