linux-kernel - Re: [PATCH V1] input: fix memory leak in da9052 touchscreen driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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/