On 2022/11/17 18:49, Andy Shevchenko wrote:
On Thu, Nov 17, 2022 at 05:02:47PM +0800, Zeng Heng wrote:
Here is a report about memory leak detected in gpiochip_setup_dev():
unreferenced object 0xffff88810b406400 (size 512):
comm "python3", pid 1682, jiffies 4295346908 (age 24.090s)
backtrace:
kmalloc_trace
device_add device_private_init at drivers/base/core.c:3361
Seems like unneeded space after device_add. Also note, we refer to
the functions as func().
Just emphasize the location of memory leak happened.
(inlined by) device_add at drivers/base/core.c:3411
cdev_device_add
gpiolib_cdev_register
gpiochip_setup_dev
gpiochip_add_data_with_key
gcdev_register() & gcdev_unregister() would call device_add() &
device_del() (no matter CONFIG_GPIO_CDEV is enabled or not) to
register/unregister device.
However, if device_add() succeeds, some resource (like
struct device_private allocated by device_private_init())
is not released by device_del().
Therefore, after device_add() succeeds by gcdev_register(), it
needs to call put_device() to release resource in the error handle
path.
Here we move forward the register of release function, and let it
release every piece of resource by put_device() instead of kfree().
Fixes: 159f3cd92f17 ("gpiolib: Defer gpio device setup until after gpiolib initialization")
Signed-off-by: Zeng Heng <zengheng4@xxxxxxxxxx>
---
Where is changelog since we see this as v3?
...
change in v3:
- use put_device() instead of kfree()
err_free_gpiochip_mask:
gpiochip_remove_pin_ranges(gc);
gpiochip_free_valid_mask(gc);
+ /*
+ * If gdev->dev.release has been registered by
+ * gpiochip_setup_dev(), print err msg and
+ * call put_device() to release all.
+ */
+ if (gdev->dev.release)
+ goto err_free_gdev;
(1)
err_remove_from_list:
spin_lock_irqsave(&gpio_lock, flags);
list_del(&gdev->list);
...
- kfree(gdev);
+ if (gdev->dev.release)
+ put_device(&gdev->dev);
Why you can't do this above at (1)?
Is there any other hidden way to get here with release set?
As already mentioned in the mail, keep the error print info.
B.R.，
Zeng Heng
+ else
+ kfree(gdev);
return ret;
