[PATCH 1/1] mfd: core: Fix memory leak of 'cell'

[PATCH 1/1] mfd: core: Fix memory leak of 'cell'

[Lee Jones]

When creating a platform device from an MFD cell description, we
allocate some memory and make a copy which is then stored inside the
platform_device's structure.  However, care is not currently taken to
free the allocated memory when the platform device is torn down.

This patch takes care of the leak.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/mfd-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index f5a73af60dd40..e831e733b38cf 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -297,7 +297,10 @@ static int mfd_remove_devices_fn(struct device *dev, void *data)
 	regulator_bulk_unregister_supply_alias(dev, cell->parent_supplies,
 					       cell->num_parent_supplies);
 
+	kfree(cell);
+
 	platform_device_unregister(pdev);
+
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'

[Marek Szyprowski]

Hi

On 16.07.2020 16:28, Lee Jones wrote:
> When creating a platform device from an MFD cell description, we
> allocate some memory and make a copy which is then stored inside the
> platform_device's structure.  However, care is not currently taken to
> free the allocated memory when the platform device is torn down.
>
> This patch takes care of the leak.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

This patch landed recently in linux-next as commit 126f33704d9d ("mfd: 
core: Fix memory leak of 'cell'"). Sadly it causes a regression on 
Samsung Exynos4412-based Trats2 board:

8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address 00000023
pgd = (ptrval)
[00000023] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1-00055-g126f33704d9d 
#1379
Hardware name: Samsung Exynos (Flattened Device Tree)
PC is at __kmalloc_track_caller+0xe0/0x454
LR is at __kmalloc_track_caller+0x60/0x454
...
Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 4000404a  DAC: 00000051
Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
Stack: (0xef0f19d8 to 0xef0f2000)
...
[<c02b62ec>] (__kmalloc_track_caller) from [<c026e220>] (kstrdup+0x30/0x5c)
[<c026e220>] (kstrdup) from [<c0361250>] (__kernfs_new_node+0x2c/0x280)
[<c0361250>] (__kernfs_new_node) from [<c036247c>] 
(kernfs_new_node+0x40/0x60)
[<c036247c>] (kernfs_new_node) from [<c03643fc>] 
(kernfs_create_link+0x3c/0x90)
[<c03643fc>] (kernfs_create_link) from [<c0365460>] 
(sysfs_do_create_link_sd+0x64/0xd8)
[<c0365460>] (sysfs_do_create_link_sd) from [<c0646fe4>] 
(device_add+0x388/0x744)
[<c0646fe4>] (device_add) from [<c0583874>] 
(regulator_register+0xba8/0x1344)
[<c0583874>] (regulator_register) from [<c0585b74>] 
(devm_regulator_register+0x3c/0x78)
[<c0585b74>] (devm_regulator_register) from [<c058b448>] 
(max77686_pmic_probe+0xc8/0x140)
[<c058b448>] (max77686_pmic_probe) from [<c064d484>] 
(platform_drv_probe+0x6c/0xa4)
[<c064d484>] (platform_drv_probe) from [<c064aab8>] 
(really_probe+0x200/0x48c)
[<c064aab8>] (really_probe) from [<c064aeac>] 
(driver_probe_device+0x78/0x1fc)
[<c064aeac>] (driver_probe_device) from [<c0648998>] 
(bus_for_each_drv+0x74/0xb8)
[<c0648998>] (bus_for_each_drv) from [<c064a818>] 
(__device_attach+0xd4/0x16c)
[<c064a818>] (__device_attach) from [<c064995c>] 
(bus_probe_device+0x88/0x90)
[<c064995c>] (bus_probe_device) from [<c06470ec>] (device_add+0x490/0x744)
[<c06470ec>] (device_add) from [<c064d1e8>] 
(platform_device_add+0x114/0x258)
[<c064d1e8>] (platform_device_add) from [<c067d69c>] 
(mfd_add_devices+0x344/0x408)
[<c067d69c>] (mfd_add_devices) from [<c067d7c4>] 
(devm_mfd_add_devices+0x64/0xa4)
[<c067d7c4>] (devm_mfd_add_devices) from [<c067dfe4>] 
(max77686_i2c_probe+0xfc/0x19c)
[<c067dfe4>] (max77686_i2c_probe) from [<c07c5e40>] 
(i2c_device_probe+0x12c/0x2c4)
[<c07c5e40>] (i2c_device_probe) from [<c064aab8>] (really_probe+0x200/0x48c)
[<c064aab8>] (really_probe) from [<c064aeac>] 
(driver_probe_device+0x78/0x1fc)
[<c064aeac>] (driver_probe_device) from [<c064b294>] 
(device_driver_attach+0x58/0x60)
[<c064b294>] (device_driver_attach) from [<c064b378>] 
(__driver_attach+0xdc/0x174)
[<c064b378>] (__driver_attach) from [<c06488c4>] 
(bus_for_each_dev+0x68/0xb4)
[<c06488c4>] (bus_for_each_dev) from [<c0649bf8>] 
(bus_add_driver+0x158/0x214)
[<c0649bf8>] (bus_add_driver) from [<c064c24c>] (driver_register+0x78/0x110)
[<c064c24c>] (driver_register) from [<c07c6dac>] 
(i2c_register_driver+0x3c/0xac)
[<c07c6dac>] (i2c_register_driver) from [<c0102378>] 
(do_one_initcall+0x8c/0x424)
[<c0102378>] (do_one_initcall) from [<c1001158>] 
(kernel_init_freeable+0x190/0x204)
[<c1001158>] (kernel_init_freeable) from [<c0ab600c>] 
(kernel_init+0x8/0x118)
[<c0ab600c>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
Exception stack(0xef0f1fb0 to 0xef0f1ff8)
...
---[ end trace 1d585642d0e3339f ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

I suspect that this is somehow related to the deferred probe and/or 
devm_ helpers, but I didn't analyze it further yet. Reverting it on top 
of current linux-next (and resolving conflict) fixes the boot. Bisecting 
it was really hard because the issue is not fully reproducible, what 
suggests memory trashing. Various tests of linux-next with the reverted 
patch have shown at least that the issue is gone.

I've compiled the kernel from exynos_defconfig, the dts used for the 
test is arch/arm/boot/dts/exynos4412-trats2.dts

 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'

[Marek Szyprowski]

Hi All,

On 05.08.2020 16:26, Marek Szyprowski wrote:
> On 16.07.2020 16:28, Lee Jones wrote:
>> When creating a platform device from an MFD cell description, we
>> allocate some memory and make a copy which is then stored inside the
>> platform_device's structure.  However, care is not currently taken to
>> free the allocated memory when the platform device is torn down.
>>
>> This patch takes care of the leak.
>>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>
> This patch landed recently in linux-next as commit 126f33704d9d ("mfd: 
> core: Fix memory leak of 'cell'"). Sadly it causes a regression on 
> Samsung Exynos4412-based Trats2 board:
> ...
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> I suspect that this is somehow related to the deferred probe and/or 
> devm_ helpers, but I didn't analyze it further yet. Reverting it on 
> top of current linux-next (and resolving conflict) fixes the boot. 
> Bisecting it was really hard because the issue is not fully 
> reproducible, what suggests memory trashing. Various tests of 
> linux-next with the reverted patch have shown at least that the issue 
> is gone.
>
> I've compiled the kernel from exynos_defconfig, the dts used for the 
> test is arch/arm/boot/dts/exynos4412-trats2.dts

Finally I've found some time to analyze this issue.

Indeed this patch is wrong it causes double free on the mfd_cell. 
mfd_cell is already properly freed by the platform_device_release() 
function when kref of the pdev goes down to zero: 
https://elixir.bootlin.com/linux/latest/source/drivers/base/platform.c#L426

I will send a revert with the explaination.

Here is a better log I've captured with more debugging options enabled:

=============================================================================
BUG kmalloc-128 (Not tainted): Object already free
-----------------------------------------------------------------------------

Disabling lock debugging due to kernel taint
INFO: Allocated in kobject_get_path+0x60/0xfc age=2 cpu=2 pid=1
  __kmalloc+0x298/0x544
  kobject_get_path+0x60/0xfc
  kobject_uevent_env+0x140/0x684
  device_del+0x264/0x39c
  device_unregister+0x24/0x64
  regulator_unregister+0xb8/0xe4
  release_nodes+0x18c/0x238
  device_release_driver_internal+0x104/0x1bc
  bus_remove_device+0xe4/0x13c
  device_del+0x158/0x39c
  platform_device_del+0x20/0x88
  platform_device_unregister+0xc/0x18
  mfd_remove_devices_fn+0x9c/0xcc
  device_for_each_child_reverse+0x60/0x9c
  mfd_remove_devices+0x30/0x4c
  wm8994_i2c_probe+0x318/0x8f8
INFO: Freed in kobject_uevent_env+0x178/0x684 age=2 cpu=2 pid=1
  kobject_uevent_env+0x178/0x684
  device_del+0x264/0x39c
  device_unregister+0x24/0x64
  regulator_unregister+0xb8/0xe4
  release_nodes+0x18c/0x238
  device_release_driver_internal+0x104/0x1bc
  bus_remove_device+0xe4/0x13c
  device_del+0x158/0x39c
  platform_device_del+0x20/0x88
  platform_device_unregister+0xc/0x18
  mfd_remove_devices_fn+0x9c/0xcc
  device_for_each_child_reverse+0x60/0x9c
  mfd_remove_devices+0x30/0x4c
  wm8994_i2c_probe+0x318/0x8f8
  i2c_device_probe+0x254/0x2bc
  really_probe+0x200/0x4fc
INFO: Slab 0x(ptrval) objects=16 used=9 fp=0x(ptrval) flags=0x10201
INFO: Object 0x(ptrval) @offset=3712 fp=0x00000000

Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 
kkkkkkkkkkkkkkk.
Redzone (ptrval): bb bb bb bb ....
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ
CPU: 2 PID: 1 Comm: swapper/0 Tainted: G    B 5.8.0-next-20200812 #8987
Hardware name: Samsung Exynos (Flattened Device Tree)
[<c0111b08>] (unwind_backtrace) from [<c010d32c>] (show_stack+0x10/0x14)
[<c010d32c>] (show_stack) from [<c054d994>] (dump_stack+0xbc/0xe8)
[<c054d994>] (dump_stack) from [<c02c8860>] 
(free_debug_processing+0x24c/0x3c0)
[<c02c8860>] (free_debug_processing) from [<c02c8ca0>] 
(__slab_free+0x2cc/0x4bc)
[<c02c8ca0>] (__slab_free) from [<c02c9144>] (kfree+0x2b4/0x480)
[<c02c9144>] (kfree) from [<c0688134>] (platform_device_release+0x18/0x34)
[<c0688134>] (platform_device_release) from [<c067ea8c>] 
(device_release+0x28/0x98)
[<c067ea8c>] (device_release) from [<c05542bc>] (kobject_put+0x104/0x288)
[<c05542bc>] (kobject_put) from [<c0553ac4>] (klist_prev+0xd8/0x16c)
[<c0553ac4>] (klist_prev) from [<c067f148>] 
(device_for_each_child_reverse+0x6c/0x9c)
[<c067f148>] (device_for_each_child_reverse) from [<c06bab74>] 
(mfd_remove_devices+0x30/0x4c)
[<c06bab74>] (mfd_remove_devices) from [<c06b9be8>] 
(wm8994_i2c_probe+0x318/0x8f8)
[<c06b9be8>] (wm8994_i2c_probe) from [<c080d6c8>] 
(i2c_device_probe+0x254/0x2bc)
[<c080d6c8>] (i2c_device_probe) from [<c06858b4>] (really_probe+0x200/0x4fc)
[<c06858b4>] (really_probe) from [<c0685d78>] 
(driver_probe_device+0x78/0x1fc)
[<c0685d78>] (driver_probe_device) from [<c0686160>] 
(device_driver_attach+0x58/0x60)
[<c0686160>] (device_driver_attach) from [<c0686244>] 
(__driver_attach+0xdc/0x174)
[<c0686244>] (__driver_attach) from [<c068363c>] 
(bus_for_each_dev+0x68/0xb4)
[<c068363c>] (bus_for_each_dev) from [<c0684970>] 
(bus_add_driver+0x158/0x214)
[<c0684970>] (bus_add_driver) from [<c0687358>] (driver_register+0x78/0x110)
[<c0687358>] (driver_register) from [<c080e508>] 
(i2c_register_driver+0x3c/0xac)
[<c080e508>] (i2c_register_driver) from [<c010254c>] 
(do_one_initcall+0x94/0x53c)
[<c010254c>] (do_one_initcall) from [<c1101200>] 
(kernel_init_freeable+0x190/0x1dc)
[<c1101200>] (kernel_init_freeable) from [<c0b18024>] 
(kernel_init+0x8/0x118)
[<c0b18024>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
Exception stack(0xef1bbfb0 to 0xef1bbff8)
bfa0:                                     00000000 00000000 00000000 
00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 
00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
FIX kmalloc-128: Object at 0x(ptrval) not freed
=============================================================================
BUG kmalloc-128 (Tainted: G    B            ): Wrong object count. 
Counter is 9 but counted were 14
-----------------------------------------------------------------------------

INFO: Slab 0x(ptrval) objects=16 used=9 fp=0x(ptrval) flags=0x10201
CPU: 2 PID: 1 Comm: swapper/0 Tainted: G    B 5.8.0-next-20200812 #8987
Hardware name: Samsung Exynos (Flattened Device Tree)
[<c0111b08>] (unwind_backtrace) from [<c010d32c>] (show_stack+0x10/0x14)
[<c010d32c>] (show_stack) from [<c054d994>] (dump_stack+0xbc/0xe8)
[<c054d994>] (dump_stack) from [<c02c35c4>] (slab_err+0x80/0xa4)
[<c02c35c4>] (slab_err) from [<c02c8234>] (on_freelist+0x1a8/0x23c)
[<c02c8234>] (on_freelist) from [<c02c8774>] 
(free_debug_processing+0x160/0x3c0)
[<c02c8774>] (free_debug_processing) from [<c02c8ca0>] 
(__slab_free+0x2cc/0x4bc)
[<c02c8ca0>] (__slab_free) from [<c02c9144>] (kfree+0x2b4/0x480)
[<c02c9144>] (kfree) from [<c06bad14>] (mfd_remove_devices_fn+0x94/0xcc)
[<c06bad14>] (mfd_remove_devices_fn) from [<c067f13c>] 
(device_for_each_child_reverse+0x60/0x9c)
[<c067f13c>] (device_for_each_child_reverse) from [<c06bab74>] 
(mfd_remove_devices+0x30/0x4c)
[<c06bab74>] (mfd_remove_devices) from [<c06b9be8>] 
(wm8994_i2c_probe+0x318/0x8f8)
[<c06b9be8>] (wm8994_i2c_probe) from [<c080d6c8>] 
(i2c_device_probe+0x254/0x2bc)
[<c080d6c8>] (i2c_device_probe) from [<c06858b4>] (really_probe+0x200/0x4fc)
[<c06858b4>] (really_probe) from [<c0685d78>] 
(driver_probe_device+0x78/0x1fc)
[<c0685d78>] (driver_probe_device) from [<c0686160>] 
(device_driver_attach+0x58/0x60)
[<c0686160>] (device_driver_attach) from [<c0686244>] 
(__driver_attach+0xdc/0x174)
[<c0686244>] (__driver_attach) from [<c068363c>] 
(bus_for_each_dev+0x68/0xb4)
[<c068363c>] (bus_for_each_dev) from [<c0684970>] 
(bus_add_driver+0x158/0x214)
[<c0684970>] (bus_add_driver) from [<c0687358>] (driver_register+0x78/0x110)
[<c0687358>] (driver_register) from [<c080e508>] 
(i2c_register_driver+0x3c/0xac)
[<c080e508>] (i2c_register_driver) from [<c010254c>] 
(do_one_initcall+0x94/0x53c)
[<c010254c>] (do_one_initcall) from [<c1101200>] 
(kernel_init_freeable+0x190/0x1dc)
[<c1101200>] (kernel_init_freeable) from [<c0b18024>] 
(kernel_init+0x8/0x118)
[<c0b18024>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
Exception stack(0xef1bbfb0 to 0xef1bbff8)
bfa0:                                     00000000 00000000 00000000 
00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 
00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
FIX kmalloc-128: Object count adjusted.
=============================================================================
BUG kmalloc-128 (Tainted: G    B            ): Object already free
-----------------------------------------------------------------------------

INFO: Allocated in kobject_get_path+0x60/0xfc age=2 cpu=2 pid=1
  __kmalloc+0x298/0x544
  kobject_get_path+0x60/0xfc
  kobject_uevent_env+0x140/0x684
  device_del+0x264/0x39c
  device_unregister+0x24/0x64
  regulator_unregister+0xb8/0xe4
  release_nodes+0x18c/0x238
  device_release_driver_internal+0x104/0x1bc
  bus_remove_device+0xe4/0x13c
  device_del+0x158/0x39c
  platform_device_del+0x20/0x88
  platform_device_unregister+0xc/0x18
  mfd_remove_devices_fn+0x9c/0xcc
  device_for_each_child_reverse+0x60/0x9c
  mfd_remove_devices+0x30/0x4c
  wm8994_i2c_probe+0x318/0x8f8
INFO: Freed in kobject_uevent_env+0x178/0x684 age=2 cpu=2 pid=1
  kobject_uevent_env+0x178/0x684
  device_del+0x264/0x39c
  device_unregister+0x24/0x64
  regulator_unregister+0xb8/0xe4
  release_nodes+0x18c/0x238
  device_release_driver_internal+0x104/0x1bc
  bus_remove_device+0xe4/0x13c
  device_del+0x158/0x39c
  platform_device_del+0x20/0x88
  platform_device_unregister+0xc/0x18
  mfd_remove_devices_fn+0x9c/0xcc
  device_for_each_child_reverse+0x60/0x9c
  mfd_remove_devices+0x30/0x4c
  wm8994_i2c_probe+0x318/0x8f8
  i2c_device_probe+0x254/0x2bc
  really_probe+0x200/0x4fc
INFO: Slab 0x(ptrval) objects=16 used=12 fp=0x(ptrval) flags=0x10201
INFO: Object 0x(ptrval) @offset=1664 fp=0x00000000

Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Redzone (ptrval): bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 
................
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 
kkkkkkkkkkkkkkkk
Object (ptrval): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 
kkkkkkkkkkkkkkk.
Redzone (ptrval): bb bb bb bb ....
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 
ZZZZZZZZZZZZZZZZ
Padding (ptrval): 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ
CPU: 2 PID: 1 Comm: swapper/0 Tainted: G    B 5.8.0-next-20200812 #8987
Hardware name: Samsung Exynos (Flattened Device Tree)
[<c0111b08>] (unwind_backtrace) from [<c010d32c>] (show_stack+0x10/0x14)
[<c010d32c>] (show_stack) from [<c054d994>] (dump_stack+0xbc/0xe8)
[<c054d994>] (dump_stack) from [<c02c8860>] 
(free_debug_processing+0x24c/0x3c0)
[<c02c8860>] (free_debug_processing) from [<c02c8ca0>] 
(__slab_free+0x2cc/0x4bc)
[<c02c8ca0>] (__slab_free) from [<c02c9144>] (kfree+0x2b4/0x480)
[<c02c9144>] (kfree) from [<c0688134>] (platform_device_release+0x18/0x34)
[<c0688134>] (platform_device_release) from [<c067ea8c>] 
(device_release+0x28/0x98)
[<c067ea8c>] (device_release) from [<c05542bc>] (kobject_put+0x104/0x288)
[<c05542bc>] (kobject_put) from [<c0553ac4>] (klist_prev+0xd8/0x16c)
[<c0553ac4>] (klist_prev) from [<c067f148>] 
(device_for_each_child_reverse+0x6c/0x9c)
[<c067f148>] (device_for_each_child_reverse) from [<c06bab74>] 
(mfd_remove_devices+0x30/0x4c)
[<c06bab74>] (mfd_remove_devices) from [<c06b9be8>] 
(wm8994_i2c_probe+0x318/0x8f8)
[<c06b9be8>] (wm8994_i2c_probe) from [<c080d6c8>] 
(i2c_device_probe+0x254/0x2bc)
[<c080d6c8>] (i2c_device_probe) from [<c06858b4>] (really_probe+0x200/0x4fc)
[<c06858b4>] (really_probe) from [<c0685d78>] 
(driver_probe_device+0x78/0x1fc)
[<c0685d78>] (driver_probe_device) from [<c0686160>] 
(device_driver_attach+0x58/0x60)
[<c0686160>] (device_driver_attach) from [<c0686244>] 
(__driver_attach+0xdc/0x174)
[<c0686244>] (__driver_attach) from [<c068363c>] 
(bus_for_each_dev+0x68/0xb4)
[<c068363c>] (bus_for_each_dev) from [<c0684970>] 
(bus_add_driver+0x158/0x214)
[<c0684970>] (bus_add_driver) from [<c0687358>] (driver_register+0x78/0x110)
[<c0687358>] (driver_register) from [<c080e508>] 
(i2c_register_driver+0x3c/0xac)
[<c080e508>] (i2c_register_driver) from [<c010254c>] 
(do_one_initcall+0x94/0x53c)
[<c010254c>] (do_one_initcall) from [<c1101200>] 
(kernel_init_freeable+0x190/0x1dc)
[<c1101200>] (kernel_init_freeable) from [<c0b18024>] 
(kernel_init+0x8/0x118)
[<c0b18024>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
Exception stack(0xef1bbfb0 to 0xef1bbff8)
bfa0:                                     00000000 00000000 00000000 
00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 
00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
FIX kmalloc-128: Object at 0x(ptrval) not freed


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'

[Lee Jones]

On Thu, 13 Aug 2020, Marek Szyprowski wrote:

> Hi All,
> 
> On 05.08.2020 16:26, Marek Szyprowski wrote:
> > On 16.07.2020 16:28, Lee Jones wrote:
> >> When creating a platform device from an MFD cell description, we
> >> allocate some memory and make a copy which is then stored inside the
> >> platform_device's structure.  However, care is not currently taken to
> >> free the allocated memory when the platform device is torn down.
> >>
> >> This patch takes care of the leak.
> >>
> >> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >
> > This patch landed recently in linux-next as commit 126f33704d9d ("mfd: 
> > core: Fix memory leak of 'cell'"). Sadly it causes a regression on 
> > Samsung Exynos4412-based Trats2 board:
> > ...
> > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >
> > I suspect that this is somehow related to the deferred probe and/or 
> > devm_ helpers, but I didn't analyze it further yet. Reverting it on 
> > top of current linux-next (and resolving conflict) fixes the boot. 
> > Bisecting it was really hard because the issue is not fully 
> > reproducible, what suggests memory trashing. Various tests of 
> > linux-next with the reverted patch have shown at least that the issue 
> > is gone.
> >
> > I've compiled the kernel from exynos_defconfig, the dts used for the 
> > test is arch/arm/boot/dts/exynos4412-trats2.dts
> 
> Finally I've found some time to analyze this issue.
> 
> Indeed this patch is wrong it causes double free on the mfd_cell. 
> mfd_cell is already properly freed by the platform_device_release() 
> function when kref of the pdev goes down to zero: 
> https://elixir.bootlin.com/linux/latest/source/drivers/base/platform.c#L426

Thank you for the explanation.

> I will send a revert with the explaination.

No need.  I remove it from my tree.  Thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog