Re: [PATCH] firmware: arm_scmi: Balance device refcount when destroying devices

[Cristian Marussi <cristian.marussi@arm.com>]

On Fri, Mar 07, 2025 at 09:34:15AM +0000, Alice Ryhl wrote:
> On Thu, Mar 06, 2025 at 06:54:47PM +0000, Cristian Marussi wrote:
> > Using device_find_child() to lookup the proper SCMI device to destroy
> > causes an unbalance in device refcount, since device_find_child() calls an
> > implicit get_device(): this, in turns, inhibits the call of the provided
> > release methods upon devices destruction.
> > 
> > As a consequence, one of the structures that is not freed properly upon
> > destruction is the internal struct device_private dev->p populated by the
> > drivers subsystem core.
> > 
> > KMemleak detects this situation since loading/unloding some SCMI driver
> > causes related devices to be created/destroyed without calling any
> > device_release method.
> > 
> > unreferenced object 0xffff00000f583800 (size 512):
> >   comm "insmod", pid 227, jiffies 4294912190
> >   hex dump (first 32 bytes):
> >     00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00  .....N..........
> >     ff ff ff ff ff ff ff ff 60 36 1d 8a 00 80 ff ff  ........`6......
> >   backtrace (crc 114e2eed):
> >     kmemleak_alloc+0xbc/0xd8
> >     __kmalloc_cache_noprof+0x2dc/0x398
> >     device_add+0x954/0x12d0
> >     device_register+0x28/0x40
> >     __scmi_device_create.part.0+0x1bc/0x380
> >     scmi_device_create+0x2d0/0x390
> >     scmi_create_protocol_devices+0x74/0xf8
> >     scmi_device_request_notifier+0x1f8/0x2a8
> >     notifier_call_chain+0x110/0x3b0
> >     blocking_notifier_call_chain+0x70/0xb0
> >     scmi_driver_register+0x350/0x7f0
> >     0xffff80000a3b3038
> >     do_one_initcall+0x12c/0x730
> >     do_init_module+0x1dc/0x640
> >     load_module+0x4b20/0x5b70
> >     init_module_from_file+0xec/0x158
> > 
> > $ ./scripts/faddr2line ./vmlinux device_add+0x954/0x12d0
> > device_add+0x954/0x12d0:
> > kmalloc_noprof at include/linux/slab.h:901
> > (inlined by) kzalloc_noprof at include/linux/slab.h:1037
> > (inlined by) device_private_init at drivers/base/core.c:3510
> > (inlined by) device_add at drivers/base/core.c:3561
> > 
> > Balance device refcount by issuing a put_device() on devices found via
> > device_find_child().
> > 
> > Reported-by: Alice Ryhl <aliceryhl@google.com>
> > Closes: https://lore.kernel.org/linux-arm-kernel/Z8nK3uFkspy61yjP@arm.com/T/#mc1f73a0ea5e41014fa145147b7b839fc988ada8f
> > CC: Sudeep Holla <sudeep.holla@arm.com>
> > CC: Catalin Marinas <catalin.marinas@arm.com>
> > Fixes: d4f9dddd21f3 ("firmware: arm_scmi: Add dynamic scmi devices creation")
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> 
> I was not able to reproduce the memory leak after applying this patch.
> 
> Tested-by: Alice Ryhl <aliceryhl@google.com>

Thanks a lot for testing (and for the report) !

Cristian