Hi Mike, Here is the buggy line: 861 err_out: 862 cti_pm_release(drvdata); ^^^^^^^

To me it's a red flag any time there is a label called "out:". The label should say what is being released like "goto unregister_notifier;". The style of error handling here is called a "free everything function" and it is the most error prone style of error handling. A better way to write error handling is to track the most recently allocated resource and free it with a well named goto. a = alloc(); if (!a) return -ENOMEM; b = alloc(); if (!b) { ret = -ENOMEM; goto free_a; } c = alloc(); if (!c) { ret = -ENOMEM; goto free_b; ... return 0; free_b: free(b); free_a: free(a); The advantage of this is that 1) You only have to track the most recent allocation. 2) You can easily verify that the most recent allocation is freed. 3) Now you can create a free function by copy and pasting and adding a free(c); void my_free(struct whatever *p) { free(c); free(b); free(a); } This style uses about the same number of lines of code because although we duplicate the free(b) and free(a), we can remove some if statements so it ends up being about the same. The main problem with free everything function is that they free things which have not been allocated. I have added more explanation at the bottom of this bug report. I am also sending a patch which hopefully is clear but I can't actually compile it. :( tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: b791d1bdf9212d944d749a5c7ff6febdba241771 commit: e9b880581d555c8f7b58c7d19cc3f8f9016a1b5f coresight: cti: Add CPU Hotplug handling to CTI driver config: arm-randconfig-m031-20200612 (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp(a)intel.com&gt; Reported-by: Dan Carpenter <dan.carpenter(a)oracle.com&gt; smatch warnings: drivers/hwtracing/coresight/coresight-cti.c:862 cti_probe() error: we previously assumed 'drvdata' could be null (see line 759) # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit... git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git remote update linus git checkout e9b880581d555c8f7b58c7d19cc3f8f9016a1b5f vim +/drvdata +862 drivers/hwtracing/coresight/coresight-cti.c 835d722ba10ac92 Mike Leach 2020-03-20 747 static int cti_probe(struct amba_device *adev, const struct amba_id *id) 835d722ba10ac92 Mike Leach 2020-03-20 748 { 835d722ba10ac92 Mike Leach 2020-03-20 749 int ret = 0; 835d722ba10ac92 Mike Leach 2020-03-20 750 void __iomem *base; 835d722ba10ac92 Mike Leach 2020-03-20 751 struct device *dev = &adev->dev; 835d722ba10ac92 Mike Leach 2020-03-20 752 struct cti_drvdata *drvdata = NULL; 835d722ba10ac92 Mike Leach 2020-03-20 753 struct coresight_desc cti_desc; 835d722ba10ac92 Mike Leach 2020-03-20 754 struct coresight_platform_data *pdata = NULL; 835d722ba10ac92 Mike Leach 2020-03-20 755 struct resource *res = &adev->res; 835d722ba10ac92 Mike Leach 2020-03-20 756 835d722ba10ac92 Mike Leach 2020-03-20 757 /* driver data*/ 835d722ba10ac92 Mike Leach 2020-03-20 758 drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); 835d722ba10ac92 Mike Leach 2020-03-20 @759 if (!drvdata) { 835d722ba10ac92 Mike Leach 2020-03-20 760 ret = -ENOMEM; 835d722ba10ac92 Mike Leach 2020-03-20 761 dev_info(dev, "%s, mem err\n", __func__); No need to print an error message for kmalloc() failures. It already has a stack trace built in. 835d722ba10ac92 Mike Leach 2020-03-20 762 goto err_out; ^^^^^^^^^^^^ 835d722ba10ac92 Mike Leach 2020-03-20 763 } 835d722ba10ac92 Mike Leach 2020-03-20 764 835d722ba10ac92 Mike Leach 2020-03-20 765 /* Validity for the resource is already checked by the AMBA core */ 835d722ba10ac92 Mike Leach 2020-03-20 766 base = devm_ioremap_resource(dev, res); 835d722ba10ac92 Mike Leach 2020-03-20 767 if (IS_ERR(base)) { 835d722ba10ac92 Mike Leach 2020-03-20 768 ret = PTR_ERR(base); 835d722ba10ac92 Mike Leach 2020-03-20 769 dev_err(dev, "%s, remap err\n", __func__); At this point "drvdata->ctidev.cpu" is zero. 835d722ba10ac92 Mike Leach 2020-03-20 770 goto err_out; 835d722ba10ac92 Mike Leach 2020-03-20 771 } 835d722ba10ac92 Mike Leach 2020-03-20 772 drvdata->base = base; 835d722ba10ac92 Mike Leach 2020-03-20 773 835d722ba10ac92 Mike Leach 2020-03-20 774 dev_set_drvdata(dev, drvdata); 835d722ba10ac92 Mike Leach 2020-03-20 775 835d722ba10ac92 Mike Leach 2020-03-20 776 /* default CTI device info */ 835d722ba10ac92 Mike Leach 2020-03-20 777 drvdata->ctidev.cpu = -1; ^^^^^^^^^^^^^^^^^^^^^^^^ 835d722ba10ac92 Mike Leach 2020-03-20 778 drvdata->ctidev.nr_trig_con = 0; 835d722ba10ac92 Mike Leach 2020-03-20 779 drvdata->ctidev.ctm_id = 0; 835d722ba10ac92 Mike Leach 2020-03-20 780 INIT_LIST_HEAD(&drvdata->ctidev.trig_cons); 835d722ba10ac92 Mike Leach 2020-03-20 781 835d722ba10ac92 Mike Leach 2020-03-20 782 spin_lock_init(&drvdata->spinlock); 835d722ba10ac92 Mike Leach 2020-03-20 783 835d722ba10ac92 Mike Leach 2020-03-20 784 /* initialise CTI driver config values */ 835d722ba10ac92 Mike Leach 2020-03-20 785 cti_set_default_config(dev, drvdata); 835d722ba10ac92 Mike Leach 2020-03-20 786 835d722ba10ac92 Mike Leach 2020-03-20 787 pdata = coresight_cti_get_platform_data(dev); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This function sets drvdata->ctidev.cpu on some success paths and also on certain failure paths. 835d722ba10ac92 Mike Leach 2020-03-20 788 if (IS_ERR(pdata)) { 835d722ba10ac92 Mike Leach 2020-03-20 789 dev_err(dev, "coresight_cti_get_platform_data err\n"); 835d722ba10ac92 Mike Leach 2020-03-20 790 ret = PTR_ERR(pdata); 835d722ba10ac92 Mike Leach 2020-03-20 791 goto err_out; 835d722ba10ac92 Mike Leach 2020-03-20 792 } 835d722ba10ac92 Mike Leach 2020-03-20 793 835d722ba10ac92 Mike Leach 2020-03-20 794 /* default to powered - could change on PM notifications */ 835d722ba10ac92 Mike Leach 2020-03-20 795 drvdata->config.hw_powered = true; 835d722ba10ac92 Mike Leach 2020-03-20 796 835d722ba10ac92 Mike Leach 2020-03-20 797 /* set up device name - will depend if cpu bound or otherwise */ 835d722ba10ac92 Mike Leach 2020-03-20 798 if (drvdata->ctidev.cpu >= 0) 835d722ba10ac92 Mike Leach 2020-03-20 799 cti_desc.name = devm_kasprintf(dev, GFP_KERNEL, "cti_cpu%d", 835d722ba10ac92 Mike Leach 2020-03-20 800 drvdata->ctidev.cpu); 835d722ba10ac92 Mike Leach 2020-03-20 801 else 835d722ba10ac92 Mike Leach 2020-03-20 802 cti_desc.name = coresight_alloc_device_name(&cti_sys_devs, dev); 835d722ba10ac92 Mike Leach 2020-03-20 803 if (!cti_desc.name) { 835d722ba10ac92 Mike Leach 2020-03-20 804 ret = -ENOMEM; 835d722ba10ac92 Mike Leach 2020-03-20 805 goto err_out; 835d722ba10ac92 Mike Leach 2020-03-20 806 } 835d722ba10ac92 Mike Leach 2020-03-20 807 e9b880581d555c8 Mike Leach 2020-05-18 808 /* setup CPU power management handling for CPU bound CTI devices. */ e9b880581d555c8 Mike Leach 2020-05-18 809 if (drvdata->ctidev.cpu >= 0) { e9b880581d555c8 Mike Leach 2020-05-18 810 cti_cpu_drvdata[drvdata->ctidev.cpu] = drvdata; e9b880581d555c8 Mike Leach 2020-05-18 811 if (!nr_cti_cpu++) { ^^^^^^^^^^^^ e9b880581d555c8 Mike Leach 2020-05-18 812 cpus_read_lock(); e9b880581d555c8 Mike Leach 2020-05-18 813 ret = cpuhp_setup_state_nocalls_cpuslocked( e9b880581d555c8 Mike Leach 2020-05-18 814 CPUHP_AP_ARM_CORESIGHT_CTI_STARTING, e9b880581d555c8 Mike Leach 2020-05-18 815 "arm/coresight_cti:starting", e9b880581d555c8 Mike Leach 2020-05-18 816 cti_starting_cpu, cti_dying_cpu); e9b880581d555c8 Mike Leach 2020-05-18 817 e9b880581d555c8 Mike Leach 2020-05-18 818 cpus_read_unlock(); e9b880581d555c8 Mike Leach 2020-05-18 819 if (ret) e9b880581d555c8 Mike Leach 2020-05-18 820 goto err_out; e9b880581d555c8 Mike Leach 2020-05-18 821 } e9b880581d555c8 Mike Leach 2020-05-18 822 } e9b880581d555c8 Mike Leach 2020-05-18 823 3c5597e398124e6 Mike Leach 2020-03-20 824 /* create dynamic attributes for connections */ 3c5597e398124e6 Mike Leach 2020-03-20 825 ret = cti_create_cons_sysfs(dev, drvdata); 3c5597e398124e6 Mike Leach 2020-03-20 826 if (ret) { 3c5597e398124e6 Mike Leach 2020-03-20 827 dev_err(dev, "%s: create dynamic sysfs entries failed\n", 3c5597e398124e6 Mike Leach 2020-03-20 828 cti_desc.name); 3c5597e398124e6 Mike Leach 2020-03-20 829 goto err_out; 3c5597e398124e6 Mike Leach 2020-03-20 830 } 3c5597e398124e6 Mike Leach 2020-03-20 831 835d722ba10ac92 Mike Leach 2020-03-20 832 /* set up coresight component description */ 835d722ba10ac92 Mike Leach 2020-03-20 833 cti_desc.pdata = pdata; 835d722ba10ac92 Mike Leach 2020-03-20 834 cti_desc.type = CORESIGHT_DEV_TYPE_ECT; 835d722ba10ac92 Mike Leach 2020-03-20 835 cti_desc.subtype.ect_subtype = CORESIGHT_DEV_SUBTYPE_ECT_CTI; 835d722ba10ac92 Mike Leach 2020-03-20 836 cti_desc.ops = &cti_ops; 3c5597e398124e6 Mike Leach 2020-03-20 837 cti_desc.groups = drvdata->ctidev.con_groups; 835d722ba10ac92 Mike Leach 2020-03-20 838 cti_desc.dev = dev; 835d722ba10ac92 Mike Leach 2020-03-20 839 drvdata->csdev = coresight_register(&cti_desc); 835d722ba10ac92 Mike Leach 2020-03-20 840 if (IS_ERR(drvdata->csdev)) { 835d722ba10ac92 Mike Leach 2020-03-20 841 ret = PTR_ERR(drvdata->csdev); 835d722ba10ac92 Mike Leach 2020-03-20 842 goto err_out; 835d722ba10ac92 Mike Leach 2020-03-20 843 } 835d722ba10ac92 Mike Leach 2020-03-20 844 835d722ba10ac92 Mike Leach 2020-03-20 845 /* add to list of CTI devices */ 835d722ba10ac92 Mike Leach 2020-03-20 846 mutex_lock(&ect_mutex); 835d722ba10ac92 Mike Leach 2020-03-20 847 list_add(&drvdata->node, &ect_net); 177af8285b59a38 Mike Leach 2020-03-20 848 /* set any cross references */ 177af8285b59a38 Mike Leach 2020-03-20 849 cti_update_conn_xrefs(drvdata); 835d722ba10ac92 Mike Leach 2020-03-20 850 mutex_unlock(&ect_mutex); 835d722ba10ac92 Mike Leach 2020-03-20 851 835d722ba10ac92 Mike Leach 2020-03-20 852 /* set up release chain */ 835d722ba10ac92 Mike Leach 2020-03-20 853 drvdata->csdev_release = drvdata->csdev->dev.release; 835d722ba10ac92 Mike Leach 2020-03-20 854 drvdata->csdev->dev.release = cti_device_release; 835d722ba10ac92 Mike Leach 2020-03-20 855 835d722ba10ac92 Mike Leach 2020-03-20 856 /* all done - dec pm refcount */ 835d722ba10ac92 Mike Leach 2020-03-20 857 pm_runtime_put(&adev->dev); 835d722ba10ac92 Mike Leach 2020-03-20 858 dev_info(&drvdata->csdev->dev, "CTI initialized\n"); 835d722ba10ac92 Mike Leach 2020-03-20 859 return 0; 835d722ba10ac92 Mike Leach 2020-03-20 860 835d722ba10ac92 Mike Leach 2020-03-20 861 err_out: e9b880581d555c8 Mike Leach 2020-05-18 @862 cti_pm_release(drvdata); ^^^^^^^ 835d722ba10ac92 Mike Leach 2020-03-20 863 return ret; 835d722ba10ac92 Mike Leach 2020-03-20 864 } 750 /* release PM registrations */ 751 static void cti_pm_release(struct cti_drvdata *drvdata) 752 { 753 if (drvdata->ctidev.cpu >= 0) { ^^^^^^^ We are dereferencing this when it wasn't allocated. 754 if (--nr_cti_cpu == 0) { ^^^^^^^^^^^^ If devm_kasprintf() fails then we are decrementing this when it wasn't incremented so now it can be negative. 755 cpu_pm_unregister_notifier(&cti_cpu_pm_nb); If the cpu_pm_register_notifier() fails then we are unregistering this when it wasn't registered. It turns out this is harmless but if we only free things which have been allocated then it becomes a lot easier to audit the code. 756 757 cpuhp_remove_state_nocalls( 758 CPUHP_AP_ARM_CORESIGHT_CTI_STARTING); If cpuhp_setup_state_nocalls_cpuslocked() failed then this wasn't allocated. I believe this is harmless. 759 } 760 cti_cpu_drvdata[drvdata->ctidev.cpu] = NULL; 761 } 762 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org _______________________________________________ kbuild mailing list -- kbuild(a)lists.01.org To unsubscribe send an email to kbuild-leave(a)lists.01.org