Re: [PATCH V1] input: fix memory leak in da9052 touchscreen driver
On Wed, Jan 08, 2014 at 06:12:27PM +, Opensource [Anthony Olech] wrote: > > -Original Message- > > From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] > > Sent: 08 January 2014 17:26 > > To: Opensource [Anthony Olech] > > Cc: linux-in...@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 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 > > >Signed-off-by: Anthony Olech > > >--- > > >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. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH V1] input: fix memory leak in da9052 touchscreen driver
> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: 08 January 2014 17:26
> To: Opensource [Anthony Olech]
> Cc: linux-in...@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 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
> >Signed-off-by: Anthony Olech
> >---
> >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
Re: [PATCH V1] input: fix memory leak in da9052 touchscreen driver
Anthony Olech 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
>Signed-off-by: Anthony Olech
>---
>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;
> }
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V1] input: fix memory leak in da9052 touchscreen driver
Anthony Olech anthony.olech.opensou...@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 liuh...@mails.tsinghua.edu.cn
Signed-off-by: Anthony Olech anthony.olech.opensou...@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;
}
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH V1] input: fix memory leak in da9052 touchscreen driver
-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
Sent: 08 January 2014 17:26
To: Opensource [Anthony Olech]
Cc: linux-in...@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.opensou...@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 liuh...@mails.tsinghua.edu.cn
Signed-off-by: Anthony Olech anthony.olech.opensou...@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
Re: [PATCH V1] input: fix memory leak in da9052 touchscreen driver
On Wed, Jan 08, 2014 at 06:12:27PM +, Opensource [Anthony Olech] wrote: -Original Message- From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] Sent: 08 January 2014 17:26 To: Opensource [Anthony Olech] Cc: linux-in...@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.opensou...@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 liuh...@mails.tsinghua.edu.cn Signed-off-by: Anthony Olech anthony.olech.opensou...@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. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
