Hi Douglas, Please always CC the mailing list. I'm not the only person working on this code. On Sun, Oct 08, 2023 at 06:59:19PM -0400, Douglas Gilbert wrote: > Hi, > I was debugging something else in lk 6.6.0-rc1 and was tailing > /var/log/syslog and noticed several: > kmemleak: 6 new suspected memory leaks > messages so I had a look and saw many of these: > > unreferenced object 0xffff8882943a4df8 (size 8): > comm "kworker/u32:41", pid 73935, jiffies 4437924777 (age 6489.122s) > hex dump (first 8 bytes): > 70 64 35 00 82 88 ff ff pd5..... > backtrace: > [<ffffffff812d247c>] __kmalloc_node_track_caller+0x4c/0x150 > [<ffffffff815c8585>] kvasprintf+0x65/0xd0 > [<ffffffff81b1b56c>] kobject_set_name_vargs+0x1c/0x90 > [<ffffffff8178f30e>] dev_set_name+0x4e/0x70 > [<ffffffffa05f78b4>] usb_power_delivery_register+0x84/0xe0 [typec] > [<ffffffffa0748112>] ucsi_register_partner_pdos+0x62/0x190 [typec_ucsi] > [<ffffffffa07464a8>] ucsi_poll_worker+0x38/0x110 [typec_ucsi] > [<ffffffff810a9d48>] process_one_work+0x1d8/0x4b0 > [<ffffffff810ab149>] worker_thread+0x1c9/0x3b0 > [<ffffffff810b60d2>] kthread+0xf2/0x130 > [<ffffffff8102d588>] ret_from_fork+0x28/0x40 > [<ffffffff81001aeb>] ret_from_fork_asm+0x1b/0x30 > > and similar variants involving UCSI. Looking at ucsi_register_partner_pdos() > there seems to be a leak of con->partner_pd if one other the other functions > that depend on it fail. If it was my code, I would replace most of the > returns in that function with 'goto err;' branches which would then call > usb_power_delivery_unregister(con->partner_pd); No, partner_pd object isn't there only for the PDOs. We need it even when PDO details are not supported by the interface (PDO details are optional in UCSI). I don't see any leak here. All these object, including partner_pd, are unregistered in the end as they should, no? The function looks a bit funny because it has to always try to read both sink and source PDOs (UCSI does not tell us what is available beforehand). We can probable refactor the code a bit to make it more understandable. The actual registration of the PDOs should be split into separate function and this one should be renamed to ucsi_register_partner_pd(). thanks, -- heikki
