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:	Fri, 9 May 2014 09:40:50 +0000
From:	"Opensource [Anthony Olech]" <anthony.olech.opensource@...semi.com>
To:	Himangi Saraogi <himangi774@...il.com>,
	Support Opensource <Support.Opensource@...semi.com>,
	"dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	"julia.lawall@...6.fr" <julia.lawall@...6.fr>,
	"Opensource [Anthony Olech]" <anthony.olech.opensource@...semi.com>
Subject: RE: [PATCH] Input: Introduce the use of managed version of kzalloc

> -----Original Message-----
> From: Himangi Saraogi [mailto:himangi774@...il.com]
> Sent: 08 May 2014 05:00
> To: Support Opensource; dmitry.torokhov@...il.com; linux-
> input@...r.kernel.org; linux-kernel@...r.kernel.org
> Cc: julia.lawall@...6.fr
> Subject: [PATCH] Input: Introduce the use of managed version of kzalloc
> This patch moves data allocated using kzalloc to managed data allocated
> using devm_kzalloc and cleans now unnecessary kfrees in probe and remove
> functions. This data is the third argument to da9052_request_irq in the two
> cases below.

The remainder of the description .....

> The following Coccinelle semantic patch was used for making the change:
> @platform@
> identifier p, probefn, removefn;
> @@
> struct platform_driver p = {
>   .probe = probefn,
>   .remove = removefn,
> };
> @prb@
> identifier platform.probefn, pdev;
> expression e, e1, e2;
> @@
> probefn(struct platform_device *pdev, ...) {
>   <+...
> - e = kzalloc(e1, e2)
> + e = devm_kzalloc(&pdev->dev, e1, e2)
>   ...
> ?-kfree(e);
>   ...+>
> }
> @rem depends on prb@
> identifier platform.removefn;
> expression e;
> @@
> removefn(...) {
>   <...
> - kfree(e);
>   ...>
> }

.... does not seems appropriate for the commit message, it should IMHO go in the following section

> Signed-off-by: Himangi Saraogi <himangi774@...il.com>
> Acked-by: Julia Lawall <julia.lawall@...6.fr>
> ---
> As a follow up patch I would like to know if it would be desirable to modify
> request_threaded_irq to devm_request_threaded_irq in the helper function
> da9052_request_irq :
> int da9052_request_irq(struct da9052 *da9052, int irq, char *name,
> 			   irq_handler_t handler, void *data) {
> 	irq = da9052_map_irq(da9052, irq);
> 	if (irq < 0)
> 		return irq;
> 	return request_threaded_irq(irq, NULL, handler,
> 				     IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> 				     name, data);
> }

The problem here is that there is no way back to the 'dev' associated with the irq.
To solve this requires a change to the function, or even better, just placing the function's
code inline, in which case a two stage approach is required:- first 'inline' the function
(from the common header file) to each of the PMIC's component drivers and second
raise a patch for each component driver do the change as you suggest which will
work because the correct 'dev' is available. Not that the second stage will have to
wait until the first statge is actually in main-line.

>  drivers/input/misc/da9052_onkey.c      | 4 +---
>  drivers/input/touchscreen/da9052_tsi.c | 4 +---
>  2 files changed, 2 insertions(+), 6 deletions(-)
> diff --git a/drivers/input/misc/da9052_onkey.c
> b/drivers/input/misc/da9052_onkey.c
> index 184c8f2..6fc8243 100644
> --- a/drivers/input/misc/da9052_onkey.c
> +++ b/drivers/input/misc/da9052_onkey.c
> @@ -84,7 +84,7 @@ static int da9052_onkey_probe(struct platform_device
> *pdev)
>  		return -EINVAL;
>  	}
> 
> -	onkey = kzalloc(sizeof(*onkey), GFP_KERNEL);
> +	onkey = devm_kzalloc(&pdev->dev, sizeof(*onkey), GFP_KERNEL);
>  	input_dev = input_allocate_device();
>  	if (!onkey || !input_dev) {
>  		dev_err(&pdev->dev, "Failed to allocate memory\n"); @@ -
> 126,7 +126,6 @@ err_free_irq:
>  	cancel_delayed_work_sync(&onkey->work);
>  err_free_mem:
>  	input_free_device(input_dev);
> -	kfree(onkey);
> 
>  	return error;
>  }
> @@ -139,7 +138,6 @@ static int da9052_onkey_remove(struct
> platform_device *pdev)
>  	cancel_delayed_work_sync(&onkey->work);
> 
>  	input_unregister_device(onkey->input);
> -	kfree(onkey);
> 
>  	return 0;
>  }
> diff --git a/drivers/input/touchscreen/da9052_tsi.c
> b/drivers/input/touchscreen/da9052_tsi.c
> index ab64d58..dff6a2e 100644
> --- a/drivers/input/touchscreen/da9052_tsi.c
> +++ b/drivers/input/touchscreen/da9052_tsi.c
> @@ -238,7 +238,7 @@ static int da9052_ts_probe(struct platform_device
> *pdev)
>  	if (!da9052)
>  		return -EINVAL;
> 
> -	tsi = kzalloc(sizeof(struct da9052_tsi), GFP_KERNEL);
> +	tsi = devm_kzalloc(&pdev->dev, sizeof(struct da9052_tsi),
> GFP_KERNEL);
>  	input_dev = input_allocate_device();
>  	if (!tsi || !input_dev) {
>  		error = -ENOMEM;
> @@ -311,7 +311,6 @@ err_free_datardy_irq:
>  err_free_pendwn_irq:
>  	da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
>  err_free_mem:
> -	kfree(tsi);
>  	input_free_device(input_dev);
> 
>  	return error;
> @@ -327,7 +326,6 @@ static int  da9052_ts_remove(struct platform_device
> *pdev)
>  	da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
> 
>  	input_unregister_device(tsi->dev);
> -	kfree(tsi);
> 
>  	return 0;
>  }
> --
> 1.9.1

for the content of this patch:

Acked-by: Opensource [Anthony Olech] <anthony.olech.opensource@...semi.com>

--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ