| Message ID | 201401081709.s08H9nMr038265@swsrvapps-02.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Anthony Olech <anthony.olech.opensource@diasemi.com> wrote: >The touchscreen component driver for the da9052/3 Dialog PMICs >leaks memory by not freeing the input device in the remove call. > >This patch matches the existing call to input_alloc_device() >in da9052_ts_probe() to a new call to input_free_device() in >da9052_ts_remove() > >Suggested-by: Huqiu Liu <liuhq11@mails.tsinghua.edu.cn> >Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com> >--- >This patch is relative to linux-next repository tag next-20140108 > >Many thanks to Huqiu Liu who found the bug. No, this is not a bug. Please refer to input API spec in input.h Thanks. > > drivers/input/touchscreen/da9052_tsi.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/drivers/input/touchscreen/da9052_tsi.c >b/drivers/input/touchscreen/da9052_tsi.c >index ab64d58..43a69d1 100644 >--- a/drivers/input/touchscreen/da9052_tsi.c >+++ b/drivers/input/touchscreen/da9052_tsi.c >@@ -320,6 +320,7 @@ err_free_mem: > static int da9052_ts_remove(struct platform_device *pdev) > { > struct da9052_tsi *tsi = platform_get_drvdata(pdev); >+ struct input_dev *input_dev = tsi->dev; > > da9052_reg_write(tsi->da9052, DA9052_LDO9_REG, 0x19); > >@@ -328,6 +329,7 @@ static int da9052_ts_remove(struct platform_device >*pdev) > > input_unregister_device(tsi->dev); > kfree(tsi); >+ input_free_device(input_dev); > > return 0; > }
> -----Original Message----- > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] > Sent: 08 January 2014 17:26 > To: Opensource [Anthony Olech] > Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Huqiu Liu; > David Dajun Chen > Subject: Re: [PATCH V1] input: fix memory leak in da9052 touchscreen driver > Anthony Olech <anthony.olech.opensource@diasemi.com> wrote: > >The touchscreen component driver for the da9052/3 Dialog PMICs leaks > >memory by not freeing the input device in the remove call. > >This patch matches the existing call to input_alloc_device() in > >da9052_ts_probe() to a new call to input_free_device() in > >da9052_ts_remove() > >Suggested-by: Huqiu Liu <liuhq11@mails.tsinghua.edu.cn> > >Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com> > >--- > >This patch is relative to linux-next repository tag next-20140108 > >Many thanks to Huqiu Liu who found the bug. > No, this is not a bug. Please refer to input API spec in input.h > Thanks. Hi Dmitry, the spec *seems* to say that if the allocation is done via devm_input_allocate_device(dev) then no explicit freeing must be done. However that is not the case here, so the bug exists. It does seem that there is an alternative way of resolving the issue, namely to: 1) change from allocate_device() to devm_input_allocate_device(dev) and 2) remove the exiosting input_free_device() from the error path in the probe() function I will update the patch submission to the alternative tomorrow Tony Olech (Dialog Semiconductor) > > drivers/input/touchscreen/da9052_tsi.c | 2 ++ > > 1 file changed, 2 insertions(+) > >diff --git a/drivers/input/touchscreen/da9052_tsi.c > >b/drivers/input/touchscreen/da9052_tsi.c > >index ab64d58..43a69d1 100644 > >--- a/drivers/input/touchscreen/da9052_tsi.c > >+++ b/drivers/input/touchscreen/da9052_tsi.c > >@@ -320,6 +320,7 @@ err_free_mem: > > static int da9052_ts_remove(struct platform_device *pdev) { > > struct da9052_tsi *tsi = platform_get_drvdata(pdev); > >+ struct input_dev *input_dev = tsi->dev; > > da9052_reg_write(tsi->da9052, DA9052_LDO9_REG, 0x19); > >@@ -328,6 +329,7 @@ static int da9052_ts_remove(struct > platform_device > >*pdev) > > input_unregister_device(tsi->dev); > > kfree(tsi); > >+ input_free_device(input_dev); > > return 0; > > } > -- > Dmitry
On Wed, Jan 08, 2014 at 06:12:27PM +0000, Opensource [Anthony Olech] wrote: > > -----Original Message----- > > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] > > Sent: 08 January 2014 17:26 > > To: Opensource [Anthony Olech] > > Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Huqiu Liu; > > David Dajun Chen > > Subject: Re: [PATCH V1] input: fix memory leak in da9052 touchscreen driver > > Anthony Olech <anthony.olech.opensource@diasemi.com> wrote: > > >The touchscreen component driver for the da9052/3 Dialog PMICs leaks > > >memory by not freeing the input device in the remove call. > > >This patch matches the existing call to input_alloc_device() in > > >da9052_ts_probe() to a new call to input_free_device() in > > >da9052_ts_remove() > > >Suggested-by: Huqiu Liu <liuhq11@mails.tsinghua.edu.cn> > > >Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com> > > >--- > > >This patch is relative to linux-next repository tag next-20140108 > > >Many thanks to Huqiu Liu who found the bug. > > No, this is not a bug. Please refer to input API spec in input.h > > Thanks. > > Hi Dmitry, > the spec *seems* to say that if the allocation is done via devm_input_allocate_device(dev) > then no explicit freeing must be done. > > However that is not the case here, so the bug exists. *sigh* /** * input_free_device - free memory occupied by input_dev structure * @dev: input device to free * * This function should only be used if input_register_device() * was not called yet or if it failed. Once device was registered * use input_unregister_device() and memory will be freed once last * reference to the device is dropped. THIS ^^^^^^^^^^^^^^^^^^^^ * * Device should be allocated by input_allocate_device(). * * NOTE: If there are references to the input device then memory * will not be freed until last reference is dropped. */ /** * input_allocate_device - allocate memory for new input device * * Returns prepared struct input_dev or %NULL. * * NOTE: Use input_free_device() to free devices that have not been * registered; input_unregister_device() should be used for already * registered devices. and this ^^^^^^^^^^^^^^^^^^^^^^^^ */ > > It does seem that there is an alternative way of resolving the issue, namely to: > 1) change from allocate_device() to devm_input_allocate_device(dev) and > 2) remove the exiosting input_free_device() from the error path in the probe() function There is no issue, but if you want to convert the driver to using managed resources that would be fine. Thanks.
diff --git a/drivers/input/touchscreen/da9052_tsi.c b/drivers/input/touchscreen/da9052_tsi.c index ab64d58..43a69d1 100644 --- a/drivers/input/touchscreen/da9052_tsi.c +++ b/drivers/input/touchscreen/da9052_tsi.c @@ -320,6 +320,7 @@ err_free_mem: static int da9052_ts_remove(struct platform_device *pdev) { struct da9052_tsi *tsi = platform_get_drvdata(pdev); + struct input_dev *input_dev = tsi->dev; da9052_reg_write(tsi->da9052, DA9052_LDO9_REG, 0x19); @@ -328,6 +329,7 @@ static int da9052_ts_remove(struct platform_device *pdev) input_unregister_device(tsi->dev); kfree(tsi); + input_free_device(input_dev); return 0; }
The touchscreen component driver for the da9052/3 Dialog PMICs leaks memory by not freeing the input device in the remove call. This patch matches the existing call to input_alloc_device() in da9052_ts_probe() to a new call to input_free_device() in da9052_ts_remove() Suggested-by: Huqiu Liu <liuhq11@mails.tsinghua.edu.cn> Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com> --- This patch is relative to linux-next repository tag next-20140108 Many thanks to Huqiu Liu who found the bug. drivers/input/touchscreen/da9052_tsi.c | 2 ++ 1 file changed, 2 insertions(+)