[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20140108191229.GA25785@core.coreip.homeip.net>
Date: Wed, 8 Jan 2014 11:12:29 -0800
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: "Opensource [Anthony Olech]" <anthony.olech.opensource@...semi.com>
Cc: "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Huqiu Liu <liuhq11@...ls.tsinghua.edu.cn>,
David Dajun Chen <david.chen@...semi.com>
Subject: Re: [PATCH V1] input: fix memory leak in da9052 touchscreen driver
On Wed, Jan 08, 2014 at 06:12:27PM +0000, Opensource [Anthony Olech] wrote:
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@...il.com]
> > Sent: 08 January 2014 17:26
> > To: Opensource [Anthony Olech]
> > Cc: linux-input@...r.kernel.org; linux-kernel@...r.kernel.org; Huqiu Liu;
> > David Dajun Chen
> > Subject: Re: [PATCH V1] input: fix memory leak in da9052 touchscreen driver
> > Anthony Olech <anthony.olech.opensource@...semi.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@...ls.tsinghua.edu.cn>
> > >Signed-off-by: Anthony Olech <anthony.olech.opensource@...semi.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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists