On Wed, 2013-05-15 at 16:02 +0100, Catalin Marinas wrote: > On Wed, May 15, 2013 at 04:15:46AM +0100, Larry Finger wrote: > > I do not see that particular one; however, I see 4 instances of > > > > unreferenced object 0xffff8800b7979750 (size 8): > > comm "swapper/0", pid 1, jiffies 4294892402 (age 21888.316s) > > hex dump (first 8 bytes): > > 31 38 00 b7 00 88 ff ff 18...... > > backtrace: > > [<ffffffff81432ea1>] kmemleak_alloc+0x21/0x50 > > [<ffffffff81145d50>] __kmalloc_track_caller+0x140/0x2b0 > > [<ffffffff81119fb5>] kstrdup+0x35/0x70 > > [<ffffffff8125febc>] acpi_set_pnp_ids+0xd0/0x304 > > [<ffffffff81260c47>] acpi_scan_init_hotplug+0x47/0xa1 > > [<ffffffff81261223>] acpi_bus_check_add+0x66/0xd7 > > [<ffffffff8127877a>] acpi_ns_walk_namespace+0xb9/0x173 > > [<ffffffff81278bf3>] acpi_walk_namespace+0x93/0xc6 > > [<ffffffff812612dc>] acpi_bus_scan+0x48/0x9a > > [<ffffffff818c983d>] acpi_scan_init+0x57/0x14b > > [<ffffffff818c966a>] acpi_init+0x244/0x286 > > [<ffffffff810002fa>] do_one_initcall+0x10a/0x160 > > [<ffffffff8189cef0>] kernel_init_freeable+0x103/0x192 > > [<ffffffff814313a9>] kernel_init+0x9/0xf0 > > [<ffffffff8144992c>] ret_from_fork+0x7c/0xb0 > > [<ffffffffffffffff>] 0xffffffffffffffff > > > > All four were allocated early in the bootup, and are the only leaks reported in > > my system. I have not yet tested to see if they are false. > > This looks to me like a real leak, possibly introduced by commit > 6b772e8f9 (ACPI: Update PNPID match handling for notify). The > acpi_scan_init_hotplug() function calls acpi_set_pnp_ids() which > allocates pnp.unique_id (kstrdup()) but for some reason it fails and > does not set pnp.type.hardware_id. The return does not call > acpi_free_pnp_ids() which would be responsible for such freeing. I agree with your analysis. It appears that this ACPI device object has _UID but not _HID. ACPI spec defines that _UID is a unique ID among a same _HID. So, it is odd to have _UID without _HID. But, nonetheless, this case needs to be handled as such systems exist. > Something like below, but not tested and may fail some NULL pointer > checks: The change looks good. acpi_free_pnp_ids() handles this empty list pnp->ids properly. Are you going to submit this patch with your signed-off? Thanks! -Toshi > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index fe158fd..c1bc608 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1785,7 +1785,7 @@ static void acpi_scan_init_hotplug(acpi_handle handle, int type) > acpi_set_pnp_ids(handle, &pnp, type); > > if (!pnp.type.hardware_id) > - return; > + goto out; > > /* > * This relies on the fact that acpi_install_notify_handler() will not > @@ -1800,6 +1800,7 @@ static void acpi_scan_init_hotplug(acpi_handle handle, int type) > } > } > > +out: > acpi_free_pnp_ids(&pnp); > } > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
