| Message ID | 20250306185447.2039336-1-cristian.marussi@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | firmware: arm_scmi: Balance device refcount when destroying devices | expand |
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>
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
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c index 725c7c565333..7732e46d8a95 100644 --- a/drivers/firmware/arm_scmi/bus.c +++ b/drivers/firmware/arm_scmi/bus.c @@ -271,6 +271,9 @@ static struct scmi_device *scmi_child_dev_find(struct device *parent, if (!dev) return NULL; + /* Drop the refcnt bumped implicitly by device_find_child */ + put_device(dev); + return to_scmi_dev(dev); }
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> --- drivers/firmware/arm_scmi/bus.c | 3 +++ 1 file changed, 3 insertions(+)