Thread (10 messages) flat view 10 messages, 5 authors, 2011-11-01

Re: [PATCH 3/5 v2] drivers/hid/hid-roccat.c: eliminate a null pointer dereference

From: Jiri Kosina <hidden>
Date: 2011-10-29 17:00:28
Also in: kernel-janitors, lkml

On Sat, 29 Oct 2011, Julia Lawall wrote:
quoted hunk ↗ jump to hunk
From: Julia Lawall <redacted>

It is not possible to take the lock in device if device is NULL.
The mutex_lock is thus moved after the NULL test, and the relevant part of
the shared error handling code is moved up.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r@
expression E, E1;
identifier f;
statement S1,S2,S3;
@@

if (E == NULL)
{
  ... when != if (E == NULL || ...) S1 else S2
      when != E = E1
*E->f
  ... when any
  return ...;
}
else S3
// </smpl>

Signed-off-by: Julia Lawall <redacted>

---
mutex_lock changed to mutex_unlock in error handling code

 drivers/hid/hid-roccat.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index 2596321..36a28b8 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -163,14 +163,15 @@ static int roccat_open(struct inode *inode, struct file *file)
 
 	device = devices[minor];
 
-	mutex_lock(&device->readers_lock);
-
 	if (!device) {
 		pr_emerg("roccat device with minor %d doesn't exist\n", minor);
-		error = -ENODEV;
-		goto exit_err;
+		kfree(reader);
+		mutex_unlock(&devices_lock);
+		return -ENODEV;
 	}
 
+	mutex_lock(&device->readers_lock);
+
 	if (!device->open++) {
 		/* power on device on adding first reader */
 		error = hid_hw_power(device->hid, PM_HINT_FULLON);
Julia,

thanks a lot for fixing this.

Could you please redo the patch in a way that it adds second 
'exit_unlock1' label (and renames 'exit_unlock' to 'exit_unlock2') (or 
any appropriate variation of the names) and preserve error path using goto 
instead of mixture of returns and gotos that this patch would introduce?

Thanks,

-- 
Jiri Kosina
SUSE Labs
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help