Vulnerability: drivers/infiniband/ulp/rtrs/rtrs-clt.c in the Linux kernel before 5.16.12 has a double free related to rtrs_clt_dev_release.
Vulnerability ID: CVE-2022-29156
CWE: CWE-415
Severity score: 10
Project: https://github.com/torvalds/linux
Published: 2022-04-13
File: drivers/infiniband/ulp/rtrs/rtrs-clt.c
Commit message: RDMA/rtrs-clt: Fix possible double free in error case

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.

Replace device_register() with device_initialize() and device_add() so that
dev_set_name can() be used appropriately.

Move mutex_destroy() to the release function so it can be called in
the alloc_clt err path.

Fixes: eab098246625 ("RDMA/rtrs-clt: Refactor the failure cases in alloc_clt")
Link: https://lore.kernel.org/r/20220217030929.323849-1-haris.iqbal@ionos.com
Reported-by: Miaoqian Lin <linmq006@gmail.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Commit author: Md Haris Iqbal

Vulnerable version (before fix):
 static void rtrs_clt_dev_release(struct device *dev) 	struct rtrs_clt_sess *clt = container_of(dev, struct rtrs_clt_sess, 						 dev);  	kfree(clt); }  static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, 		return ERR_PTR(-ENOMEM); 	}  	uuid_gen(&clt->paths_uuid); 	INIT_LIST_HEAD_RCU(&clt->paths_list); 	clt->paths_num = paths_num; static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, 	init_waitqueue_head(&clt->permits_wait); 	mutex_init(&clt->paths_ev_mutex); 	mutex_init(&clt->paths_mutex);  	clt->dev.class = rtrs_clt_dev_class; 	clt->dev.release = rtrs_clt_dev_release; 	err = dev_set_name(&clt->dev, "%s", sessname); 	if (err) 		goto err; 	/* 	 * Suppress user space notification until 	 * sysfs files are created 	 */ 	dev_set_uevent_suppress(&clt->dev, true); 	err = device_register(&clt->dev); 	if (err) { 		put_device(&clt->dev); 		goto err; 	}  	clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj); 	if (!clt->kobj_paths) { 		err = -ENOMEM; 		goto err_dev; 	} 	err = rtrs_clt_create_sysfs_root_files(clt); 	if (err) { 		kobject_del(clt->kobj_paths); 		kobject_put(clt->kobj_paths); 		goto err_dev; 	} 	dev_set_uevent_suppress(&clt->dev, false); 	kobject_uevent(&clt->dev.kobj, KOBJ_ADD);  	return clt; 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 */ 	device_unregister(&clt->dev); } 

Fixed version (after fix):
 static void rtrs_clt_dev_release(struct device *dev) 	struct rtrs_clt_sess *clt = container_of(dev, struct rtrs_clt_sess, 						 dev);  	mutex_destroy(&clt->paths_ev_mutex); 	mutex_destroy(&clt->paths_mutex); 	kfree(clt); }  static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, 		return ERR_PTR(-ENOMEM); 	}  	clt->dev.class = rtrs_clt_dev_class; 	clt->dev.release = rtrs_clt_dev_release; 	uuid_gen(&clt->paths_uuid); 	INIT_LIST_HEAD_RCU(&clt->paths_list); 	clt->paths_num = paths_num; static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, 	init_waitqueue_head(&clt->permits_wait); 	mutex_init(&clt->paths_ev_mutex); 	mutex_init(&clt->paths_mutex); 	device_initialize(&clt->dev);  	err = dev_set_name(&clt->dev, "%s", sessname); 	if (err) 		goto err_put;  	/* 	 * Suppress user space notification until 	 * sysfs files are created 	 */ 	dev_set_uevent_suppress(&clt->dev, true); 	err = device_add(&clt->dev); 	if (err) 		goto err_put;  	clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj); 	if (!clt->kobj_paths) { 		err = -ENOMEM; 		goto err_del; 	} 	err = rtrs_clt_create_sysfs_root_files(clt); 	if (err) { 		kobject_del(clt->kobj_paths); 		kobject_put(clt->kobj_paths); 		goto err_del; 	} 	dev_set_uevent_suppress(&clt->dev, false); 	kobject_uevent(&clt->dev.kobj, KOBJ_ADD);  	return clt; err_del: 	device_del(&clt->dev); err_put: 	free_percpu(clt->pcpu_path); 	put_device(&clt->dev); 	return ERR_PTR(err); }  static void free_clt(struct rtrs_clt_sess *clt) { 	free_permits(clt); 	free_percpu(clt->pcpu_path);  	/* 	 * release callback will free clt and destroy mutexes in last put 	 */ 	device_unregister(&clt->dev); } 

Patch:
patch: @@ -2682,6 +2682,8 @@ static void rtrs_clt_dev_release(struct device *dev)
 	struct rtrs_clt_sess *clt = container_of(dev, struct rtrs_clt_sess,
 						 dev);
 
+	mutex_destroy(&clt->paths_ev_mutex);
+	mutex_destroy(&clt->paths_mutex);
 	kfree(clt);
 }
 
@@ -2711,6 +2713,8 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	clt->dev.class = rtrs_clt_dev_class;
+	clt->dev.release = rtrs_clt_dev_release;
 	uuid_gen(&clt->paths_uuid);
 	INIT_LIST_HEAD_RCU(&clt->paths_list);
 	clt->paths_num = paths_num;
@@ -2727,53 +2731,52 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
 	init_waitqueue_head(&clt->permits_wait);
 	mutex_init(&clt->paths_ev_mutex);
 	mutex_init(&clt->paths_mutex);
+	device_initialize(&clt->dev);
 
-	clt->dev.class = rtrs_clt_dev_class;
-	clt->dev.release = rtrs_clt_dev_release;
 	err = dev_set_name(&clt->dev, "%s", sessname);
 	if (err)
-		goto err;
+		goto err_put;
+
 	/*
 	 * Suppress user space notification until
 	 * sysfs files are created
 	 */
 	dev_set_uevent_suppress(&clt->dev, true);
-	err = device_register(&clt->dev);
-	if (err) {
-		put_device(&clt->dev);
-		goto err;
-	}
+	err = device_add(&clt->dev);
+	if (err)
+		goto err_put;
 
 	clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj);
 	if (!clt->kobj_paths) {
 		err = -ENOMEM;
-		goto err_dev;
+		goto err_del;
 	}
 	err = rtrs_clt_create_sysfs_root_files(clt);
 	if (err) {
 		kobject_del(clt->kobj_paths);
 		kobject_put(clt->kobj_paths);
-		goto err_dev;
+		goto err_del;
 	}
 	dev_set_uevent_suppress(&clt->dev, false);
 	kobject_uevent(&clt->dev.kobj, KOBJ_ADD);
 
 	return clt;
-err_dev:
-	device_unregister(&clt->dev);
-err:
+err_del:
+	device_del(&clt->dev);
+err_put:
 	free_percpu(clt->pcpu_path);
-	kfree(clt);
+	put_device(&clt->dev);
 	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 and destroy mutexes in last put
+	 */
 	device_unregister(&clt->dev);
 }
path: drivers/infiniband/ulp/rtrs/rtrs-clt.c