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

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

Date:	Sat, 29 Oct 2011 14:11:47 +0200 (CEST)
From:	Julia Lawall <julia@...u.dk>
To:	David Herrmann <dh.herrmann@...glemail.com>
Cc:	Arend van Spriel <arend@...adcom.com>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	Jiri Kosina <jkosina@...e.cz>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/5 v2] drivers/hid/hid-roccat.c: eliminate a null pointer
 dereference

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

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 <julia@...u.dk>

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

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