linux-kernel - [PATCH] drivers/staging/cptm1217/clearpad_tm1217.c: Correct call to input_free

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>] [day] [month] [year] [list]

Message-Id: <1305746443-694-1-git-send-email-julia@diku.dk>
Date:	Wed, 18 May 2011 21:20:43 +0200
From:	Julia Lawall <julia@...u.dk>
To:	Greg Kroah-Hartman <gregkh@...e.de>
Cc:	kernel-janitors@...r.kernel.org, Alan Cox <alan@...ux.intel.com>,
	Lucas De Marchi <lucas.demarchi@...fusion.mobi>,
	Ramesh Agarwal <ramesh.agarwal@...el.com>,
	Dan Carpenter <error27@...il.com>, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] drivers/staging/cptm1217/clearpad_tm1217.c: Correct call to input_free_device

From: Julia Lawall <julia@...u.dk>

This code is in a loop that currently is only executed once.  Because of
this property, the first block of code is currently actually correct.
Nevertheless, the comments associated with the code suggest that the loop
is planned to take more than one iteration in the future, and thus this
patch is made with that case in mind.

In the first block of code, there is currently an immediate abort from the
function.  It is changed to jump to the error handling code at fail, to be
able to unregister and free the resources allocated on previous iterations.

In the second block of code, the input_dev for the current iteration has
been allocated, but has not been registered.  It has also not been stored
in ts->cp_input_info[i].input.  Thus on jumping to fail, it will not be
freed.  In this case, we want to free, but not unregister, so the free for
this most recently allocated resource is put before the jump.

A simplified version of the semantic match that finds this problem is:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression struct input_dev * x;
expression ra,rr;
position p1,p2;
@@

x = input_allocate_device@p1(...)
...  when != x = rr
     when != input_free_device(x,...)
     when != if (...) { ... input_free_device(x,...) ...}
if(...) { ... when != x = ra
     when forall
     when != input_free_device(x,...)
 \(return <+...x...+>; \| return@.....; \) }

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

cocci.print_main("input_allocate_device",p1)
cocci.print_secs("input_free_device",p2)
// </smpl>

Signed-off-by: Julia Lawall <julia@...u.dk>

---
Only compile tested.

 drivers/staging/cptm1217/clearpad_tm1217.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/cptm1217/clearpad_tm1217.c b/drivers/staging/cptm1217/clearpad_tm1217.c
index 0fe713e..5456f82 100644
--- a/drivers/staging/cptm1217/clearpad_tm1217.c
+++ b/drivers/staging/cptm1217/clearpad_tm1217.c
@@ -462,8 +462,8 @@ static int cp_tm1217_probe(struct i2c_client *client,
 		if (input_dev == NULL) {
 			dev_err(ts->dev,
 				"cp_tm1217:Input Device Struct alloc failed\n");
-			kfree(ts);
-			return -ENOMEM;
+			retval = -ENOMEM;
+			goto fail;
 		}
 		input_info = &ts->cp_input_info[i];
 		snprintf(input_info->name, sizeof(input_info->name),
@@ -486,6 +486,7 @@ static int cp_tm1217_probe(struct i2c_client *client,
 			dev_err(ts->dev,
 				"Input dev registration failed for %s\n",
 					input_dev->name);
+			input_free_device(input_dev);
 			goto fail;
 		}
 		input_info->input = input_dev;

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