* [PATCH] gpiolib: fix memory leak in gpiochip_setup_dev @ 2022-11-08 11:53 Zeng Heng 2022-11-08 16:57 ` Kent Gibson 0 siblings, 1 reply; 16+ messages in thread From: Zeng Heng @ 2022-11-08 11:53 UTC (permalink / raw) To: warthog618, linus.walleij, brgl; +Cc: zengheng4, liwei391, linux-gpio gcdev_register & gcdev_unregister call device_add & device_del to request/release source. But in device_add, the dev->p allocated by device_private_init is not released by device_del. So when calling gcdev_unregister to release gdev, it needs put_device to release dev in the following. Otherwise, kmemleak would report memory leak such as below: unreferenced object 0xffff88810b406400 (size 512): comm "python3", pid 1682, jiffies 4295346908 (age 24.090s) hex dump (first 32 bytes): 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... ff ff ff ff ff ff ff ff a0 5e 23 90 ff ff ff ff .........^#..... backtrace: [<00000000a58ee5fe>] kmalloc_trace+0x22/0x110 [<0000000045fe2058>] device_add+0xb34/0x1130 [<00000000d778b45f>] cdev_device_add+0x83/0xe0 [<0000000089f948ed>] gpiolib_cdev_register+0x73/0xa0 [<00000000a3a8a316>] gpiochip_setup_dev+0x1c/0x70 [<00000000787227b4>] gpiochip_add_data_with_key+0x10f6/0x1bf0 [<000000009ac5742c>] devm_gpiochip_add_data_with_key+0x2e/0x80 [<00000000bf2b23d9>] xra1403_probe+0x192/0x1b0 [gpio_xra1403] [<000000005b5ef2d4>] spi_probe+0xe1/0x140 [<000000002b26f6f1>] really_probe+0x17c/0x3f0 [<00000000dd2dad9c>] __driver_probe_device+0xe3/0x170 [<000000005ca60d2a>] device_driver_attach+0x34/0x80 [<00000000e9db90db>] bind_store+0x10b/0x1a0 [<00000000e2650f8a>] drv_attr_store+0x49/0x70 [<0000000080a80b2b>] sysfs_kf_write+0x8c/0xb0 [<00000000a28b45b9>] kernfs_fop_write_iter+0x216/0x2e0 unreferenced object 0xffff888100de9800 (size 512): comm "python3", pid 264, jiffies 4294737615 (age 33.514s) hex dump (first 32 bytes): 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... ff ff ff ff ff ff ff ff a0 5e 63 a1 ff ff ff ff .........^c..... backtrace: [<00000000bcc571d0>] kmalloc_trace+0x22/0x110 [<00000000eeb06124>] device_add+0xb34/0x1130 [<000000007e5cd2fd>] cdev_device_add+0x83/0xe0 [<000000008f6bcd3a>] gpiolib_cdev_register+0x73/0xa0 [<0000000012c93b24>] gpiochip_setup_dev+0x1c/0x70 [<00000000a24b646a>] gpiochip_add_data_with_key+0x10f6/0x1bf0 [<000000000c225212>] tpic2810_probe+0x16e/0x196 [gpio_tpic2810] [<00000000b52d04ff>] i2c_device_probe+0x651/0x680 [<0000000058d3ff6b>] really_probe+0x17c/0x3f0 [<00000000586f43d3>] __driver_probe_device+0xe3/0x170 [<000000003f428602>] device_driver_attach+0x34/0x80 [<0000000040e91a1b>] bind_store+0x10b/0x1a0 [<00000000c1d990b9>] drv_attr_store+0x49/0x70 [<00000000a23bfc22>] sysfs_kf_write+0x8c/0xb0 [<00000000064e6572>] kernfs_fop_write_iter+0x216/0x2e0 [<00000000026ce093>] vfs_write+0x658/0x810 Because at the point of gpiochip_setup_dev here, where dev.release does not set yet, calling put_device would cause the warning of no release function and double-free in the following fault handler route (when kfree dev_name). So directly calling kfree to release dev->p here in case of memory leak. Fixes: 1f5eb8b17f02 ("gpiolib: fix sysfs when cdev is not selected") Signed-off-by: Zeng Heng <zengheng4@huawei.com> --- drivers/gpio/gpiolib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 4756ea08894f..fa659af86d07 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -539,6 +539,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) err_remove_device: gcdev_unregister(gdev); + kfree(gdev->dev.p); return ret; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] gpiolib: fix memory leak in gpiochip_setup_dev 2022-11-08 11:53 [PATCH] gpiolib: fix memory leak in gpiochip_setup_dev Zeng Heng @ 2022-11-08 16:57 ` Kent Gibson 2022-11-09 5:12 ` Kent Gibson 0 siblings, 1 reply; 16+ messages in thread From: Kent Gibson @ 2022-11-08 16:57 UTC (permalink / raw) To: Zeng Heng; +Cc: linus.walleij, brgl, liwei391, linux-gpio On Tue, Nov 08, 2022 at 07:53:24PM +0800, Zeng Heng wrote: > gcdev_register & gcdev_unregister call device_add & device_del to > request/release source. But in device_add, the dev->p allocated by > device_private_init is not released by device_del. > > So when calling gcdev_unregister to release gdev, it needs put_device > to release dev in the following. > > Otherwise, kmemleak would report memory leak such as below: > > unreferenced object 0xffff88810b406400 (size 512): > comm "python3", pid 1682, jiffies 4295346908 (age 24.090s) > hex dump (first 32 bytes): > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > ff ff ff ff ff ff ff ff a0 5e 23 90 ff ff ff ff .........^#..... > backtrace: > [<00000000a58ee5fe>] kmalloc_trace+0x22/0x110 > [<0000000045fe2058>] device_add+0xb34/0x1130 > [<00000000d778b45f>] cdev_device_add+0x83/0xe0 > [<0000000089f948ed>] gpiolib_cdev_register+0x73/0xa0 > [<00000000a3a8a316>] gpiochip_setup_dev+0x1c/0x70 > [<00000000787227b4>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > [<000000009ac5742c>] devm_gpiochip_add_data_with_key+0x2e/0x80 > [<00000000bf2b23d9>] xra1403_probe+0x192/0x1b0 [gpio_xra1403] > [<000000005b5ef2d4>] spi_probe+0xe1/0x140 > [<000000002b26f6f1>] really_probe+0x17c/0x3f0 > [<00000000dd2dad9c>] __driver_probe_device+0xe3/0x170 > [<000000005ca60d2a>] device_driver_attach+0x34/0x80 > [<00000000e9db90db>] bind_store+0x10b/0x1a0 > [<00000000e2650f8a>] drv_attr_store+0x49/0x70 > [<0000000080a80b2b>] sysfs_kf_write+0x8c/0xb0 > [<00000000a28b45b9>] kernfs_fop_write_iter+0x216/0x2e0 > > unreferenced object 0xffff888100de9800 (size 512): > comm "python3", pid 264, jiffies 4294737615 (age 33.514s) > hex dump (first 32 bytes): > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > ff ff ff ff ff ff ff ff a0 5e 63 a1 ff ff ff ff .........^c..... > backtrace: > [<00000000bcc571d0>] kmalloc_trace+0x22/0x110 > [<00000000eeb06124>] device_add+0xb34/0x1130 > [<000000007e5cd2fd>] cdev_device_add+0x83/0xe0 > [<000000008f6bcd3a>] gpiolib_cdev_register+0x73/0xa0 > [<0000000012c93b24>] gpiochip_setup_dev+0x1c/0x70 > [<00000000a24b646a>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > [<000000000c225212>] tpic2810_probe+0x16e/0x196 [gpio_tpic2810] > [<00000000b52d04ff>] i2c_device_probe+0x651/0x680 > [<0000000058d3ff6b>] really_probe+0x17c/0x3f0 > [<00000000586f43d3>] __driver_probe_device+0xe3/0x170 > [<000000003f428602>] device_driver_attach+0x34/0x80 > [<0000000040e91a1b>] bind_store+0x10b/0x1a0 > [<00000000c1d990b9>] drv_attr_store+0x49/0x70 > [<00000000a23bfc22>] sysfs_kf_write+0x8c/0xb0 > [<00000000064e6572>] kernfs_fop_write_iter+0x216/0x2e0 > [<00000000026ce093>] vfs_write+0x658/0x810 > > Because at the point of gpiochip_setup_dev here, where dev.release > does not set yet, calling put_device would cause the warning of > no release function and double-free in the following fault handler > route (when kfree dev_name). So directly calling kfree to release > dev->p here in case of memory leak. > > Fixes: 1f5eb8b17f02 ("gpiolib: fix sysfs when cdev is not selected") I'm confused. You say "gcdev_register & gcdev_unregister call device_add & device_del" - which is only the case when CONFIG_GPIO_CDEV is not set. But your trace shows CONFIG_GPIO_CDEV is set, as otherwise gpiolib_cdev_register() would not exist. Can you clarify the configuration in which you are seeing the problem? Assuming CONFIG_GPIO_CDEV is NOT set: Provide a more appropriate trace. From a reading of the device_add() documentation, there is a problem if the device_add() fails - in that case put_device() should be called - and it is not, instead gpiochip_setup_dev() returns immediately - not going via the err_remove_device path where your fix is??. The correct fix for that would be to change the gcdev_register() to call put_device() if device_add() fails. Cheers, Kent. > Signed-off-by: Zeng Heng <zengheng4@huawei.com> > --- > drivers/gpio/gpiolib.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 4756ea08894f..fa659af86d07 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -539,6 +539,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) > > err_remove_device: > gcdev_unregister(gdev); > + kfree(gdev->dev.p); > return ret; > } > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] gpiolib: fix memory leak in gpiochip_setup_dev 2022-11-08 16:57 ` Kent Gibson @ 2022-11-09 5:12 ` Kent Gibson 2022-11-09 8:27 ` Zeng Heng 0 siblings, 1 reply; 16+ messages in thread From: Kent Gibson @ 2022-11-09 5:12 UTC (permalink / raw) To: Zeng Heng; +Cc: linus.walleij, brgl, liwei391, linux-gpio On Wed, Nov 09, 2022 at 12:57:48AM +0800, Kent Gibson wrote: > On Tue, Nov 08, 2022 at 07:53:24PM +0800, Zeng Heng wrote: > > gcdev_register & gcdev_unregister call device_add & device_del to > > request/release source. But in device_add, the dev->p allocated by > > device_private_init is not released by device_del. > > > > So when calling gcdev_unregister to release gdev, it needs put_device > > to release dev in the following. > > > > Otherwise, kmemleak would report memory leak such as below: > > > > unreferenced object 0xffff88810b406400 (size 512): > > comm "python3", pid 1682, jiffies 4295346908 (age 24.090s) > > hex dump (first 32 bytes): > > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > > ff ff ff ff ff ff ff ff a0 5e 23 90 ff ff ff ff .........^#..... > > backtrace: > > [<00000000a58ee5fe>] kmalloc_trace+0x22/0x110 > > [<0000000045fe2058>] device_add+0xb34/0x1130 > > [<00000000d778b45f>] cdev_device_add+0x83/0xe0 > > [<0000000089f948ed>] gpiolib_cdev_register+0x73/0xa0 > > [<00000000a3a8a316>] gpiochip_setup_dev+0x1c/0x70 > > [<00000000787227b4>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > > [<000000009ac5742c>] devm_gpiochip_add_data_with_key+0x2e/0x80 > > [<00000000bf2b23d9>] xra1403_probe+0x192/0x1b0 [gpio_xra1403] > > [<000000005b5ef2d4>] spi_probe+0xe1/0x140 > > [<000000002b26f6f1>] really_probe+0x17c/0x3f0 > > [<00000000dd2dad9c>] __driver_probe_device+0xe3/0x170 > > [<000000005ca60d2a>] device_driver_attach+0x34/0x80 > > [<00000000e9db90db>] bind_store+0x10b/0x1a0 > > [<00000000e2650f8a>] drv_attr_store+0x49/0x70 > > [<0000000080a80b2b>] sysfs_kf_write+0x8c/0xb0 > > [<00000000a28b45b9>] kernfs_fop_write_iter+0x216/0x2e0 > > > > unreferenced object 0xffff888100de9800 (size 512): > > comm "python3", pid 264, jiffies 4294737615 (age 33.514s) > > hex dump (first 32 bytes): > > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > > ff ff ff ff ff ff ff ff a0 5e 63 a1 ff ff ff ff .........^c..... > > backtrace: > > [<00000000bcc571d0>] kmalloc_trace+0x22/0x110 > > [<00000000eeb06124>] device_add+0xb34/0x1130 > > [<000000007e5cd2fd>] cdev_device_add+0x83/0xe0 > > [<000000008f6bcd3a>] gpiolib_cdev_register+0x73/0xa0 > > [<0000000012c93b24>] gpiochip_setup_dev+0x1c/0x70 > > [<00000000a24b646a>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > > [<000000000c225212>] tpic2810_probe+0x16e/0x196 [gpio_tpic2810] > > [<00000000b52d04ff>] i2c_device_probe+0x651/0x680 > > [<0000000058d3ff6b>] really_probe+0x17c/0x3f0 > > [<00000000586f43d3>] __driver_probe_device+0xe3/0x170 > > [<000000003f428602>] device_driver_attach+0x34/0x80 > > [<0000000040e91a1b>] bind_store+0x10b/0x1a0 > > [<00000000c1d990b9>] drv_attr_store+0x49/0x70 > > [<00000000a23bfc22>] sysfs_kf_write+0x8c/0xb0 > > [<00000000064e6572>] kernfs_fop_write_iter+0x216/0x2e0 > > [<00000000026ce093>] vfs_write+0x658/0x810 > > > > Because at the point of gpiochip_setup_dev here, where dev.release > > does not set yet, calling put_device would cause the warning of > > no release function and double-free in the following fault handler > > route (when kfree dev_name). So directly calling kfree to release > > dev->p here in case of memory leak. > > > > Fixes: 1f5eb8b17f02 ("gpiolib: fix sysfs when cdev is not selected") > > I'm confused. You say "gcdev_register & gcdev_unregister call device_add > & device_del" - which is only the case when CONFIG_GPIO_CDEV is not set. > > But your trace shows CONFIG_GPIO_CDEV is set, as otherwise > gpiolib_cdev_register() would not exist. > > Can you clarify the configuration in which you are seeing the problem? > > Assuming CONFIG_GPIO_CDEV is NOT set: > > Provide a more appropriate trace. > > From a reading of the device_add() documentation, there is a problem if > the device_add() fails - in that case put_device() should be called - and > it is not, instead gpiochip_setup_dev() returns immediately - not going > via the err_remove_device path where your fix is??. > The correct fix for that would be to change the gcdev_register() to call > put_device() if device_add() fails. > Ignore that - as you mentioned the dev.release hasn't been set at that point. Having another look at this, I don't think the problem is related to the Fixed commit at all - it looks more general. How did you conclude that that commit introduced the problem? Is it easily repeatable and have you bisected for it? Cheers, Kent. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] gpiolib: fix memory leak in gpiochip_setup_dev 2022-11-09 5:12 ` Kent Gibson @ 2022-11-09 8:27 ` Zeng Heng 2022-11-09 9:06 ` Kent Gibson 0 siblings, 1 reply; 16+ messages in thread From: Zeng Heng @ 2022-11-09 8:27 UTC (permalink / raw) To: Kent Gibson; +Cc: linus.walleij, brgl, liwei391, linux-gpio On 2022/11/9 13:12, Kent Gibson wrote: > On Wed, Nov 09, 2022 at 12:57:48AM +0800, Kent Gibson wrote: >> On Tue, Nov 08, 2022 at 07:53:24PM +0800, Zeng Heng wrote: >>> gcdev_register & gcdev_unregister call device_add & device_del to >>> request/release source. But in device_add, the dev->p allocated by >>> device_private_init is not released by device_del. >>> >>> So when calling gcdev_unregister to release gdev, it needs put_device >>> to release dev in the following. >>> >>> Otherwise, kmemleak would report memory leak such as below: >>> >>> unreferenced object 0xffff88810b406400 (size 512): >>> comm "python3", pid 1682, jiffies 4295346908 (age 24.090s) >>> hex dump (first 32 bytes): >>> 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... >>> ff ff ff ff ff ff ff ff a0 5e 23 90 ff ff ff ff .........^#..... >>> backtrace: >>> [<00000000a58ee5fe>] kmalloc_trace+0x22/0x110 >>> [<0000000045fe2058>] device_add+0xb34/0x1130 >>> [<00000000d778b45f>] cdev_device_add+0x83/0xe0 >>> [<0000000089f948ed>] gpiolib_cdev_register+0x73/0xa0 >>> [<00000000a3a8a316>] gpiochip_setup_dev+0x1c/0x70 >>> [<00000000787227b4>] gpiochip_add_data_with_key+0x10f6/0x1bf0 >>> [<000000009ac5742c>] devm_gpiochip_add_data_with_key+0x2e/0x80 >>> [<00000000bf2b23d9>] xra1403_probe+0x192/0x1b0 [gpio_xra1403] >>> [<000000005b5ef2d4>] spi_probe+0xe1/0x140 >>> [<000000002b26f6f1>] really_probe+0x17c/0x3f0 >>> [<00000000dd2dad9c>] __driver_probe_device+0xe3/0x170 >>> [<000000005ca60d2a>] device_driver_attach+0x34/0x80 >>> [<00000000e9db90db>] bind_store+0x10b/0x1a0 >>> [<00000000e2650f8a>] drv_attr_store+0x49/0x70 >>> [<0000000080a80b2b>] sysfs_kf_write+0x8c/0xb0 >>> [<00000000a28b45b9>] kernfs_fop_write_iter+0x216/0x2e0 >>> >>> unreferenced object 0xffff888100de9800 (size 512): >>> comm "python3", pid 264, jiffies 4294737615 (age 33.514s) >>> hex dump (first 32 bytes): >>> 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... >>> ff ff ff ff ff ff ff ff a0 5e 63 a1 ff ff ff ff .........^c..... >>> backtrace: >>> [<00000000bcc571d0>] kmalloc_trace+0x22/0x110 >>> [<00000000eeb06124>] device_add+0xb34/0x1130 >>> [<000000007e5cd2fd>] cdev_device_add+0x83/0xe0 >>> [<000000008f6bcd3a>] gpiolib_cdev_register+0x73/0xa0 >>> [<0000000012c93b24>] gpiochip_setup_dev+0x1c/0x70 >>> [<00000000a24b646a>] gpiochip_add_data_with_key+0x10f6/0x1bf0 >>> [<000000000c225212>] tpic2810_probe+0x16e/0x196 [gpio_tpic2810] >>> [<00000000b52d04ff>] i2c_device_probe+0x651/0x680 >>> [<0000000058d3ff6b>] really_probe+0x17c/0x3f0 >>> [<00000000586f43d3>] __driver_probe_device+0xe3/0x170 >>> [<000000003f428602>] device_driver_attach+0x34/0x80 >>> [<0000000040e91a1b>] bind_store+0x10b/0x1a0 >>> [<00000000c1d990b9>] drv_attr_store+0x49/0x70 >>> [<00000000a23bfc22>] sysfs_kf_write+0x8c/0xb0 >>> [<00000000064e6572>] kernfs_fop_write_iter+0x216/0x2e0 >>> [<00000000026ce093>] vfs_write+0x658/0x810 >>> >>> Because at the point of gpiochip_setup_dev here, where dev.release >>> does not set yet, calling put_device would cause the warning of >>> no release function and double-free in the following fault handler >>> route (when kfree dev_name). So directly calling kfree to release >>> dev->p here in case of memory leak. >>> >>> Fixes: 1f5eb8b17f02 ("gpiolib: fix sysfs when cdev is not selected") >> I'm confused. You say "gcdev_register & gcdev_unregister call device_add >> & device_del" - which is only the case when CONFIG_GPIO_CDEV is not set. >> >> But your trace shows CONFIG_GPIO_CDEV is set, as otherwise >> gpiolib_cdev_register() would not exist. >> >> Can you clarify the configuration in which you are seeing the problem? >> >> Assuming CONFIG_GPIO_CDEV is NOT set: >> >> Provide a more appropriate trace. >> >> From a reading of the device_add() documentation, there is a problem if >> the device_add() fails - in that case put_device() should be called - and >> it is not, instead gpiochip_setup_dev() returns immediately - not going >> via the err_remove_device path where your fix is??. >> The correct fix for that would be to change the gcdev_register() to call >> put_device() if device_add() fails. >> > Ignore that - as you mentioned the dev.release hasn't been set at that > point. > > Having another look at this, I don't think the problem is related to the > Fixed commit at all - it looks more general. > How did you conclude that that commit introduced the problem? > Is it easily repeatable and have you bisected for it? > > Cheers, > Kent. > Thanks to your patience for taking another look at this mail. And allow me to claim that I indeed do the inject-fault test and regression test based on the patch. My test environment has included CONFIG_GPIO_CDEV config, but no matter includes this config or not, there is still memory leak in fault handle route about dev->p because of calling device_add. Apologize for the fixed commit is not accurate, and exactly it's this one: Fixes: 159f3cd92f17 ("gpiolib: Defer gpio device setup until after gpiolib initialization") If everything is all right, I would send the second version patch and correct fix tag. thanks all, Zeng Heng ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] gpiolib: fix memory leak in gpiochip_setup_dev 2022-11-09 8:27 ` Zeng Heng @ 2022-11-09 9:06 ` Kent Gibson 2022-11-09 9:31 ` [PATCH v2] " Zeng Heng 0 siblings, 1 reply; 16+ messages in thread From: Kent Gibson @ 2022-11-09 9:06 UTC (permalink / raw) To: Zeng Heng; +Cc: linus.walleij, brgl, liwei391, linux-gpio On Wed, Nov 09, 2022 at 04:27:26PM +0800, Zeng Heng wrote: > > On 2022/11/9 13:12, Kent Gibson wrote: > > On Wed, Nov 09, 2022 at 12:57:48AM +0800, Kent Gibson wrote: > > > On Tue, Nov 08, 2022 at 07:53:24PM +0800, Zeng Heng wrote: > > > > gcdev_register & gcdev_unregister call device_add & device_del to > > > > request/release source. But in device_add, the dev->p allocated by > > > > device_private_init is not released by device_del. > > > > > > > > So when calling gcdev_unregister to release gdev, it needs put_device > > > > to release dev in the following. > > > > > > > > Otherwise, kmemleak would report memory leak such as below: > > > > > > > > unreferenced object 0xffff88810b406400 (size 512): > > > > comm "python3", pid 1682, jiffies 4295346908 (age 24.090s) > > > > hex dump (first 32 bytes): > > > > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > > > > ff ff ff ff ff ff ff ff a0 5e 23 90 ff ff ff ff .........^#..... > > > > backtrace: > > > > [<00000000a58ee5fe>] kmalloc_trace+0x22/0x110 > > > > [<0000000045fe2058>] device_add+0xb34/0x1130 > > > > [<00000000d778b45f>] cdev_device_add+0x83/0xe0 > > > > [<0000000089f948ed>] gpiolib_cdev_register+0x73/0xa0 > > > > [<00000000a3a8a316>] gpiochip_setup_dev+0x1c/0x70 > > > > [<00000000787227b4>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > > > > [<000000009ac5742c>] devm_gpiochip_add_data_with_key+0x2e/0x80 > > > > [<00000000bf2b23d9>] xra1403_probe+0x192/0x1b0 [gpio_xra1403] > > > > [<000000005b5ef2d4>] spi_probe+0xe1/0x140 > > > > [<000000002b26f6f1>] really_probe+0x17c/0x3f0 > > > > [<00000000dd2dad9c>] __driver_probe_device+0xe3/0x170 > > > > [<000000005ca60d2a>] device_driver_attach+0x34/0x80 > > > > [<00000000e9db90db>] bind_store+0x10b/0x1a0 > > > > [<00000000e2650f8a>] drv_attr_store+0x49/0x70 > > > > [<0000000080a80b2b>] sysfs_kf_write+0x8c/0xb0 > > > > [<00000000a28b45b9>] kernfs_fop_write_iter+0x216/0x2e0 > > > > > > > > unreferenced object 0xffff888100de9800 (size 512): > > > > comm "python3", pid 264, jiffies 4294737615 (age 33.514s) > > > > hex dump (first 32 bytes): > > > > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > > > > ff ff ff ff ff ff ff ff a0 5e 63 a1 ff ff ff ff .........^c..... > > > > backtrace: > > > > [<00000000bcc571d0>] kmalloc_trace+0x22/0x110 > > > > [<00000000eeb06124>] device_add+0xb34/0x1130 > > > > [<000000007e5cd2fd>] cdev_device_add+0x83/0xe0 > > > > [<000000008f6bcd3a>] gpiolib_cdev_register+0x73/0xa0 > > > > [<0000000012c93b24>] gpiochip_setup_dev+0x1c/0x70 > > > > [<00000000a24b646a>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > > > > [<000000000c225212>] tpic2810_probe+0x16e/0x196 [gpio_tpic2810] > > > > [<00000000b52d04ff>] i2c_device_probe+0x651/0x680 > > > > [<0000000058d3ff6b>] really_probe+0x17c/0x3f0 > > > > [<00000000586f43d3>] __driver_probe_device+0xe3/0x170 > > > > [<000000003f428602>] device_driver_attach+0x34/0x80 > > > > [<0000000040e91a1b>] bind_store+0x10b/0x1a0 > > > > [<00000000c1d990b9>] drv_attr_store+0x49/0x70 > > > > [<00000000a23bfc22>] sysfs_kf_write+0x8c/0xb0 > > > > [<00000000064e6572>] kernfs_fop_write_iter+0x216/0x2e0 > > > > [<00000000026ce093>] vfs_write+0x658/0x810 > > > > > > > > Because at the point of gpiochip_setup_dev here, where dev.release > > > > does not set yet, calling put_device would cause the warning of > > > > no release function and double-free in the following fault handler > > > > route (when kfree dev_name). So directly calling kfree to release > > > > dev->p here in case of memory leak. > > > > > > > > Fixes: 1f5eb8b17f02 ("gpiolib: fix sysfs when cdev is not selected") > > > I'm confused. You say "gcdev_register & gcdev_unregister call device_add > > > & device_del" - which is only the case when CONFIG_GPIO_CDEV is not set. > > > > > > But your trace shows CONFIG_GPIO_CDEV is set, as otherwise > > > gpiolib_cdev_register() would not exist. > > > > > > Can you clarify the configuration in which you are seeing the problem? > > > > > > Assuming CONFIG_GPIO_CDEV is NOT set: > > > > > > Provide a more appropriate trace. > > > > > > From a reading of the device_add() documentation, there is a problem if > > > the device_add() fails - in that case put_device() should be called - and > > > it is not, instead gpiochip_setup_dev() returns immediately - not going > > > via the err_remove_device path where your fix is??. > > > The correct fix for that would be to change the gcdev_register() to call > > > put_device() if device_add() fails. > > > > > Ignore that - as you mentioned the dev.release hasn't been set at that > > point. > > > > Having another look at this, I don't think the problem is related to the > > Fixed commit at all - it looks more general. > > How did you conclude that that commit introduced the problem? > > Is it easily repeatable and have you bisected for it? > > > > Cheers, > > Kent. > > Thanks to your patience for taking another look at this mail. > No problem - my bad for initially responding without giving it more consideration. > And allow me to claim that I indeed do the inject-fault test and regression > test based on the patch. > > > My test environment has included CONFIG_GPIO_CDEV config, but no matter > includes this config or not, > > there is still memory leak in fault handle route about dev->p because of > calling device_add. > > > Apologize for the fixed commit is not accurate, and exactly it's this one: > > Fixes: 159f3cd92f17 ("gpiolib: Defer gpio device setup until after gpiolib > initialization") > > > If everything is all right, I would send the second version patch and > correct fix tag. > Yeah, that makes more sense - it has been there since that section of code was reworked - quite some time ago. Good catch. Cheers, Kent. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] gpiolib: fix memory leak in gpiochip_setup_dev 2022-11-09 9:06 ` Kent Gibson @ 2022-11-09 9:31 ` Zeng Heng 2022-11-09 14:47 ` Andy Shevchenko 0 siblings, 1 reply; 16+ messages in thread From: Zeng Heng @ 2022-11-09 9:31 UTC (permalink / raw) To: warthog618, linus.walleij, brgl, linux; +Cc: linux-gpio, liwei391, yuancan gcdev_register & gcdev_unregister call device_add & device_del to request/release source. But in device_add, the dev->p allocated by device_private_init is not released by device_del. So when calling gcdev_unregister to release gdev, it needs put_device to release dev in the following. Otherwise, kmemleak would report memory leak such as below: unreferenced object 0xffff88810b406400 (size 512): comm "python3", pid 1682, jiffies 4295346908 (age 24.090s) hex dump (first 32 bytes): 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... ff ff ff ff ff ff ff ff a0 5e 23 90 ff ff ff ff .........^#..... backtrace: [<00000000a58ee5fe>] kmalloc_trace+0x22/0x110 [<0000000045fe2058>] device_add+0xb34/0x1130 [<00000000d778b45f>] cdev_device_add+0x83/0xe0 [<0000000089f948ed>] gpiolib_cdev_register+0x73/0xa0 [<00000000a3a8a316>] gpiochip_setup_dev+0x1c/0x70 [<00000000787227b4>] gpiochip_add_data_with_key+0x10f6/0x1bf0 [<000000009ac5742c>] devm_gpiochip_add_data_with_key+0x2e/0x80 [<00000000bf2b23d9>] xra1403_probe+0x192/0x1b0 [gpio_xra1403] [<000000005b5ef2d4>] spi_probe+0xe1/0x140 [<000000002b26f6f1>] really_probe+0x17c/0x3f0 [<00000000dd2dad9c>] __driver_probe_device+0xe3/0x170 [<000000005ca60d2a>] device_driver_attach+0x34/0x80 [<00000000e9db90db>] bind_store+0x10b/0x1a0 [<00000000e2650f8a>] drv_attr_store+0x49/0x70 [<0000000080a80b2b>] sysfs_kf_write+0x8c/0xb0 [<00000000a28b45b9>] kernfs_fop_write_iter+0x216/0x2e0 unreferenced object 0xffff888100de9800 (size 512): comm "python3", pid 264, jiffies 4294737615 (age 33.514s) hex dump (first 32 bytes): 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... ff ff ff ff ff ff ff ff a0 5e 63 a1 ff ff ff ff .........^c..... backtrace: [<00000000bcc571d0>] kmalloc_trace+0x22/0x110 [<00000000eeb06124>] device_add+0xb34/0x1130 [<000000007e5cd2fd>] cdev_device_add+0x83/0xe0 [<000000008f6bcd3a>] gpiolib_cdev_register+0x73/0xa0 [<0000000012c93b24>] gpiochip_setup_dev+0x1c/0x70 [<00000000a24b646a>] gpiochip_add_data_with_key+0x10f6/0x1bf0 [<000000000c225212>] tpic2810_probe+0x16e/0x196 [gpio_tpic2810] [<00000000b52d04ff>] i2c_device_probe+0x651/0x680 [<0000000058d3ff6b>] really_probe+0x17c/0x3f0 [<00000000586f43d3>] __driver_probe_device+0xe3/0x170 [<000000003f428602>] device_driver_attach+0x34/0x80 [<0000000040e91a1b>] bind_store+0x10b/0x1a0 [<00000000c1d990b9>] drv_attr_store+0x49/0x70 [<00000000a23bfc22>] sysfs_kf_write+0x8c/0xb0 [<00000000064e6572>] kernfs_fop_write_iter+0x216/0x2e0 [<00000000026ce093>] vfs_write+0x658/0x810 Because at the point of gpiochip_setup_dev here, where dev.release does not set yet, calling put_device would cause the warning of no release function and double-free in the following fault handler route (when kfree dev_name). So directly calling kfree to release dev->p here in case of memory leak. Fixes: 159f3cd92f17 ("gpiolib: Defer gpio device setup until after gpiolib initialization") Signed-off-by: Zeng Heng <zengheng4@huawei.com> --- changes in v2: - correct fix tag --- drivers/gpio/gpiolib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 4756ea08894f..fa659af86d07 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -539,6 +539,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) err_remove_device: gcdev_unregister(gdev); + kfree(gdev->dev.p); return ret; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] gpiolib: fix memory leak in gpiochip_setup_dev 2022-11-09 9:31 ` [PATCH v2] " Zeng Heng @ 2022-11-09 14:47 ` Andy Shevchenko 2022-11-10 1:26 ` Kent Gibson 0 siblings, 1 reply; 16+ messages in thread From: Andy Shevchenko @ 2022-11-09 14:47 UTC (permalink / raw) To: Zeng Heng Cc: warthog618, linus.walleij, brgl, linux, linux-gpio, liwei391, yuancan On Wed, Nov 09, 2022 at 05:31:20PM +0800, Zeng Heng wrote: > gcdev_register & gcdev_unregister call device_add & device_del to > request/release source. But in device_add, the dev->p allocated by > device_private_init is not released by device_del. First of all, we refer to the functions like func(). > So when calling gcdev_unregister to release gdev, it needs put_device > to release dev in the following. > > Otherwise, kmemleak would report memory leak such as below: > > unreferenced object 0xffff88810b406400 (size 512): > comm "python3", pid 1682, jiffies 4295346908 (age 24.090s) > hex dump (first 32 bytes): > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > ff ff ff ff ff ff ff ff a0 5e 23 90 ff ff ff ff .........^#..... > backtrace: Second, read Submitting Patches on how to provide your backtraces in the message body. > [<00000000a58ee5fe>] kmalloc_trace+0x22/0x110 > [<0000000045fe2058>] device_add+0xb34/0x1130 > [<00000000d778b45f>] cdev_device_add+0x83/0xe0 > [<0000000089f948ed>] gpiolib_cdev_register+0x73/0xa0 > [<00000000a3a8a316>] gpiochip_setup_dev+0x1c/0x70 > [<00000000787227b4>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > [<000000009ac5742c>] devm_gpiochip_add_data_with_key+0x2e/0x80 > [<00000000bf2b23d9>] xra1403_probe+0x192/0x1b0 [gpio_xra1403] > [<000000005b5ef2d4>] spi_probe+0xe1/0x140 > [<000000002b26f6f1>] really_probe+0x17c/0x3f0 > [<00000000dd2dad9c>] __driver_probe_device+0xe3/0x170 > [<000000005ca60d2a>] device_driver_attach+0x34/0x80 > [<00000000e9db90db>] bind_store+0x10b/0x1a0 > [<00000000e2650f8a>] drv_attr_store+0x49/0x70 > [<0000000080a80b2b>] sysfs_kf_write+0x8c/0xb0 > [<00000000a28b45b9>] kernfs_fop_write_iter+0x216/0x2e0 > > unreferenced object 0xffff888100de9800 (size 512): > comm "python3", pid 264, jiffies 4294737615 (age 33.514s) > hex dump (first 32 bytes): > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > ff ff ff ff ff ff ff ff a0 5e 63 a1 ff ff ff ff .........^c..... > backtrace: > [<00000000bcc571d0>] kmalloc_trace+0x22/0x110 > [<00000000eeb06124>] device_add+0xb34/0x1130 > [<000000007e5cd2fd>] cdev_device_add+0x83/0xe0 > [<000000008f6bcd3a>] gpiolib_cdev_register+0x73/0xa0 > [<0000000012c93b24>] gpiochip_setup_dev+0x1c/0x70 > [<00000000a24b646a>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > [<000000000c225212>] tpic2810_probe+0x16e/0x196 [gpio_tpic2810] > [<00000000b52d04ff>] i2c_device_probe+0x651/0x680 > [<0000000058d3ff6b>] really_probe+0x17c/0x3f0 > [<00000000586f43d3>] __driver_probe_device+0xe3/0x170 > [<000000003f428602>] device_driver_attach+0x34/0x80 > [<0000000040e91a1b>] bind_store+0x10b/0x1a0 > [<00000000c1d990b9>] drv_attr_store+0x49/0x70 > [<00000000a23bfc22>] sysfs_kf_write+0x8c/0xb0 > [<00000000064e6572>] kernfs_fop_write_iter+0x216/0x2e0 > [<00000000026ce093>] vfs_write+0x658/0x810 > > Because at the point of gpiochip_setup_dev here, where dev.release > does not set yet, calling put_device would cause the warning of > no release function and double-free in the following fault handler > route (when kfree dev_name). So directly calling kfree to release > dev->p here in case of memory leak. ... > @@ -539,6 +539,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) > > err_remove_device: > gcdev_unregister(gdev); > + kfree(gdev->dev.p); > return ret; Third, I do not believe it's a correct fix. Have you read comments around device_del() / etc.? NAK. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] gpiolib: fix memory leak in gpiochip_setup_dev 2022-11-09 14:47 ` Andy Shevchenko @ 2022-11-10 1:26 ` Kent Gibson 2022-11-10 2:36 ` Zeng Heng 0 siblings, 1 reply; 16+ messages in thread From: Kent Gibson @ 2022-11-10 1:26 UTC (permalink / raw) To: Zeng Heng Cc: Andy Shevchenko, linus.walleij, brgl, linux, linux-gpio, liwei391, yuancan On Wed, Nov 09, 2022 at 04:47:08PM +0200, Andy Shevchenko wrote: > On Wed, Nov 09, 2022 at 05:31:20PM +0800, Zeng Heng wrote: > > gcdev_register & gcdev_unregister call device_add & device_del to > > request/release source. But in device_add, the dev->p allocated by > > device_private_init is not released by device_del. > > First of all, we refer to the functions like func(). > Further to this, the description of the problem could be clearer - it would be helpful to indicate the code path that triggers the problem - it is gpiochip_sysfs_register() returning an error? > > So when calling gcdev_unregister to release gdev, it needs put_device > > to release dev in the following. > > > > Otherwise, kmemleak would report memory leak such as below: > > > > unreferenced object 0xffff88810b406400 (size 512): > > comm "python3", pid 1682, jiffies 4295346908 (age 24.090s) > > hex dump (first 32 bytes): > > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > > ff ff ff ff ff ff ff ff a0 5e 23 90 ff ff ff ff .........^#..... > > backtrace: > > Second, read Submitting Patches on how to provide your backtraces in the > message body. > > > [<00000000a58ee5fe>] kmalloc_trace+0x22/0x110 > > [<0000000045fe2058>] device_add+0xb34/0x1130 > > [<00000000d778b45f>] cdev_device_add+0x83/0xe0 > > [<0000000089f948ed>] gpiolib_cdev_register+0x73/0xa0 > > [<00000000a3a8a316>] gpiochip_setup_dev+0x1c/0x70 > > [<00000000787227b4>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > > [<000000009ac5742c>] devm_gpiochip_add_data_with_key+0x2e/0x80 > > [<00000000bf2b23d9>] xra1403_probe+0x192/0x1b0 [gpio_xra1403] > > [<000000005b5ef2d4>] spi_probe+0xe1/0x140 > > [<000000002b26f6f1>] really_probe+0x17c/0x3f0 > > [<00000000dd2dad9c>] __driver_probe_device+0xe3/0x170 > > [<000000005ca60d2a>] device_driver_attach+0x34/0x80 > > [<00000000e9db90db>] bind_store+0x10b/0x1a0 > > [<00000000e2650f8a>] drv_attr_store+0x49/0x70 > > [<0000000080a80b2b>] sysfs_kf_write+0x8c/0xb0 > > [<00000000a28b45b9>] kernfs_fop_write_iter+0x216/0x2e0 > > > > unreferenced object 0xffff888100de9800 (size 512): > > comm "python3", pid 264, jiffies 4294737615 (age 33.514s) > > hex dump (first 32 bytes): > > 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... > > ff ff ff ff ff ff ff ff a0 5e 63 a1 ff ff ff ff .........^c..... > > backtrace: > > [<00000000bcc571d0>] kmalloc_trace+0x22/0x110 > > [<00000000eeb06124>] device_add+0xb34/0x1130 > > [<000000007e5cd2fd>] cdev_device_add+0x83/0xe0 > > [<000000008f6bcd3a>] gpiolib_cdev_register+0x73/0xa0 > > [<0000000012c93b24>] gpiochip_setup_dev+0x1c/0x70 > > [<00000000a24b646a>] gpiochip_add_data_with_key+0x10f6/0x1bf0 > > [<000000000c225212>] tpic2810_probe+0x16e/0x196 [gpio_tpic2810] > > [<00000000b52d04ff>] i2c_device_probe+0x651/0x680 > > [<0000000058d3ff6b>] really_probe+0x17c/0x3f0 > > [<00000000586f43d3>] __driver_probe_device+0xe3/0x170 > > [<000000003f428602>] device_driver_attach+0x34/0x80 > > [<0000000040e91a1b>] bind_store+0x10b/0x1a0 > > [<00000000c1d990b9>] drv_attr_store+0x49/0x70 > > [<00000000a23bfc22>] sysfs_kf_write+0x8c/0xb0 > > [<00000000064e6572>] kernfs_fop_write_iter+0x216/0x2e0 > > [<00000000026ce093>] vfs_write+0x658/0x810 > > > > Because at the point of gpiochip_setup_dev here, where dev.release > > does not set yet, calling put_device would cause the warning of > > no release function and double-free in the following fault handler > > route (when kfree dev_name). So directly calling kfree to release > > dev->p here in case of memory leak. > Again, this could be clearer. The dev->p is normally freed by device_release() - why is that not happening in this case? (as put_device() is never called in this path) The double free you see if you do call put_device() appears to be due to different expectations as to the cleanup that gpiochip_setup_dev() will perform on error, depending on where it is called. gpiochip_setup_devs() assumes any cleanup is performed by gpiochip_setup_dev(), while gpiochip_add_data_with_key() assumes that it hasn't performed any cleanup. Having gpiochip_setup_dev() perform its own cleanup makes the most sense to me, so gpiochip_add_data_with_key() should be changed to allow for that. > ... > > > @@ -539,6 +539,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) > > > > err_remove_device: > > gcdev_unregister(gdev); > > + kfree(gdev->dev.p); > > return ret; > > Third, I do not believe it's a correct fix. > Have you read comments around device_del() / etc.? > I agree - this is not the correct fix. The correct fix is to trigger the normal cleanup mechanism, so put_device(). The fact that that triggers a warning: "Device '%s' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n" is an indicator that dev.release should be set earlier. If gpiodevice_release() is not appropriate, or cannot be modified to deal with the device state at that point, then an appropriate interim release function should be set. And, as mentioned above, gpiochip_add_data_with_key() needs to be modified to allow for gpiochip_setup_dev() cleaning up its own mess. That is my take, but that is just from perusing the code so I may be totally off base. Either way, an ACK/NACK on this from a maintainer or other gpiolib expert would be helpful to expiditing a solution. Cheers, Kent. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] gpiolib: fix memory leak in gpiochip_setup_dev 2022-11-10 1:26 ` Kent Gibson @ 2022-11-10 2:36 ` Zeng Heng 2022-11-17 9:02 ` [PATCH v3] gpiolib: fix memory leak in gpiochip_setup_dev() Zeng Heng 0 siblings, 1 reply; 16+ messages in thread From: Zeng Heng @ 2022-11-10 2:36 UTC (permalink / raw) To: Kent Gibson, Andy Shevchenko, linux Cc: linus.walleij, brgl, linux-gpio, liwei391, yuancan, Linux Kernel Mailing List On 2022/11/10 9:26, Kent Gibson wrote: > On Wed, Nov 09, 2022 at 04:47:08PM +0200, Andy Shevchenko wrote: >> On Wed, Nov 09, 2022 at 05:31:20PM +0800, Zeng Heng wrote: >>> gcdev_register & gcdev_unregister call device_add & device_del to >>> request/release source. But in device_add, the dev->p allocated by >>> device_private_init is not released by device_del. >> First of all, we refer to the functions like func(). Thanks, it would be updated in next version. > Further to this, the description of the problem could be clearer - > it would be helpful to indicate the code path that triggers the problem > - it is gpiochip_sysfs_register() returning an error? > >>> So when calling gcdev_unregister to release gdev, it needs put_device >>> to release dev in the following. >>> >>> Otherwise, kmemleak would report memory leak such as below: >>> >>> unreferenced object 0xffff88810b406400 (size 512): >>> comm "python3", pid 1682, jiffies 4295346908 (age 24.090s) >>> hex dump (first 32 bytes): >>> 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... >>> ff ff ff ff ff ff ff ff a0 5e 23 90 ff ff ff ff .........^#..... >>> backtrace: >> Second, read Submitting Patches on how to provide your backtraces in the >> message body. Thanks, it would be updated in next version. >> >>> [<00000000a58ee5fe>] kmalloc_trace+0x22/0x110 >>> [<0000000045fe2058>] device_add+0xb34/0x1130 >>> [<00000000d778b45f>] cdev_device_add+0x83/0xe0 >>> [<0000000089f948ed>] gpiolib_cdev_register+0x73/0xa0 >>> [<00000000a3a8a316>] gpiochip_setup_dev+0x1c/0x70 >>> [<00000000787227b4>] gpiochip_add_data_with_key+0x10f6/0x1bf0 >>> [<000000009ac5742c>] devm_gpiochip_add_data_with_key+0x2e/0x80 >>> [<00000000bf2b23d9>] xra1403_probe+0x192/0x1b0 [gpio_xra1403] >>> [<000000005b5ef2d4>] spi_probe+0xe1/0x140 >>> [<000000002b26f6f1>] really_probe+0x17c/0x3f0 >>> [<00000000dd2dad9c>] __driver_probe_device+0xe3/0x170 >>> [<000000005ca60d2a>] device_driver_attach+0x34/0x80 >>> [<00000000e9db90db>] bind_store+0x10b/0x1a0 >>> [<00000000e2650f8a>] drv_attr_store+0x49/0x70 >>> [<0000000080a80b2b>] sysfs_kf_write+0x8c/0xb0 >>> [<00000000a28b45b9>] kernfs_fop_write_iter+0x216/0x2e0 >>> >>> unreferenced object 0xffff888100de9800 (size 512): >>> comm "python3", pid 264, jiffies 4294737615 (age 33.514s) >>> hex dump (first 32 bytes): >>> 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N.......... >>> ff ff ff ff ff ff ff ff a0 5e 63 a1 ff ff ff ff .........^c..... >>> backtrace: >>> [<00000000bcc571d0>] kmalloc_trace+0x22/0x110 >>> [<00000000eeb06124>] device_add+0xb34/0x1130 >>> [<000000007e5cd2fd>] cdev_device_add+0x83/0xe0 >>> [<000000008f6bcd3a>] gpiolib_cdev_register+0x73/0xa0 >>> [<0000000012c93b24>] gpiochip_setup_dev+0x1c/0x70 >>> [<00000000a24b646a>] gpiochip_add_data_with_key+0x10f6/0x1bf0 >>> [<000000000c225212>] tpic2810_probe+0x16e/0x196 [gpio_tpic2810] >>> [<00000000b52d04ff>] i2c_device_probe+0x651/0x680 >>> [<0000000058d3ff6b>] really_probe+0x17c/0x3f0 >>> [<00000000586f43d3>] __driver_probe_device+0xe3/0x170 >>> [<000000003f428602>] device_driver_attach+0x34/0x80 >>> [<0000000040e91a1b>] bind_store+0x10b/0x1a0 >>> [<00000000c1d990b9>] drv_attr_store+0x49/0x70 >>> [<00000000a23bfc22>] sysfs_kf_write+0x8c/0xb0 >>> [<00000000064e6572>] kernfs_fop_write_iter+0x216/0x2e0 >>> [<00000000026ce093>] vfs_write+0x658/0x810 >>> >>> Because at the point of gpiochip_setup_dev here, where dev.release >>> does not set yet, calling put_device would cause the warning of >>> no release function and double-free in the following fault handler >>> route (when kfree dev_name). So directly calling kfree to release >>> dev->p here in case of memory leak. > Again, this could be clearer. The dev->p is normally freed by > device_release() - why is that not happening in this case? > (as put_device() is never called in this path) > > The double free you see if you do call put_device() appears to be due to > different expectations as to the cleanup that gpiochip_setup_dev() will > perform on error, depending on where it is called. gpiochip_setup_devs() > assumes any cleanup is performed by gpiochip_setup_dev(), while > gpiochip_add_data_with_key() assumes that it hasn't performed any cleanup. > > Having gpiochip_setup_dev() perform its own cleanup makes the most sense > to me, so gpiochip_add_data_with_key() should be changed to allow for > that. Right, the cleanup route of gpiochip_add_data_with_key() & gpiochip_setup_dev() has to be considered comprehensively after any possible cases of fault injections. >> ... >> >>> @@ -539,6 +539,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) >>> >>> err_remove_device: >>> gcdev_unregister(gdev); >>> + kfree(gdev->dev.p); >>> return ret; >> Third, I do not believe it's a correct fix. >> Have you read comments around device_del() / etc.? Yes, not only the comments I read, but also the device_del() implement code. Releasing the dev->p pointer is not the business with device_del(), but it's relied on put_device() calling release function. Turning back here, the release function is not set yet at this point, there is a gap between device_add() and set release function pointer. That's the reason why choose to free dev->p explicitly as the mail mentioned above. > I agree - this is not the correct fix. The correct fix is to trigger the > normal cleanup mechanism, so put_device(). > The fact that that triggers a warning: > > "Device '%s' does not have a release() function, it is broken and must be > fixed. See Documentation/core-api/kobject.rst.\n" > > is an indicator that dev.release should be set earlier. > If gpiodevice_release() is not appropriate, or cannot be modified to deal > with the device state at that point, then an appropriate interim release > function should be set. > > And, as mentioned above, gpiochip_add_data_with_key() needs to be modified > to allow for gpiochip_setup_dev() cleaning up its own mess. > > That is my take, but that is just from perusing the code so I may be > totally off base. Either way, an ACK/NACK on this from a maintainer or > other gpiolib expert would be helpful to expiditing a solution. > > Cheers, > Kent. Yes, exactly. Thanks to all, Zeng Heng ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] gpiolib: fix memory leak in gpiochip_setup_dev() 2022-11-10 2:36 ` Zeng Heng @ 2022-11-17 9:02 ` Zeng Heng 2022-11-17 10:49 ` Andy Shevchenko 0 siblings, 1 reply; 16+ messages in thread From: Zeng Heng @ 2022-11-17 9:02 UTC (permalink / raw) To: brgl, linux, linus.walleij, warthog618, andriy.shevchenko Cc: linux-gpio, zengheng4, liwei391 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 (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@huawei.com> --- drivers/gpio/gpiolib.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 4756ea08894f..bb4dedc154b7 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -526,12 +526,13 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) if (ret) return ret; + /* From this point, the .release() function cleans up gpio_device */ + gdev->dev.release = gpiodevice_release; + ret = gpiochip_sysfs_register(gdev); if (ret) goto err_remove_device; - /* From this point, the .release() function cleans up gpio_device */ - gdev->dev.release = gpiodevice_release; dev_dbg(&gdev->dev, "registered GPIOs %d to %d on %s\n", gdev->base, gdev->base + gdev->ngpio - 1, gdev->chip->label ? : "generic"); @@ -816,6 +817,13 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 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; err_remove_from_list: spin_lock_irqsave(&gpio_lock, flags); list_del(&gdev->list); @@ -835,7 +843,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, gdev->base, gdev->base + gdev->ngpio - 1, gc->label ? : "generic", ret); } - kfree(gdev); + if (gdev->dev.release) + put_device(&gdev->dev); + else + kfree(gdev); return ret; } EXPORT_SYMBOL_GPL(gpiochip_add_data_with_key); -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3] gpiolib: fix memory leak in gpiochip_setup_dev() 2022-11-17 9:02 ` [PATCH v3] gpiolib: fix memory leak in gpiochip_setup_dev() Zeng Heng @ 2022-11-17 10:49 ` Andy Shevchenko 2022-11-17 14:12 ` Zeng Heng 0 siblings, 1 reply; 16+ messages in thread From: Andy Shevchenko @ 2022-11-17 10:49 UTC (permalink / raw) To: Zeng Heng; +Cc: brgl, linux, linus.walleij, warthog618, linux-gpio, liwei391 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(). > (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@huawei.com> > --- Where is changelog since we see this as v3? ... > 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? > + else > + kfree(gdev); > return ret; -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] gpiolib: fix memory leak in gpiochip_setup_dev() 2022-11-17 10:49 ` Andy Shevchenko @ 2022-11-17 14:12 ` Zeng Heng 2022-11-17 15:31 ` Andy Shevchenko 0 siblings, 1 reply; 16+ messages in thread From: Zeng Heng @ 2022-11-17 14:12 UTC (permalink / raw) To: Andy Shevchenko Cc: brgl, linux, linus.walleij, warthog618, linux-gpio, liwei391 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@huawei.com> >> --- > 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; ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] gpiolib: fix memory leak in gpiochip_setup_dev() 2022-11-17 14:12 ` Zeng Heng @ 2022-11-17 15:31 ` Andy Shevchenko 2022-11-18 2:22 ` [PATCH v4] " Zeng Heng 2022-11-18 2:31 ` [PATCH v3] " Zeng Heng 0 siblings, 2 replies; 16+ messages in thread From: Andy Shevchenko @ 2022-11-17 15:31 UTC (permalink / raw) To: Zeng Heng; +Cc: brgl, linux, linus.walleij, warthog618, linux-gpio, liwei391 On Thu, Nov 17, 2022 at 10:12:31PM +0800, Zeng Heng wrote: > On 2022/11/17 18:49, Andy Shevchenko wrote: > > On Thu, Nov 17, 2022 at 05:02:47PM +0800, Zeng Heng wrote: ... > > > + /* > > > + * 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. Can you refactor that to avoid double condition on the ->release() presence? > > > + else > > > + kfree(gdev); > > > return ret; -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] gpiolib: fix memory leak in gpiochip_setup_dev() 2022-11-17 15:31 ` Andy Shevchenko @ 2022-11-18 2:22 ` Zeng Heng 2022-11-18 10:28 ` Andy Shevchenko 2022-11-18 2:31 ` [PATCH v3] " Zeng Heng 1 sibling, 1 reply; 16+ messages in thread From: Zeng Heng @ 2022-11-18 2:22 UTC (permalink / raw) To: brgl, linux, linus.walleij, warthog618, andriy.shevchenko Cc: linux-gpio, zengheng4, liwei391 Here is a backtrace 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 (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@huawei.com> --- changes in v4: - add gpiochip_print_register_fail() changes in v3: - use put_device() instead of kfree() explicitly changes in v2: - correct fixes tag --- drivers/gpio/gpiolib.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 4756ea08894f..c7a55f4f7ef6 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -526,12 +526,13 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) if (ret) return ret; + /* From this point, the .release() function cleans up gpio_device */ + gdev->dev.release = gpiodevice_release; + ret = gpiochip_sysfs_register(gdev); if (ret) goto err_remove_device; - /* From this point, the .release() function cleans up gpio_device */ - gdev->dev.release = gpiodevice_release; dev_dbg(&gdev->dev, "registered GPIOs %d to %d on %s\n", gdev->base, gdev->base + gdev->ngpio - 1, gdev->chip->label ? : "generic"); @@ -590,6 +591,18 @@ static void gpiochip_setup_devs(void) } } +static void gpiochip_print_register_fail(struct gpio_chip *gc, + struct gpio_device *gdev, + const char *func, int ret) +{ + /* failures here can mean systems won't boot... */ + if (ret != -EPROBE_DEFER) { + pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", func, + gdev->base, gdev->base + gdev->ngpio - 1, + gc->label ? : "generic", ret); + } +} + int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, struct lock_class_key *lock_key, struct lock_class_key *request_key) @@ -816,6 +829,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, err_free_gpiochip_mask: gpiochip_remove_pin_ranges(gc); gpiochip_free_valid_mask(gc); + if (gdev->dev.release) { + /* release() has been registered by gpiochip_setup_dev() */ + gpiochip_print_register_fail(gc, gdev, __func__, ret); + put_device(&gdev->dev); + return ret; + } err_remove_from_list: spin_lock_irqsave(&gpio_lock, flags); list_del(&gdev->list); @@ -829,12 +848,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, err_free_ida: ida_free(&gpio_ida, gdev->id); err_free_gdev: - /* failures here can mean systems won't boot... */ - if (ret != -EPROBE_DEFER) { - pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", __func__, - gdev->base, gdev->base + gdev->ngpio - 1, - gc->label ? : "generic", ret); - } + gpiochip_print_register_fail(gc, gdev, __func__, ret); kfree(gdev); return ret; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4] gpiolib: fix memory leak in gpiochip_setup_dev() 2022-11-18 2:22 ` [PATCH v4] " Zeng Heng @ 2022-11-18 10:28 ` Andy Shevchenko 0 siblings, 0 replies; 16+ messages in thread From: Andy Shevchenko @ 2022-11-18 10:28 UTC (permalink / raw) To: Zeng Heng; +Cc: brgl, linux, linus.walleij, warthog618, linux-gpio, liwei391 On Fri, Nov 18, 2022 at 10:22:36AM +0800, Zeng Heng wrote: > Here is a backtrace 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 You missed my comment here about extra space. > (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(). ... > +static void gpiochip_print_register_fail(struct gpio_chip *gc, > + struct gpio_device *gdev, > + const char *func, int ret) > +{ > + /* failures here can mean systems won't boot... */ > + if (ret != -EPROBE_DEFER) { Wouldn't the following work better? if (ret == -EPROBE_DEFER) return; /* failures here can mean systems won't boot... */ pr_err(...); > + pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", func, > + gdev->base, gdev->base + gdev->ngpio - 1, > + gc->label ? : "generic", ret); > + } > +} ... > err_free_gpiochip_mask: > gpiochip_remove_pin_ranges(gc); > gpiochip_free_valid_mask(gc); > + if (gdev->dev.release) { > + /* release() has been registered by gpiochip_setup_dev() */ This comment is most probably in a wrong line and should be one line below. > + gpiochip_print_register_fail(gc, gdev, __func__, ret); > + put_device(&gdev->dev); > + return ret; > + } ... > err_free_gdev: > - /* failures here can mean systems won't boot... */ > - if (ret != -EPROBE_DEFER) { > - pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", __func__, > - gdev->base, gdev->base + gdev->ngpio - 1, > - gc->label ? : "generic", ret); > - } > + gpiochip_print_register_fail(gc, gdev, __func__, ret); > kfree(gdev); > return ret; Now it looks cleaner, but why you can't use the same return point with the message? What you need is to neep the range on the stack (which is almost there). ... Okay, let's leave this for a while, I will think how it can be improved and then I come up with particular suggestions. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] gpiolib: fix memory leak in gpiochip_setup_dev() 2022-11-17 15:31 ` Andy Shevchenko 2022-11-18 2:22 ` [PATCH v4] " Zeng Heng @ 2022-11-18 2:31 ` Zeng Heng 1 sibling, 0 replies; 16+ messages in thread From: Zeng Heng @ 2022-11-18 2:31 UTC (permalink / raw) To: Andy Shevchenko Cc: brgl, linux, linus.walleij, warthog618, linux-gpio, liwei391 On 2022/11/17 23:31, Andy Shevchenko wrote: > On Thu, Nov 17, 2022 at 10:12:31PM +0800, Zeng Heng wrote: >> On 2022/11/17 18:49, Andy Shevchenko wrote: >>> On Thu, Nov 17, 2022 at 05:02:47PM +0800, Zeng Heng wrote: > ... > >>>> + /* >>>> + * 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. > Can you refactor that to avoid double condition on the ->release() presence? Sure. Zeng Heng > >>>> + else >>>> + kfree(gdev); >>>> return ret; ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-11-18 10:28 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-08 11:53 [PATCH] gpiolib: fix memory leak in gpiochip_setup_dev Zeng Heng 2022-11-08 16:57 ` Kent Gibson 2022-11-09 5:12 ` Kent Gibson 2022-11-09 8:27 ` Zeng Heng 2022-11-09 9:06 ` Kent Gibson 2022-11-09 9:31 ` [PATCH v2] " Zeng Heng 2022-11-09 14:47 ` Andy Shevchenko 2022-11-10 1:26 ` Kent Gibson 2022-11-10 2:36 ` Zeng Heng 2022-11-17 9:02 ` [PATCH v3] gpiolib: fix memory leak in gpiochip_setup_dev() Zeng Heng 2022-11-17 10:49 ` Andy Shevchenko 2022-11-17 14:12 ` Zeng Heng 2022-11-17 15:31 ` Andy Shevchenko 2022-11-18 2:22 ` [PATCH v4] " Zeng Heng 2022-11-18 10:28 ` Andy Shevchenko 2022-11-18 2:31 ` [PATCH v3] " Zeng Heng
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.