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 <william.xuanziyang@huawei.com> --- 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 <william.xuanziyang@huawei.com> 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 <william.xuanziyang@huawei.com> > --- > 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); > --
[-- Attachment #1: Type: text/plain, Size: 5556 bytes --] 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 <lkp@intel.com> 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/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 31512 bytes --]
[-- Attachment #1: Type: text/plain, Size: 10222 bytes --] 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 <lkp@intel.com> 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/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 39784 bytes --]