| Message ID | 20230103171811.204196-2-caleb.connolly@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] thermal/core: fix unbalanced put_device in register error path | expand |
On 2023/1/4 1:18, Caleb Connolly wrote: > Commit 4748f9687caa ("thermal: core: fix some possible name leaks in error paths") > causes thermal_cooling_device_destroy_sysfs() to be called incorrectly > in __thermal_cooling_device_register() if ->get_max_state() fails. It > also causes it to get called twice if dev_set_name() fails. > > Adjust the error paths to ensure thermal_cooling_device_destroy_sysfs() > is only called for a matching thermal_cooling_device_setup_sysfs(). > > Fixes: 4748f9687caa ("thermal: core: fix some possible name leaks in error paths") > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > drivers/thermal/thermal_core.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index 2c6995b5dcb0..28864f14b01c 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -914,10 +914,9 @@ __thermal_cooling_device_register(struct device_node *np, > > thermal_cooling_device_setup_sysfs(cdev); > ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id); > - if (ret) { > - thermal_cooling_device_destroy_sysfs(cdev); > - goto out_kfree_type; > - } > + if (ret) > + goto out_destroy_sysfs; > + > ret = device_register(&cdev->device); > if (ret) > goto out_put_device; > @@ -941,8 +940,9 @@ __thermal_cooling_device_register(struct device_node *np, > > out_put_device: > put_device(&cdev->device); > -out_kfree_type: > +out_destroy_sysfs: > thermal_cooling_device_destroy_sysfs(cdev); The 'cdev' is freed in thermal_release() while calling put_device(), it causes UAF here. Thanks, Yang > +out_kfree_type: > kfree(cdev->type); > cdev = NULL; > out_ida_remove:
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 2c6995b5dcb0..28864f14b01c 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -914,10 +914,9 @@ __thermal_cooling_device_register(struct device_node *np, thermal_cooling_device_setup_sysfs(cdev); ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id); - if (ret) { - thermal_cooling_device_destroy_sysfs(cdev); - goto out_kfree_type; - } + if (ret) + goto out_destroy_sysfs; + ret = device_register(&cdev->device); if (ret) goto out_put_device; @@ -941,8 +940,9 @@ __thermal_cooling_device_register(struct device_node *np, out_put_device: put_device(&cdev->device); -out_kfree_type: +out_destroy_sysfs: thermal_cooling_device_destroy_sysfs(cdev); +out_kfree_type: kfree(cdev->type); cdev = NULL; out_ida_remove:
Commit 4748f9687caa ("thermal: core: fix some possible name leaks in error paths") causes thermal_cooling_device_destroy_sysfs() to be called incorrectly in __thermal_cooling_device_register() if ->get_max_state() fails. It also causes it to get called twice if dev_set_name() fails. Adjust the error paths to ensure thermal_cooling_device_destroy_sysfs() is only called for a matching thermal_cooling_device_setup_sysfs(). Fixes: 4748f9687caa ("thermal: core: fix some possible name leaks in error paths") Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> --- drivers/thermal/thermal_core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)