Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'
From:   Marek Szyprowski <m.szyprowski@samsung.com>
To:     Lee Jones <lee.jones@linaro.org>
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Sylwester Nawrocki <s.nawrocki@samsung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Krzysztof Kozlowski <krzk@kernel.org>
Message-ID: <79bcd1b5-2247-5e00-7d49-2f94f9b40744@samsung.com>
Date:   Thu, 13 Aug 2020 09:24:08 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
        Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <072c29cf-b842-28b7-6abb-99ff8f2f5d57@samsung.com>
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20200716142851.1669946-1-lee.jones@linaro.org>
        <072c29cf-b842-28b7-6abb-99ff8f2f5d57@samsung.com>
        <CGME20200813072409eucas1p1dd94af95802a53a5b5f2991cd535d843@eucas1p1.samsung.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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