Re: [PATCH] resource: Fix resource leak in get_free_mem_region()

[Mika Westerberg <mika.westerberg@linux.intel.com>]

On Tue, Mar 04, 2025 at 09:44:38AM +0000, Zhijian Li (Fujitsu) wrote:
> 
> 
> On 04/03/2025 17:22, Mika Westerberg wrote:
> > On Tue, Mar 04, 2025 at 12:34:15PM +0800, Li Zhijian wrote:
> >> The leak is detected by the kernel memory leak detector (`kmemleak`)
> >> following a `cxl create-region` failure:
> >>
> >>   cxl_acpi ACPI0017:00: decoder0.0: created region2
> >>   cxl region2: HPA allocation error (-34) for size:0x0000000100000000 in CXL Window 0 [mem 0xa90000000-0x1a8fffffff flags 0x200]
> >>   kmemleak: 1 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
> >>
> >>      __kmalloc_cache_noprof+0x28c/0x350
> >>      get_free_mem_region+0x45/0x380
> >>      alloc_free_mem_region+0x1d/0x30
> >>      size_store+0x180/0x290 [cxl_core]
> >>      kernfs_fop_write_iter+0x13f/0x1e0
> >>      vfs_write+0x37c/0x540
> >>      ksys_write+0x68/0xe0
> >>      do_syscall_64+0x6e/0x190
> >>      entry_SYSCALL_64_after_hwframe+0x76/0x7e
> >>
> >> Fixes: 14b80582c43e ("resource: Introduce alloc_free_mem_region()")
> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> >> ---
> >>   kernel/resource.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/kernel/resource.c b/kernel/resource.c
> >> index 12004452d999..aa0b1da143eb 100644
> >> --- a/kernel/resource.c
> >> +++ b/kernel/resource.c
> >> @@ -2000,6 +2000,8 @@ get_free_mem_region(struct device *dev, struct resource *base,
> >>   		devres_free(dr);
> >>   	} else if (dev)
> >>   		devm_release_action(dev, remove_free_mem_region, res);
> >> +	else
> >> +		free_resource(res);
> > 
> > It should use {} as per coding style:
> 
> 
> The script/checkpatch.pl is happy with both of these 2 styles in practice.

It is but the coding style prefers then use {} around all branches, see:

https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces

	} else if (dev) {
		devm_release_action(dev, remove_free_mem_region, res);
	} else {
		free_resource(res);
	}