* [PATCH 1/1] mfd: core: Fix memory leak of 'cell'
@ 2020-07-16 14:28 Lee Jones
[not found] ` <CGME20200805142610eucas1p1dd85e87d57dab1433d3d31a9c0ec49d6@eucas1p1.samsung.com>
0 siblings, 1 reply; 4+ messages in thread
From: Lee Jones @ 2020-07-16 14:28 UTC (permalink / raw)
To: lee.jones; +Cc: linux-arm-kernel, linux-kernel
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'
[not found] ` <CGME20200805142610eucas1p1dd85e87d57dab1433d3d31a9c0ec49d6@eucas1p1.samsung.com>
@ 2020-08-05 14:26 ` Marek Szyprowski
[not found] ` <CGME20200813072409eucas1p1dd94af95802a53a5b5f2991cd535d843@eucas1p1.samsung.com>
0 siblings, 1 reply; 4+ messages in thread
From: Marek Szyprowski @ 2020-08-05 14:26 UTC (permalink / raw)
To: Lee Jones
Cc: linux-arm-kernel, linux-kernel, Sylwester Nawrocki,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'
[not found] ` <CGME20200813072409eucas1p1dd94af95802a53a5b5f2991cd535d843@eucas1p1.samsung.com>
@ 2020-08-13 7:24 ` Marek Szyprowski
2020-08-13 8:01 ` Lee Jones
0 siblings, 1 reply; 4+ messages in thread
From: Marek Szyprowski @ 2020-08-13 7:24 UTC (permalink / raw)
To: Lee Jones
Cc: linux-arm-kernel, linux-kernel, Sylwester Nawrocki,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'
2020-08-13 7:24 ` Marek Szyprowski
@ 2020-08-13 8:01 ` Lee Jones
0 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2020-08-13 8:01 UTC (permalink / raw)
To: Marek Szyprowski
Cc: linux-arm-kernel, linux-kernel, Sylwester Nawrocki,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-13 8:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 14:28 [PATCH 1/1] mfd: core: Fix memory leak of 'cell' Lee Jones
[not found] ` <CGME20200805142610eucas1p1dd85e87d57dab1433d3d31a9c0ec49d6@eucas1p1.samsung.com>
2020-08-05 14:26 ` Marek Szyprowski
[not found] ` <CGME20200813072409eucas1p1dd94af95802a53a5b5f2991cd535d843@eucas1p1.samsung.com>
2020-08-13 7:24 ` Marek Szyprowski
2020-08-13 8:01 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).