2023-07-17 04:08:43

by michenyuan

Subject: [PATCH] cxl: Fix memory leak bug in alloc_mock_res()

When gen_pool_alloc_algo() fails, the error handling path
forgets to free 'res'. It would cause a memory leak bug.

Fix it by add kfree() in error handling path.

Signed-off-by: Chenyuan Mi <[email protected]>
---
tools/testing/cxl/test/cxl.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 0e78d8e19895..250ffd147067 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -405,8 +405,10 @@ static struct cxl_mock_res *alloc_mock_res(resource_size_t size, int align)
INIT_LIST_HEAD(&res->list);
phys = gen_pool_alloc_algo(cxl_mock_pool, size,
gen_pool_first_fit_align, &data);
- if (!phys)
+ if (!phys) {
+ kfree(res);
return NULL;
+ }

res->range = (struct range) {
.start = phys,
--
2.25.1

2023-07-17 11:22:57

by Jonathan Cameron

Subject: Re: [PATCH] cxl: Fix memory leak bug in alloc_mock_res()

On Mon, 17 Jul 2023 04:16:09 +0000
Chenyuan Mi <[email protected]> wrote:

> When gen_pool_alloc_algo() fails, the error handling path
> forgets to free 'res'. It would cause a memory leak bug.
>
> Fix it by add kfree() in error handling path.
>
> Signed-off-by: Chenyuan Mi <[email protected]>

+CC linux-cxl list

Please make sure to include that for any future CXL patches.

> ---
> tools/testing/cxl/test/cxl.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 0e78d8e19895..250ffd147067 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -405,8 +405,10 @@ static struct cxl_mock_res *alloc_mock_res(resource_size_t size, int align)
> INIT_LIST_HEAD(&res->list);
> phys = gen_pool_alloc_algo(cxl_mock_pool, size,
> gen_pool_first_fit_align, &data);
> - if (!phys)
> + if (!phys) {
> + kfree(res);
> return NULL;
> + }
>
> res->range = (struct range) {
> .start = phys,

2023-07-17 16:34:10

by Dave Jiang

Subject: Re: [PATCH] cxl: Fix memory leak bug in alloc_mock_res()

On 7/16/23 21:16, Chenyuan Mi wrote:
> When gen_pool_alloc_algo() fails, the error handling path
> forgets to free 'res'. It would cause a memory leak bug.
>
> Fix it by add kfree() in error handling path.
>
> Signed-off-by: Chenyuan Mi <[email protected]>
Reviewed-by: Dave Jiang <[email protected]>

> ---
> tools/testing/cxl/test/cxl.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 0e78d8e19895..250ffd147067 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -405,8 +405,10 @@ static struct cxl_mock_res *alloc_mock_res(resource_size_t size, int align)
> INIT_LIST_HEAD(&res->list);
> phys = gen_pool_alloc_algo(cxl_mock_pool, size,
> gen_pool_first_fit_align, &data);
> - if (!phys)
> + if (!phys) {
> + kfree(res);
> return NULL;
> + }
>
> res->range = (struct range) {
> .start = phys,

2023-07-18 00:31:13

by Dan Williams

Subject: RE: [PATCH] cxl: Fix memory leak bug in alloc_mock_res()

Chenyuan Mi wrote:
> When gen_pool_alloc_algo() fails, the error handling path
> forgets to free 'res'. It would cause a memory leak bug.
>
> Fix it by add kfree() in error handling path.

Going forward I want to set the policy that any error path resource leaks be
fixed by converting to using __free() and associated helpers.

So in this case it would be:

-- >8 --
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 0e78d8e19895..ea04995fe42c 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -396,7 +396,8 @@ static void depopulate_all_mock_resources(void)

static struct cxl_mock_res *alloc_mock_res(resource_size_t size, int align)
{
- struct cxl_mock_res *res = kzalloc(sizeof(*res), GFP_KERNEL);
+ struct cxl_mock_res *res __free(kfree) =
+ kzalloc(sizeof(*res), GFP_KERNEL);
struct genpool_data_align data = {
.align = align,
};
@@ -416,7 +417,7 @@ static struct cxl_mock_res *alloc_mock_res(resource_size_t size, int align)
list_add(&res->list, &mock_res);
mutex_unlock(&mock_res_lock);

- return res;
+ return_ptr(res);
}

static int populate_cedt(void)

2023-07-18 03:01:26

by Dan Williams

Subject: RE: [PATCH] cxl: Fix memory leak bug in alloc_mock_res()

Dan Williams wrote:
> Chenyuan Mi wrote:
> > When gen_pool_alloc_algo() fails, the error handling path
> > forgets to free 'res'. It would cause a memory leak bug.
> >
> > Fix it by add kfree() in error handling path.
>
> Going forward I want to set the policy that any error path resource leaks be
> fixed by converting to using __free() and associated helpers.
>
> So in this case it would be:

...and to be clear I do think the fix should be applied in a backportable
fashion and then followed up with a conversion for the next merge window.

>
> -- >8 --
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 0e78d8e19895..ea04995fe42c 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -396,7 +396,8 @@ static void depopulate_all_mock_resources(void)
>
> static struct cxl_mock_res *alloc_mock_res(resource_size_t size, int align)
> {
> - struct cxl_mock_res *res = kzalloc(sizeof(*res), GFP_KERNEL);
> + struct cxl_mock_res *res __free(kfree) =
> + kzalloc(sizeof(*res), GFP_KERNEL);
> struct genpool_data_align data = {
> .align = align,
> };
> @@ -416,7 +417,7 @@ static struct cxl_mock_res *alloc_mock_res(resource_size_t size, int align)
> list_add(&res->list, &mock_res);
> mutex_unlock(&mock_res_lock);
>
> - return res;
> + return_ptr(res);
> }
>
> static int populate_cedt(void)