Message-ID: <072c29cf-b842-28b7-6abb-99ff8f2f5d57@samsung.com>
Date:   Wed, 5 Aug 2020 16:26:09 +0200
From:   Marek Szyprowski <m.szyprowski@...sung.com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Sylwester Nawrocki <s.nawrocki@...sung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Krzysztof Kozlowski <krzk@...nel.org>
Subject: Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'

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@...aro.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

Powered by blists - more mailing lists