| Message ID | 20200716142851.1669946-1-lee.jones@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] mfd: core: Fix memory leak of 'cell' | expand |
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
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
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.
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; }
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(+)