* [PATCH] RDMA/rtrs: Fix double free in alloc_clt
@ 2022-01-20 10:37 Miaoqian Lin
2022-01-20 14:06 ` Jinpu Wang
2022-01-23 10:44 ` Leon Romanovsky
0 siblings, 2 replies; 5+ messages in thread
From: Miaoqian Lin @ 2022-01-20 10:37 UTC (permalink / raw)
To: Md. Haris Iqbal, Jack Wang, Jason Gunthorpe, Danil Kipnis,
linux-rdma, linux-kernel
Cc: linmq006
Callback function rtrs_clt_dev_release() in put_device()
calls kfree(clt); to free memory. We shouldn't call kfree(clt) again.
Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 7c3f98e57889..61723f48fbd4 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2741,7 +2741,7 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
err = device_register(&clt->dev);
if (err) {
put_device(&clt->dev);
- goto err;
+ goto err_free_cpu;
}
clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj);
@@ -2764,6 +2764,9 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
err:
free_percpu(clt->pcpu_path);
kfree(clt);
+ clt->pcpu_path = NULL;
+err_free_cpu:
+ free_percpu(clt->pcpu_path);
return ERR_PTR(err);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] RDMA/rtrs: Fix double free in alloc_clt
2022-01-20 10:37 [PATCH] RDMA/rtrs: Fix double free in alloc_clt Miaoqian Lin
@ 2022-01-20 14:06 ` Jinpu Wang
2022-01-23 10:44 ` Leon Romanovsky
1 sibling, 0 replies; 5+ messages in thread
From: Jinpu Wang @ 2022-01-20 14:06 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Md. Haris Iqbal, Jason Gunthorpe, Danil Kipnis, linux-rdma, linux-kernel
On Thu, Jan 20, 2022 at 11:37 AM Miaoqian Lin <linmq006@gmail.com> wrote:
>
> Callback function rtrs_clt_dev_release() in put_device()
> calls kfree(clt); to free memory. We shouldn't call kfree(clt) again.
>
right, there is a bug, but the fix is not right.
> Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 7c3f98e57889..61723f48fbd4 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -2741,7 +2741,7 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
> err = device_register(&clt->dev);
> if (err) {
> put_device(&clt->dev);
> - goto err;
> + goto err_free_cpu;
> }
>
> clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj);
> @@ -2764,6 +2764,9 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
> err:
> free_percpu(clt->pcpu_path);
> kfree(clt);
> + clt->pcpu_path = NULL;
> +err_free_cpu:
> + free_percpu(clt->pcpu_path);
this still lead to use after free, because clt could be free already.
the right fix should move the free_percpu() to the release call back
before kfree(clt);
> return ERR_PTR(err);
> }
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RDMA/rtrs: Fix double free in alloc_clt
2022-01-20 10:37 [PATCH] RDMA/rtrs: Fix double free in alloc_clt Miaoqian Lin
2022-01-20 14:06 ` Jinpu Wang
@ 2022-01-23 10:44 ` Leon Romanovsky
1 sibling, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2022-01-23 10:44 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Md. Haris Iqbal, Jack Wang, Jason Gunthorpe, Danil Kipnis,
linux-rdma, linux-kernel
On Thu, Jan 20, 2022 at 10:37:14AM +0000, Miaoqian Lin wrote:
> Callback function rtrs_clt_dev_release() in put_device()
> calls kfree(clt); to free memory. We shouldn't call kfree(clt) again.
>
> Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 7c3f98e57889..61723f48fbd4 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -2741,7 +2741,7 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
> err = device_register(&clt->dev);
> if (err) {
> put_device(&clt->dev);
> - goto err;
> + goto err_free_cpu;
> }
>
> clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj);
> @@ -2764,6 +2764,9 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
> err:
> free_percpu(clt->pcpu_path);
> kfree(clt);
> + clt->pcpu_path = NULL;
Like Jinpu said, it will crash, you are trying to access clt after kfree.
Thanks
> +err_free_cpu:
> + free_percpu(clt->pcpu_path);
> return ERR_PTR(err);
> }
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RDMA/rtrs: Fix double free in alloc_clt
2022-01-20 10:37 [PATCH] RDMA/rtrs: Fix double free in alloc_clt Miaoqian Lin
@ 2022-01-26 10:05 ` Dan Carpenter
2022-01-23 10:44 ` Leon Romanovsky
1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-01-24 13:08 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 8004 bytes --]
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
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/infiniband/ulp/rtrs/rtrs-clt.c:2767 alloc_clt() error: dereferencing freed memory 'clt'
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,
6a98d71daea186 Jack Wang 2020-05-11 2691 enum rtrs_clt_link_ev ev),
6a98d71daea186 Jack Wang 2020-05-11 2692 unsigned int reconnect_delay_sec,
6a98d71daea186 Jack Wang 2020-05-11 2693 unsigned int max_reconnect_attempts)
6a98d71daea186 Jack Wang 2020-05-11 2694 {
f3433d79cd50d3 Vaishali Thakkar 2022-01-05 2695 struct rtrs_clt_sess *clt;
6a98d71daea186 Jack Wang 2020-05-11 2696 int err;
6a98d71daea186 Jack Wang 2020-05-11 2697
6a98d71daea186 Jack Wang 2020-05-11 2698 if (!paths_num || paths_num > MAX_PATHS_NUM)
6a98d71daea186 Jack Wang 2020-05-11 2699 return ERR_PTR(-EINVAL);
6a98d71daea186 Jack Wang 2020-05-11 2700
6a98d71daea186 Jack Wang 2020-05-11 2701 if (strlen(sessname) >= sizeof(clt->sessname))
6a98d71daea186 Jack Wang 2020-05-11 2702 return ERR_PTR(-EINVAL);
6a98d71daea186 Jack Wang 2020-05-11 2703
6a98d71daea186 Jack Wang 2020-05-11 2704 clt = kzalloc(sizeof(*clt), GFP_KERNEL);
6a98d71daea186 Jack Wang 2020-05-11 2705 if (!clt)
6a98d71daea186 Jack Wang 2020-05-11 2706 return ERR_PTR(-ENOMEM);
6a98d71daea186 Jack Wang 2020-05-11 2707
6a98d71daea186 Jack Wang 2020-05-11 2708 clt->pcpu_path = alloc_percpu(typeof(*clt->pcpu_path));
6a98d71daea186 Jack Wang 2020-05-11 2709 if (!clt->pcpu_path) {
6a98d71daea186 Jack Wang 2020-05-11 2710 kfree(clt);
6a98d71daea186 Jack Wang 2020-05-11 2711 return ERR_PTR(-ENOMEM);
6a98d71daea186 Jack Wang 2020-05-11 2712 }
6a98d71daea186 Jack Wang 2020-05-11 2713
6a98d71daea186 Jack Wang 2020-05-11 2714 uuid_gen(&clt->paths_uuid);
6a98d71daea186 Jack Wang 2020-05-11 2715 INIT_LIST_HEAD_RCU(&clt->paths_list);
6a98d71daea186 Jack Wang 2020-05-11 2716 clt->paths_num = paths_num;
6a98d71daea186 Jack Wang 2020-05-11 2717 clt->paths_up = MAX_PATHS_NUM;
6a98d71daea186 Jack Wang 2020-05-11 2718 clt->port = port;
6a98d71daea186 Jack Wang 2020-05-11 2719 clt->pdu_sz = pdu_sz;
6fc45596506b7a Jack Wang 2021-06-21 2720 clt->max_segments = RTRS_MAX_SEGMENTS;
6a98d71daea186 Jack Wang 2020-05-11 2721 clt->reconnect_delay_sec = reconnect_delay_sec;
6a98d71daea186 Jack Wang 2020-05-11 2722 clt->max_reconnect_attempts = max_reconnect_attempts;
6a98d71daea186 Jack Wang 2020-05-11 2723 clt->priv = priv;
6a98d71daea186 Jack Wang 2020-05-11 2724 clt->link_ev = link_ev;
6a98d71daea186 Jack Wang 2020-05-11 2725 clt->mp_policy = MP_POLICY_MIN_INFLIGHT;
2d612f0d3d4b4c Dima Stepanov 2021-05-28 2726 strscpy(clt->sessname, sessname, sizeof(clt->sessname));
6a98d71daea186 Jack Wang 2020-05-11 2727 init_waitqueue_head(&clt->permits_wait);
6a98d71daea186 Jack Wang 2020-05-11 2728 mutex_init(&clt->paths_ev_mutex);
6a98d71daea186 Jack Wang 2020-05-11 2729 mutex_init(&clt->paths_mutex);
6a98d71daea186 Jack Wang 2020-05-11 2730
6a98d71daea186 Jack Wang 2020-05-11 2731 clt->dev.class = rtrs_clt_dev_class;
6a98d71daea186 Jack Wang 2020-05-11 2732 clt->dev.release = rtrs_clt_dev_release;
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;
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
6a98d71daea186 Jack Wang 2020-05-11 2739 */
6a98d71daea186 Jack Wang 2020-05-11 2740 dev_set_uevent_suppress(&clt->dev, true);
6a98d71daea186 Jack Wang 2020-05-11 2741 err = device_register(&clt->dev);
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;
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;
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;
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);
6a98d71daea186 Jack Wang 2020-05-11 2760
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);
eab098246625e9 Guoqing Jiang 2020-12-17 2764 err:
eab098246625e9 Guoqing Jiang 2020-12-17 2765 free_percpu(clt->pcpu_path);
eab098246625e9 Guoqing Jiang 2020-12-17 2766 kfree(clt);
7580d72134cc8f Miaoqian Lin 2022-01-20 @2767 clt->pcpu_path = NULL;
7580d72134cc8f Miaoqian Lin 2022-01-20 2768 err_free_cpu:
7580d72134cc8f Miaoqian Lin 2022-01-20 2769 free_percpu(clt->pcpu_path);
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
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RDMA/rtrs: Fix double free in alloc_clt
@ 2022-01-26 10:05 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2022-01-26 10:05 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 8191 bytes --]
Hi Miaoqian,
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
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
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/infiniband/ulp/rtrs/rtrs-clt.c:2767 alloc_clt() error: dereferencing freed memory 'clt'
vim +/clt +2767 drivers/infiniband/ulp/rtrs/rtrs-clt.c
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,
6a98d71daea186 Jack Wang 2020-05-11 2691 enum rtrs_clt_link_ev ev),
6a98d71daea186 Jack Wang 2020-05-11 2692 unsigned int reconnect_delay_sec,
6a98d71daea186 Jack Wang 2020-05-11 2693 unsigned int max_reconnect_attempts)
6a98d71daea186 Jack Wang 2020-05-11 2694 {
f3433d79cd50d3 Vaishali Thakkar 2022-01-05 2695 struct rtrs_clt_sess *clt;
6a98d71daea186 Jack Wang 2020-05-11 2696 int err;
6a98d71daea186 Jack Wang 2020-05-11 2697
6a98d71daea186 Jack Wang 2020-05-11 2698 if (!paths_num || paths_num > MAX_PATHS_NUM)
6a98d71daea186 Jack Wang 2020-05-11 2699 return ERR_PTR(-EINVAL);
6a98d71daea186 Jack Wang 2020-05-11 2700
6a98d71daea186 Jack Wang 2020-05-11 2701 if (strlen(sessname) >= sizeof(clt->sessname))
6a98d71daea186 Jack Wang 2020-05-11 2702 return ERR_PTR(-EINVAL);
6a98d71daea186 Jack Wang 2020-05-11 2703
6a98d71daea186 Jack Wang 2020-05-11 2704 clt = kzalloc(sizeof(*clt), GFP_KERNEL);
6a98d71daea186 Jack Wang 2020-05-11 2705 if (!clt)
6a98d71daea186 Jack Wang 2020-05-11 2706 return ERR_PTR(-ENOMEM);
6a98d71daea186 Jack Wang 2020-05-11 2707
6a98d71daea186 Jack Wang 2020-05-11 2708 clt->pcpu_path = alloc_percpu(typeof(*clt->pcpu_path));
6a98d71daea186 Jack Wang 2020-05-11 2709 if (!clt->pcpu_path) {
6a98d71daea186 Jack Wang 2020-05-11 2710 kfree(clt);
6a98d71daea186 Jack Wang 2020-05-11 2711 return ERR_PTR(-ENOMEM);
6a98d71daea186 Jack Wang 2020-05-11 2712 }
6a98d71daea186 Jack Wang 2020-05-11 2713
6a98d71daea186 Jack Wang 2020-05-11 2714 uuid_gen(&clt->paths_uuid);
6a98d71daea186 Jack Wang 2020-05-11 2715 INIT_LIST_HEAD_RCU(&clt->paths_list);
6a98d71daea186 Jack Wang 2020-05-11 2716 clt->paths_num = paths_num;
6a98d71daea186 Jack Wang 2020-05-11 2717 clt->paths_up = MAX_PATHS_NUM;
6a98d71daea186 Jack Wang 2020-05-11 2718 clt->port = port;
6a98d71daea186 Jack Wang 2020-05-11 2719 clt->pdu_sz = pdu_sz;
6fc45596506b7a Jack Wang 2021-06-21 2720 clt->max_segments = RTRS_MAX_SEGMENTS;
6a98d71daea186 Jack Wang 2020-05-11 2721 clt->reconnect_delay_sec = reconnect_delay_sec;
6a98d71daea186 Jack Wang 2020-05-11 2722 clt->max_reconnect_attempts = max_reconnect_attempts;
6a98d71daea186 Jack Wang 2020-05-11 2723 clt->priv = priv;
6a98d71daea186 Jack Wang 2020-05-11 2724 clt->link_ev = link_ev;
6a98d71daea186 Jack Wang 2020-05-11 2725 clt->mp_policy = MP_POLICY_MIN_INFLIGHT;
2d612f0d3d4b4c Dima Stepanov 2021-05-28 2726 strscpy(clt->sessname, sessname, sizeof(clt->sessname));
6a98d71daea186 Jack Wang 2020-05-11 2727 init_waitqueue_head(&clt->permits_wait);
6a98d71daea186 Jack Wang 2020-05-11 2728 mutex_init(&clt->paths_ev_mutex);
6a98d71daea186 Jack Wang 2020-05-11 2729 mutex_init(&clt->paths_mutex);
6a98d71daea186 Jack Wang 2020-05-11 2730
6a98d71daea186 Jack Wang 2020-05-11 2731 clt->dev.class = rtrs_clt_dev_class;
6a98d71daea186 Jack Wang 2020-05-11 2732 clt->dev.release = rtrs_clt_dev_release;
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
6a98d71daea186 Jack Wang 2020-05-11 2739 */
6a98d71daea186 Jack Wang 2020-05-11 2740 dev_set_uevent_suppress(&clt->dev, true);
6a98d71daea186 Jack Wang 2020-05-11 2741 err = device_register(&clt->dev);
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);
6a98d71daea186 Jack Wang 2020-05-11 2760
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 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-26 10:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-20 10:37 [PATCH] RDMA/rtrs: Fix double free in alloc_clt Miaoqian Lin
2022-01-20 14:06 ` Jinpu Wang
2022-01-23 10:44 ` Leon Romanovsky
2022-01-24 13:08 kernel test robot
2022-01-26 10:05 ` Dan Carpenter
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.