diff for duplicates of <202201242029.Oiu4ZZAL-lkp@intel.com> diff --git a/a/1.txt b/N1/1.txt index e5e69d7..b8d9b48 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,22 +1,7 @@ -CC: kbuild-all(a)lists.01.org -In-Reply-To: <20220120103714.32108-1-linmq006@gmail.com> -References: <20220120103714.32108-1-linmq006@gmail.com> -TO: Miaoqian Lin <linmq006@gmail.com> - Hi Miaoqian, -Thank you for the patch! Perhaps something to improve: - -[auto build test WARNING on rdma/for-next] -[also build test WARNING on v5.17-rc1 next-20220124] -[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/Miaoqian-Lin/RDMA-rtrs-Fix-double-free-in-alloc_clt/20220120-183823 base: https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next -:::::: branch date: 4 days ago -:::::: commit date: 4 days ago config: x86_64-randconfig-m001-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242029.Oiu4ZZAL-lkp(a)intel.com/config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 @@ -29,7 +14,6 @@ drivers/infiniband/ulp/rtrs/rtrs-clt.c:2767 alloc_clt() error: dereferencing fre vim +/clt +2767 drivers/infiniband/ulp/rtrs/rtrs-clt.c -6a98d71daea186 Jack Wang 2020-05-11 2687 f3433d79cd50d3 Vaishali Thakkar 2022-01-05 2688 static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, 6a98d71daea186 Jack Wang 2020-05-11 2689 u16 port, size_t pdu_sz, void *priv, 6a98d71daea186 Jack Wang 2020-05-11 2690 void (*link_ev)(void *priv, @@ -78,6 +62,15 @@ f3433d79cd50d3 Vaishali Thakkar 2022-01-05 2695 struct rtrs_clt_sess *clt; 6a98d71daea186 Jack Wang 2020-05-11 2733 err = dev_set_name(&clt->dev, "%s", sessname); eab098246625e9 Guoqing Jiang 2020-12-17 2734 if (err) eab098246625e9 Guoqing Jiang 2020-12-17 2735 goto err; + +I suggest that the better way is: + + if (err) { + free_percpu(clt->pcpu_path); + kfree(clt); + return err; + } + 6a98d71daea186 Jack Wang 2020-05-11 2736 /* 6a98d71daea186 Jack Wang 2020-05-11 2737 * Suppress user space notification until 6a98d71daea186 Jack Wang 2020-05-11 2738 * sysfs files are created @@ -87,18 +80,39 @@ eab098246625e9 Guoqing Jiang 2020-12-17 2735 goto err; 6a98d71daea186 Jack Wang 2020-05-11 2742 if (err) { 6a98d71daea186 Jack Wang 2020-05-11 2743 put_device(&clt->dev); 7580d72134cc8f Miaoqian Lin 2022-01-20 2744 goto err_free_cpu; + +The right way to fix this is to add free_percpu(clt->pcpu_path); to +the rtrs_clt_dev_release() function. Then this becomes: + + if (err) + goto put_ctl; + 6a98d71daea186 Jack Wang 2020-05-11 2745 } 6a98d71daea186 Jack Wang 2020-05-11 2746 6a98d71daea186 Jack Wang 2020-05-11 2747 clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj); 6a98d71daea186 Jack Wang 2020-05-11 2748 if (!clt->kobj_paths) { eab098246625e9 Guoqing Jiang 2020-12-17 2749 err = -ENOMEM; eab098246625e9 Guoqing Jiang 2020-12-17 2750 goto err_dev; + +This is wrong. You cannot kfree(clt) after it device_register(). + 6a98d71daea186 Jack Wang 2020-05-11 2751 } 6a98d71daea186 Jack Wang 2020-05-11 2752 err = rtrs_clt_create_sysfs_root_files(clt); 6a98d71daea186 Jack Wang 2020-05-11 2753 if (err) { 6a98d71daea186 Jack Wang 2020-05-11 2754 kobject_del(clt->kobj_paths); 6a98d71daea186 Jack Wang 2020-05-11 2755 kobject_put(clt->kobj_paths); eab098246625e9 Guoqing Jiang 2020-12-17 2756 goto err_dev; + +The temptation is to add the following to rtrs_clt_dev_release(). + + if (clt->kobj_paths) { + kobject_del(clt->kobj_paths); + kobject_put(clt->kobj_paths); + } + +Does ordering matter? I kind of hate the register_device() stuff +because it creates bug prone and even unfixable code. + 6a98d71daea186 Jack Wang 2020-05-11 2757 } 6a98d71daea186 Jack Wang 2020-05-11 2758 dev_set_uevent_suppress(&clt->dev, false); 6a98d71daea186 Jack Wang 2020-05-11 2759 kobject_uevent(&clt->dev.kobj, KOBJ_ADD); @@ -106,15 +120,29 @@ eab098246625e9 Guoqing Jiang 2020-12-17 2756 goto err_dev; 6a98d71daea186 Jack Wang 2020-05-11 2761 return clt; eab098246625e9 Guoqing Jiang 2020-12-17 2762 err_dev: eab098246625e9 Guoqing Jiang 2020-12-17 2763 device_unregister(&clt->dev); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +This will call rtrs_clt_dev_release() and free clt. + eab098246625e9 Guoqing Jiang 2020-12-17 2764 err: eab098246625e9 Guoqing Jiang 2020-12-17 2765 free_percpu(clt->pcpu_path); + +Use after fre. + eab098246625e9 Guoqing Jiang 2020-12-17 2766 kfree(clt); + +Double free. + 7580d72134cc8f Miaoqian Lin 2022-01-20 @2767 clt->pcpu_path = NULL; + +Use after double free. + 7580d72134cc8f Miaoqian Lin 2022-01-20 2768 err_free_cpu: 7580d72134cc8f Miaoqian Lin 2022-01-20 2769 free_percpu(clt->pcpu_path); + +Another use after double free. + eab098246625e9 Guoqing Jiang 2020-12-17 2770 return ERR_PTR(err); 6a98d71daea186 Jack Wang 2020-05-11 2771 } -6a98d71daea186 Jack Wang 2020-05-11 2772 --- 0-DAY CI Kernel Test Service, Intel Corporation diff --git a/a/content_digest b/N1/content_digest index e44bf31..84d6694 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,28 +1,14 @@ - "From\0kernel test robot <lkp@intel.com>\0" + "ref\020220120103714.32108-1-linmq006@gmail.com\0" + "From\0Dan Carpenter <dan.carpenter@oracle.com>\0" "Subject\0Re: [PATCH] RDMA/rtrs: Fix double free in alloc_clt\0" - "Date\0Mon, 24 Jan 2022 21:08:17 +0800\0" - "To\0kbuild@lists.01.org\0" + "Date\0Wed, 26 Jan 2022 13:05:48 +0300\0" + "To\0kbuild-all@lists.01.org\0" "\01:1\0" "b\0" - "CC: kbuild-all(a)lists.01.org\n" - "In-Reply-To: <20220120103714.32108-1-linmq006@gmail.com>\n" - "References: <20220120103714.32108-1-linmq006@gmail.com>\n" - "TO: Miaoqian Lin <linmq006@gmail.com>\n" - "\n" "Hi Miaoqian,\n" "\n" - "Thank you for the patch! Perhaps something to improve:\n" - "\n" - "[auto build test WARNING on rdma/for-next]\n" - "[also build test WARNING on v5.17-rc1 next-20220124]\n" - "[If your patch is applied to the wrong git tree, kindly drop us a note.\n" - "And when submitting patch, we suggest to use '--base' as documented in\n" - "https://git-scm.com/docs/git-format-patch]\n" - "\n" "url: https://github.com/0day-ci/linux/commits/Miaoqian-Lin/RDMA-rtrs-Fix-double-free-in-alloc_clt/20220120-183823\n" "base: https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next\n" - ":::::: branch date: 4 days ago\n" - ":::::: commit date: 4 days ago\n" "config: x86_64-randconfig-m001-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242029.Oiu4ZZAL-lkp(a)intel.com/config)\n" "compiler: gcc-9 (Debian 9.3.0-22) 9.3.0\n" "\n" @@ -35,7 +21,6 @@ "\n" "vim +/clt +2767 drivers/infiniband/ulp/rtrs/rtrs-clt.c\n" "\n" - "6a98d71daea186 Jack Wang 2020-05-11 2687 \n" "f3433d79cd50d3 Vaishali Thakkar 2022-01-05 2688 static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,\n" "6a98d71daea186 Jack Wang 2020-05-11 2689 \t\t\t\t u16 port, size_t pdu_sz, void *priv,\n" "6a98d71daea186 Jack Wang 2020-05-11 2690 \t\t\t\t void\t(*link_ev)(void *priv,\n" @@ -84,6 +69,15 @@ "6a98d71daea186 Jack Wang 2020-05-11 2733 \terr = dev_set_name(&clt->dev, \"%s\", sessname);\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2734 \tif (err)\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2735 \t\tgoto err;\n" + "\n" + "I suggest that the better way is:\n" + "\n" + "\tif (err) {\n" + "\t\tfree_percpu(clt->pcpu_path);\n" + "\t\tkfree(clt);\n" + "\t\treturn err;\n" + "\t}\n" + "\n" "6a98d71daea186 Jack Wang 2020-05-11 2736 \t/*\n" "6a98d71daea186 Jack Wang 2020-05-11 2737 \t * Suppress user space notification until\n" "6a98d71daea186 Jack Wang 2020-05-11 2738 \t * sysfs files are created\n" @@ -93,18 +87,39 @@ "6a98d71daea186 Jack Wang 2020-05-11 2742 \tif (err) {\n" "6a98d71daea186 Jack Wang 2020-05-11 2743 \t\tput_device(&clt->dev);\n" "7580d72134cc8f Miaoqian Lin 2022-01-20 2744 \t\tgoto err_free_cpu;\n" + "\n" + "The right way to fix this is to add free_percpu(clt->pcpu_path); to\n" + "the rtrs_clt_dev_release() function. Then this becomes:\n" + "\n" + "\tif (err)\n" + "\t\tgoto put_ctl;\n" + "\n" "6a98d71daea186 Jack Wang 2020-05-11 2745 \t}\n" "6a98d71daea186 Jack Wang 2020-05-11 2746 \n" "6a98d71daea186 Jack Wang 2020-05-11 2747 \tclt->kobj_paths = kobject_create_and_add(\"paths\", &clt->dev.kobj);\n" "6a98d71daea186 Jack Wang 2020-05-11 2748 \tif (!clt->kobj_paths) {\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2749 \t\terr = -ENOMEM;\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2750 \t\tgoto err_dev;\n" + "\n" + "This is wrong. You cannot kfree(clt) after it device_register().\n" + "\n" "6a98d71daea186 Jack Wang 2020-05-11 2751 \t}\n" "6a98d71daea186 Jack Wang 2020-05-11 2752 \terr = rtrs_clt_create_sysfs_root_files(clt);\n" "6a98d71daea186 Jack Wang 2020-05-11 2753 \tif (err) {\n" "6a98d71daea186 Jack Wang 2020-05-11 2754 \t\tkobject_del(clt->kobj_paths);\n" "6a98d71daea186 Jack Wang 2020-05-11 2755 \t\tkobject_put(clt->kobj_paths);\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2756 \t\tgoto err_dev;\n" + "\n" + "The temptation is to add the following to rtrs_clt_dev_release().\n" + "\n" + "\tif (clt->kobj_paths) {\n" + "\t\tkobject_del(clt->kobj_paths);\n" + "\t\tkobject_put(clt->kobj_paths);\n" + "\t}\n" + "\n" + "Does ordering matter? I kind of hate the register_device() stuff\n" + "because it creates bug prone and even unfixable code.\n" + "\n" "6a98d71daea186 Jack Wang 2020-05-11 2757 \t}\n" "6a98d71daea186 Jack Wang 2020-05-11 2758 \tdev_set_uevent_suppress(&clt->dev, false);\n" "6a98d71daea186 Jack Wang 2020-05-11 2759 \tkobject_uevent(&clt->dev.kobj, KOBJ_ADD);\n" @@ -112,18 +127,32 @@ "6a98d71daea186 Jack Wang 2020-05-11 2761 \treturn clt;\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2762 err_dev:\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2763 \tdevice_unregister(&clt->dev);\n" + " ^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + "This will call rtrs_clt_dev_release() and free clt.\n" + "\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2764 err:\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2765 \tfree_percpu(clt->pcpu_path);\n" + "\n" + "Use after fre.\n" + "\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2766 \tkfree(clt);\n" + "\n" + "Double free.\n" + "\n" "7580d72134cc8f Miaoqian Lin 2022-01-20 @2767 \tclt->pcpu_path = NULL;\n" + "\n" + "Use after double free.\n" + "\n" "7580d72134cc8f Miaoqian Lin 2022-01-20 2768 err_free_cpu:\n" "7580d72134cc8f Miaoqian Lin 2022-01-20 2769 \tfree_percpu(clt->pcpu_path);\n" + "\n" + "Another use after double free.\n" + "\n" "eab098246625e9 Guoqing Jiang 2020-12-17 2770 \treturn ERR_PTR(err);\n" "6a98d71daea186 Jack Wang 2020-05-11 2771 }\n" - "6a98d71daea186 Jack Wang 2020-05-11 2772 \n" "\n" "---\n" "0-DAY CI Kernel Test Service, Intel Corporation\n" https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org -f05851e1bf9b7021c53918d01a5436b51c66a454fca6f738fc5c72cd9050e781 +ea9c3497f980bd2433c9ce420cd142eb550316118a7096fb5812c9265695c1cd
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.