Messages in this thread | | | Date | Wed, 20 Oct 2021 07:21:50 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH] ptp: Fix possible memory leak in ptp_clock_register() |
| |
On Wed, 20 Oct 2021 16:18:34 +0800 Yang Yingliang wrote: > I got memory leak as follows when doing fault injection test: > > unreferenced object 0xffff88800906c618 (size 8): > comm "i2c-idt82p33931", pid 4421, jiffies 4294948083 (age 13.188s) > hex dump (first 8 bytes): > 70 74 70 30 00 00 00 00 ptp0.... > backtrace: > [<00000000312ed458>] __kmalloc_track_caller+0x19f/0x3a0 > [<0000000079f6e2ff>] kvasprintf+0xb5/0x150 > [<0000000026aae54f>] kvasprintf_const+0x60/0x190 > [<00000000f323a5f7>] kobject_set_name_vargs+0x56/0x150 > [<000000004e35abdd>] dev_set_name+0xc0/0x100 > [<00000000f20cfe25>] ptp_clock_register+0x9f4/0xd30 [ptp] > [<000000008bb9f0de>] idt82p33_probe.cold+0x8b6/0x1561 [ptp_idt82p33] > > When posix_clock_register() returns an error, the name allocated > in dev_set_name() will be leaked, the put_device() should be used > to give up the device reference, then the name will be freed in > kobject_cleanup() and other memory will be freed in ptp_clock_release(). > > Reported-by: Hulk Robot <hulkci@huawei.com> > Fixes: a33121e5487b ("ptp: fix the race between the release of ptp_clock and cdev") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c > index 4dfc52e06704..7fd02aabd79a 100644 > --- a/drivers/ptp/ptp_clock.c > +++ b/drivers/ptp/ptp_clock.c > @@ -283,15 +283,22 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info, > /* Create a posix clock and link it to the device. */ > err = posix_clock_register(&ptp->clock, &ptp->dev); > if (err) { > + if (ptp->pps_source) > + pps_unregister_source(ptp->pps_source); > + > + kfree(ptp->vclock_index);
I think the way ptp->vclock_index is freed is also buggy.
It's accessed from sysfs so it should be freed from the release function, not directly here or in ptp_clock_unregister(), right?
If that makes sense please submit a separate fix for the issue.
> + if (ptp->kworker) > + kthread_destroy_worker(ptp->kworker); > + > + put_device(&ptp->dev); > + > pr_err("failed to create posix clock\n"); > - goto no_clock; > + return ERR_PTR(err); > } > > return ptp; > > -no_clock: > - if (ptp->pps_source) > - pps_unregister_source(ptp->pps_source); > no_pps: > ptp_cleanup_pin_groups(ptp); > no_pin_groups:
| |