When device_register() return failed, program will goto out_kfree_type
to release 'cdev->device' by put_device(). That will call thermal_release()
to free 'cdev'. But the follow-up processes access 'cdev' continually.
That trggers the UAF bug.
====================================================================
BUG: KASAN: use-after-free in __thermal_cooling_device_register+0x75b/0xa90
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
Call Trace:
dump_stack_lvl+0xe2/0x152
print_address_description.constprop.0+0x21/0x140
? __thermal_cooling_device_register+0x75b/0xa90
kasan_report.cold+0x7f/0x11b
? __thermal_cooling_device_register+0x75b/0xa90
__thermal_cooling_device_register+0x75b/0xa90
? memset+0x20/0x40
? __sanitizer_cov_trace_pc+0x1d/0x50
? __devres_alloc_node+0x130/0x180
devm_thermal_of_cooling_device_register+0x67/0xf0
max6650_probe.cold+0x557/0x6aa
......
Freed by task 258:
kasan_save_stack+0x1b/0x40
kasan_set_track+0x1c/0x30
kasan_set_free_info+0x20/0x30
__kasan_slab_free+0x109/0x140
kfree+0x117/0x4c0
thermal_release+0xa0/0x110
device_release+0xa7/0x240
kobject_put+0x1ce/0x540
put_device+0x20/0x30
__thermal_cooling_device_register+0x731/0xa90
devm_thermal_of_cooling_device_register+0x67/0xf0
max6650_probe.cold+0x557/0x6aa [max6650]
Do not use 'cdev' again after put_device() to fix the problem like doing
in thermal_zone_device_register().
Fixes: 584837618100 ("thermal/drivers/core: Use a char pointer for the cooling device name")
Signed-off-by: Ziyang Xuan <[email protected]>
---
drivers/thermal/thermal_core.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 97ef9b040b84..ee7453944ccb 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -888,7 +888,7 @@ __thermal_cooling_device_register(struct device_node *np,
{
struct thermal_cooling_device *cdev;
struct thermal_zone_device *pos = NULL;
- int ret;
+ int id, ret;
if (!ops || !ops->get_max_state || !ops->get_cur_state ||
!ops->set_cur_state)
@@ -898,10 +898,10 @@ __thermal_cooling_device_register(struct device_node *np,
if (!cdev)
return ERR_PTR(-ENOMEM);
- ret = ida_simple_get(&thermal_cdev_ida, 0, 0, GFP_KERNEL);
- if (ret < 0)
+ id = ida_simple_get(&thermal_cdev_ida, 0, 0, GFP_KERNEL);
+ if (id < 0)
goto out_kfree_cdev;
- cdev->id = ret;
+ cdev->id = id;
cdev->type = kstrdup(type ? type : "", GFP_KERNEL);
if (!cdev->type) {
@@ -942,8 +942,9 @@ __thermal_cooling_device_register(struct device_node *np,
out_kfree_type:
kfree(cdev->type);
put_device(&cdev->device);
+ cdev = NULL;
out_ida_remove:
- ida_simple_remove(&thermal_cdev_ida, cdev->id);
+ ida_simple_remove(&thermal_cdev_ida, id);
out_kfree_cdev:
kfree(cdev);
return ERR_PTR(ret);
--
2.25.1
On Thu, Oct 14, 2021 at 10:47 AM Ziyang Xuan
<[email protected]> wrote:
>
> When device_register() return failed, program will goto out_kfree_type
> to release 'cdev->device' by put_device(). That will call thermal_release()
> to free 'cdev'. But the follow-up processes access 'cdev' continually.
> That trggers the UAF bug.
>
> ====================================================================
> BUG: KASAN: use-after-free in __thermal_cooling_device_register+0x75b/0xa90
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> Call Trace:
> dump_stack_lvl+0xe2/0x152
> print_address_description.constprop.0+0x21/0x140
> ? __thermal_cooling_device_register+0x75b/0xa90
> kasan_report.cold+0x7f/0x11b
> ? __thermal_cooling_device_register+0x75b/0xa90
> __thermal_cooling_device_register+0x75b/0xa90
> ? memset+0x20/0x40
> ? __sanitizer_cov_trace_pc+0x1d/0x50
> ? __devres_alloc_node+0x130/0x180
> devm_thermal_of_cooling_device_register+0x67/0xf0
> max6650_probe.cold+0x557/0x6aa
> ......
>
> Freed by task 258:
> kasan_save_stack+0x1b/0x40
> kasan_set_track+0x1c/0x30
> kasan_set_free_info+0x20/0x30
> __kasan_slab_free+0x109/0x140
> kfree+0x117/0x4c0
> thermal_release+0xa0/0x110
> device_release+0xa7/0x240
> kobject_put+0x1ce/0x540
> put_device+0x20/0x30
> __thermal_cooling_device_register+0x731/0xa90
> devm_thermal_of_cooling_device_register+0x67/0xf0
> max6650_probe.cold+0x557/0x6aa [max6650]
>
> Do not use 'cdev' again after put_device() to fix the problem like doing
> in thermal_zone_device_register().
>
> Fixes: 584837618100 ("thermal/drivers/core: Use a char pointer for the cooling device name")
> Signed-off-by: Ziyang Xuan <[email protected]>
> ---
> drivers/thermal/thermal_core.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 97ef9b040b84..ee7453944ccb 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -888,7 +888,7 @@ __thermal_cooling_device_register(struct device_node *np,
> {
> struct thermal_cooling_device *cdev;
> struct thermal_zone_device *pos = NULL;
> - int ret;
> + int id, ret;
>
> if (!ops || !ops->get_max_state || !ops->get_cur_state ||
> !ops->set_cur_state)
> @@ -898,10 +898,10 @@ __thermal_cooling_device_register(struct device_node *np,
> if (!cdev)
> return ERR_PTR(-ENOMEM);
>
> - ret = ida_simple_get(&thermal_cdev_ida, 0, 0, GFP_KERNEL);
> - if (ret < 0)
> + id = ida_simple_get(&thermal_cdev_ida, 0, 0, GFP_KERNEL);
> + if (id < 0)
ret = id;
is missing here.
> goto out_kfree_cdev;
However, IMO it would be better to leave the code as is above and do
id = ret;
here.
> - cdev->id = ret;
> + cdev->id = id;
>
> cdev->type = kstrdup(type ? type : "", GFP_KERNEL);
> if (!cdev->type) {
> @@ -942,8 +942,9 @@ __thermal_cooling_device_register(struct device_node *np,
> out_kfree_type:
> kfree(cdev->type);
> put_device(&cdev->device);
> + cdev = NULL;
> out_ida_remove:
> - ida_simple_remove(&thermal_cdev_ida, cdev->id);
> + ida_simple_remove(&thermal_cdev_ida, id);
> out_kfree_cdev:
> kfree(cdev);
> return ERR_PTR(ret);
> --
Hi Ziyang,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on rafael-pm/thermal]
[also build test WARNING on v5.15-rc5 next-20211013]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ziyang-Xuan/thermal-core-fix-a-UAF-bug-in-__thermal_cooling_device_register/20211014-164859
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal
config: hexagon-randconfig-r005-20211014 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6c76d0101193aa4eb891a6954ff047eda2f9cf71)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/fb39770678d4d898891ede9121c811844b5f2890
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ziyang-Xuan/thermal-core-fix-a-UAF-bug-in-__thermal_cooling_device_register/20211014-164859
git checkout fb39770678d4d898891ede9121c811844b5f2890
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/thermal/thermal_core.c:901:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (id < 0)
^~~~~~
drivers/thermal/thermal_core.c:949:17: note: uninitialized use occurs here
return ERR_PTR(ret);
^~~
drivers/thermal/thermal_core.c:901:2: note: remove the 'if' if its condition is always false
if (id < 0)
^~~~~~~~~~~
drivers/thermal/thermal_core.c:890:13: note: initialize the variable 'ret' to silence this warning
int id, ret;
^
= 0
1 warning generated.
vim +901 drivers/thermal/thermal_core.c
866
867 /**
868 * __thermal_cooling_device_register() - register a new thermal cooling device
869 * @np: a pointer to a device tree node.
870 * @type: the thermal cooling device type.
871 * @devdata: device private data.
872 * @ops: standard thermal cooling devices callbacks.
873 *
874 * This interface function adds a new thermal cooling device (fan/processor/...)
875 * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
876 * to all the thermal zone devices registered at the same time.
877 * It also gives the opportunity to link the cooling device to a device tree
878 * node, so that it can be bound to a thermal zone created out of device tree.
879 *
880 * Return: a pointer to the created struct thermal_cooling_device or an
881 * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
882 */
883 static struct thermal_cooling_device *
884 __thermal_cooling_device_register(struct device_node *np,
885 const char *type, void *devdata,
886 const struct thermal_cooling_device_ops *ops)
887 {
888 struct thermal_cooling_device *cdev;
889 struct thermal_zone_device *pos = NULL;
890 int id, ret;
891
892 if (!ops || !ops->get_max_state || !ops->get_cur_state ||
893 !ops->set_cur_state)
894 return ERR_PTR(-EINVAL);
895
896 cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
897 if (!cdev)
898 return ERR_PTR(-ENOMEM);
899
900 id = ida_simple_get(&thermal_cdev_ida, 0, 0, GFP_KERNEL);
> 901 if (id < 0)
902 goto out_kfree_cdev;
903 cdev->id = id;
904
905 cdev->type = kstrdup(type ? type : "", GFP_KERNEL);
906 if (!cdev->type) {
907 ret = -ENOMEM;
908 goto out_ida_remove;
909 }
910
911 mutex_init(&cdev->lock);
912 INIT_LIST_HEAD(&cdev->thermal_instances);
913 cdev->np = np;
914 cdev->ops = ops;
915 cdev->updated = false;
916 cdev->device.class = &thermal_class;
917 cdev->devdata = devdata;
918 thermal_cooling_device_setup_sysfs(cdev);
919 dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
920 ret = device_register(&cdev->device);
921 if (ret)
922 goto out_kfree_type;
923
924 /* Add 'this' new cdev to the global cdev list */
925 mutex_lock(&thermal_list_lock);
926 list_add(&cdev->node, &thermal_cdev_list);
927 mutex_unlock(&thermal_list_lock);
928
929 /* Update binding information for 'this' new cdev */
930 bind_cdev(cdev);
931
932 mutex_lock(&thermal_list_lock);
933 list_for_each_entry(pos, &thermal_tz_list, node)
934 if (atomic_cmpxchg(&pos->need_update, 1, 0))
935 thermal_zone_device_update(pos,
936 THERMAL_EVENT_UNSPECIFIED);
937 mutex_unlock(&thermal_list_lock);
938
939 return cdev;
940
941 out_kfree_type:
942 kfree(cdev->type);
943 put_device(&cdev->device);
944 cdev = NULL;
945 out_ida_remove:
946 ida_simple_remove(&thermal_cdev_ida, id);
947 out_kfree_cdev:
948 kfree(cdev);
949 return ERR_PTR(ret);
950 }
951
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi Ziyang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rafael-pm/thermal]
[also build test ERROR on v5.15-rc5 next-20211013]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ziyang-Xuan/thermal-core-fix-a-UAF-bug-in-__thermal_cooling_device_register/20211014-164859
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal
config: riscv-buildonly-randconfig-r001-20211014 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6c76d0101193aa4eb891a6954ff047eda2f9cf71)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/fb39770678d4d898891ede9121c811844b5f2890
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ziyang-Xuan/thermal-core-fix-a-UAF-bug-in-__thermal_cooling_device_register/20211014-164859
git checkout fb39770678d4d898891ede9121c811844b5f2890
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
In file included from drivers/thermal/thermal_core.c:22:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:464:31: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:477:61: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:36:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/thermal/thermal_core.c:22:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:490:61: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:34:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/thermal/thermal_core.c:22:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:501:33: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:1024:55: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
~~~~~~~~~~ ^
>> drivers/thermal/thermal_core.c:901:6: error: variable 'ret' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (id < 0)
^~~~~~
drivers/thermal/thermal_core.c:949:17: note: uninitialized use occurs here
return ERR_PTR(ret);
^~~
drivers/thermal/thermal_core.c:901:2: note: remove the 'if' if its condition is always false
if (id < 0)
^~~~~~~~~~~
drivers/thermal/thermal_core.c:890:13: note: initialize the variable 'ret' to silence this warning
int id, ret;
^
= 0
8 errors generated.
vim +901 drivers/thermal/thermal_core.c
866
867 /**
868 * __thermal_cooling_device_register() - register a new thermal cooling device
869 * @np: a pointer to a device tree node.
870 * @type: the thermal cooling device type.
871 * @devdata: device private data.
872 * @ops: standard thermal cooling devices callbacks.
873 *
874 * This interface function adds a new thermal cooling device (fan/processor/...)
875 * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
876 * to all the thermal zone devices registered at the same time.
877 * It also gives the opportunity to link the cooling device to a device tree
878 * node, so that it can be bound to a thermal zone created out of device tree.
879 *
880 * Return: a pointer to the created struct thermal_cooling_device or an
881 * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
882 */
883 static struct thermal_cooling_device *
884 __thermal_cooling_device_register(struct device_node *np,
885 const char *type, void *devdata,
886 const struct thermal_cooling_device_ops *ops)
887 {
888 struct thermal_cooling_device *cdev;
889 struct thermal_zone_device *pos = NULL;
890 int id, ret;
891
892 if (!ops || !ops->get_max_state || !ops->get_cur_state ||
893 !ops->set_cur_state)
894 return ERR_PTR(-EINVAL);
895
896 cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
897 if (!cdev)
898 return ERR_PTR(-ENOMEM);
899
900 id = ida_simple_get(&thermal_cdev_ida, 0, 0, GFP_KERNEL);
> 901 if (id < 0)
902 goto out_kfree_cdev;
903 cdev->id = id;
904
905 cdev->type = kstrdup(type ? type : "", GFP_KERNEL);
906 if (!cdev->type) {
907 ret = -ENOMEM;
908 goto out_ida_remove;
909 }
910
911 mutex_init(&cdev->lock);
912 INIT_LIST_HEAD(&cdev->thermal_instances);
913 cdev->np = np;
914 cdev->ops = ops;
915 cdev->updated = false;
916 cdev->device.class = &thermal_class;
917 cdev->devdata = devdata;
918 thermal_cooling_device_setup_sysfs(cdev);
919 dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
920 ret = device_register(&cdev->device);
921 if (ret)
922 goto out_kfree_type;
923
924 /* Add 'this' new cdev to the global cdev list */
925 mutex_lock(&thermal_list_lock);
926 list_add(&cdev->node, &thermal_cdev_list);
927 mutex_unlock(&thermal_list_lock);
928
929 /* Update binding information for 'this' new cdev */
930 bind_cdev(cdev);
931
932 mutex_lock(&thermal_list_lock);
933 list_for_each_entry(pos, &thermal_tz_list, node)
934 if (atomic_cmpxchg(&pos->need_update, 1, 0))
935 thermal_zone_device_update(pos,
936 THERMAL_EVENT_UNSPECIFIED);
937 mutex_unlock(&thermal_list_lock);
938
939 return cdev;
940
941 out_kfree_type:
942 kfree(cdev->type);
943 put_device(&cdev->device);
944 cdev = NULL;
945 out_ida_remove:
946 ida_simple_remove(&thermal_cdev_ida, id);
947 out_kfree_cdev:
948 kfree(cdev);
949 return ERR_PTR(ret);
950 }
951
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]