On Sat, Jan 30, 2021 at 4:45 AM quanyang.wang <quanyang.w...@windriver.com> wrote: > > Hi Andy, > > On 1/30/21 1:26 AM, Andy Shevchenko wrote: > > On Fri, Jan 29, 2021 at 2:01 PM <quanyang.w...@windriver.com> wrote: > >> From: Quanyang Wang <quanyang.w...@windriver.com> > >> > >> In gpiochip_add_data_with_key, we should check the return value of > >> dev_set_name to ensure that device name is allocated successfully > >> and then add a label on the error path to free device name to fix > >> kmemleak as below: > > Thanks for the report. > > Unfortunately... > > > >> + ret = dev_set_name(&gdev->dev, GPIOCHIP_NAME "%d", gdev->id); > >> + if (ret) > >> + goto err_free_ida; > > ... > > > >> +err_free_dev_name: > >> + kfree(dev_name(&gdev->dev)); > > ...this approach seems to create a possible double free if I'm not > > mistaken. > Thanks for your comment. I didn't catch the double free. Would you > please point it out? > > > > The idea is that device name should be cleaned in kobject ->release() > > callback when device is put. > > Yes, the device name should be freed by calling put_device(&gdev->dev). > But int gpiochip_add_data_with_key, > > when running dev_set_name, "gdev->dev.release" hasn't been installed > until in the tail of gpiochip_add_data_with_key. > > So we couldn't call put_device here. > > Any suggestion is much appreciated. > > Thanks, > > Quanyang > > > Can you elaborate? > >
Andy, gdev->dev.release is assigned as the very last step in gpiochip_add_data_with_key() so the patch looks correct to me. Do you still have objections? Maybe I'm not seeing something. Bart